Skip to content

lola: Extract skeleton shared memory functionality#257

Merged
LittleHuba merged 5 commits intomainfrom
brem_skeleton_memory_manager
Apr 8, 2026
Merged

lola: Extract skeleton shared memory functionality#257
LittleHuba merged 5 commits intomainfrom
brem_skeleton_memory_manager

Conversation

@bemerybmw
Copy link
Copy Markdown
Contributor

The Skeleton currently performs Skeleton specific functionality (e.g.
PrepareOffer, PrepareStopOffer(), managing instance / usage marker
files, managing method subscription etc.). It also performs all
functionality to create / open shared memory and also insert events into
the maps in shared memory.

This commit splits up the functionality so that we have a single class
which is responsible for interacting with shared memory. This makes the
code much easier to navigate since the old skeleton code was a huge
monolith and also will allow us to optionally split up our unit tests as
well into skeleton functionality and shared memory operations. We could
even introduce an interface for SkeletonMemoryManager to completely
decouple the skeleton functionality tests from shared memory.

For now, we treat Skeleton and SkeletonMemoryManager as a single unit,
so we don't test the public interface of SkeletonMemoryManager.

Depends-on: #247

@bemerybmw bemerybmw force-pushed the brem_skeleton_memory_manager branch 2 times, most recently from 8cb811d to 534e998 Compare March 31, 2026 14:05
@bemerybmw bemerybmw marked this pull request as ready for review April 1, 2026 06:06
@bemerybmw bemerybmw force-pushed the brem_skeleton_memory_manager branch from 534e998 to 15b6693 Compare April 1, 2026 08:46
The Skeleton currently performs Skeleton specific functionality (e.g.
PrepareOffer, PrepareStopOffer(), managing instance / usage marker
files, managing method subscription etc.). It also performs all
functionality to create / open shared memory and also insert events into
the maps in shared memory.

This commit splits up the functionality so that we have a single class
which is responsible for interacting with shared memory. This makes the
code much easier to navigate since the old skeleton code was a huge
monolith and also will allow us to optionally split up our unit tests as
well into skeleton functionality and shared memory operations. We could
even introduce an interface for SkeletonMemoryManager to completely
decouple the skeleton functionality tests from shared memory.

For now, we treat Skeleton and SkeletonMemoryManager as a single unit,
so we don't test the public interface of SkeletonMemoryManager.
@bemerybmw bemerybmw force-pushed the brem_skeleton_memory_manager branch from 0f1f0fd to addf2f1 Compare April 1, 2026 14:11
namespace score::mw::com::impl::lola
{

/// \brief SkeletonMemoryManager manages are shared memory related functionality of a Skeleton.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// \brief SkeletonMemoryManager manages are shared memory related functionality of a Skeleton.
/// \brief SkeletonMemoryManager manages shared memory related functionality of a Skeleton.


/// \brief SkeletonMemoryManager manages are shared memory related functionality of a Skeleton.
///
/// A SkeletonMemoryManager is owned and dispatched to by a Skeleton.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// A SkeletonMemoryManager is owned and dispatched to by a Skeleton.
/// A SkeletonMemoryManager is owned and dispatched by a Skeleton.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really clear what it means to be dispached by the skeleton? Does it just mean that it is used by the skeleton?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I use dispatch here to indicate the the Skeleton is basically just forwarding calls to this class and this class contains all the functionality. For me, "uses" could also be that this class contains some helper functions which the Skeleton uses, but the skeleton still has some logic related to shared memory. But if you think it's unclear, I can change it. Don't have a strong opinion.

/// A SkeletonMemoryManager is owned and dispatched to by a Skeleton.
class SkeletonMemoryManager final
{
// Suppress "AUTOSAR C++14 A11-3-1", The rule declares: "Friend declarations shall not be used".
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rephrase the direct quote of the rule.

Suppress "AUTOSAR C++14 A11-3-1", Forbids the use of friends declaration.

class SkeletonMemoryManager final
{
// Suppress "AUTOSAR C++14 A11-3-1", The rule declares: "Friend declarations shall not be used".
// The "SkeletonAttorney" class is a helper, which sets the internal state of "Skeleton" accessing
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// The "SkeletonAttorney" class is a helper, which sets the internal state of "Skeleton" accessing
// The "SkeletonMemoryManagerTestAttorney" class is a helper, which sets the internal state of "Skeleton" accessing

class ShmResourceStorageSizes
{
public:
// Suppress "AUTOSAR C++14 M11-0-1" rule findings. This rule states: "Member data in non-POD class types shall
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rephrase the direct quote of the rule

auto SkeletonMemoryManager::OpenEventDataFromOpenedSharedMemory(const ElementFqId element_fq_id)
-> std::pair<EventDataStorage<SampleType>*, EventDataControlComposite<>>
{
// Suppress "AUTOSAR C++14 A15-5-3" rule findings. This rule states: "The std::terminate() function shall not be
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also remove the direct quote here

// coverity[autosar_cpp14_a15_4_2_violation]
register_shm_object_trace_callback.value()(
tracing::TracingRuntime::kDummyElementNameForShmRegisterCallback,
// Suppress "AUTOSAR C++14 A4-5-1" rule findings. This rule states: "Expressions with type enum or enum
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove direct quote

}

template <typename SampleType>
// Suppress "AUTOSAR C++14 M3-2-2" rule finding. This rule declares: "The One Definition Rule shall not be
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrase the direct quote

event_data_control_asil_b = &(event_control_asil_b_it->second.data_control);
}

// Suppress "AUTOSAR C++14 A5-3-2" rule finding. This rule declares: "Null pointers shall not be dereferenced.".
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrase the direct quote

"Could not get EventDataStorage*");

// Suppress "AUTOSAR C++14 A3-8-1" rule findings. This rule declares:
// "An object shall not be accessed outside of its lifetime"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrase direct quote

@LittleHuba LittleHuba added this pull request to the merge queue Apr 8, 2026
Merged via the queue into main with commit 06e1257 Apr 8, 2026
13 checks passed
@LittleHuba LittleHuba deleted the brem_skeleton_memory_manager branch April 8, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants