mw/com: Update event slot size based on field tags#746
Open
bemerybmw wants to merge 4 commits into
Open
Conversation
bemerybmw
force-pushed
the
brem_fields_update_slot_counts
branch
3 times, most recently
from
July 23, 2026 07:24
e8f58d6 to
50076f0
Compare
bemerybmw
changed the title
bemerybmw
force-pushed
the
brem_fields_update_slot_counts
branch
from
July 23, 2026 11:36
50076f0 to
15806fc
Compare
bemerybmw
force-pushed
the
brem_fields_update_slot_counts
branch
from
July 23, 2026 13:24
15806fc to
94153a6
Compare
Co-Authored-By: Krishna Satish Deshkulkarni Shankaranarayana <krishna-satish.deshkulkarni-shankaranarayana@bmw.de>
Co-Authored-By: Krishna Satish Deshkulkarni Shankaranarayana <krishna-satish.deshkulkarni-shankaranarayana@bmw.de>
Co-Authored-By: Krishna Satish Deshkulkarni Shankaranarayana <krishna-satish.deshkulkarni-shankaranarayana@bmw.de>
The number of slots required for a field now takes into account the slot requirements for different combinations of WithNotifier, WithGetter and WithSetter. These are stored in SkeletonEventProperties which is passed down to the lola binding. These values are used both to size the event slot vectors and also by the lola::SkeletonEventCommon to determine whether a getter was enabled or not. Further details are documented in score/mw/com/design/events_fields/README.md. Co-Authored-By: Krishna Satish Deshkulkarni Shankaranarayana <krishna-satish.deshkulkarni-shankaranarayana@bmw.de>
bemerybmw
force-pushed
the
brem_fields_update_slot_counts
branch
from
July 24, 2026 07:08
94153a6 to
39ebb93
Compare
bemerybmw
marked this pull request as ready for review
July 24, 2026 07:41
bemerybmw
requested review from
LittleHuba,
castler,
crimson11,
hoe-jo and
limdor
as code owners
July 24, 2026 07:41
crimson11
requested changes
Jul 24, 2026
| { | ||
| } | ||
|
|
||
| /// \brief Returns the total number of slots configured for the event (or field) configured by the user (via |
Contributor
There was a problem hiding this comment.
one of the configured words can be removed :)
| } | ||
|
|
||
| /// \brief Returns the total number of slots configured for the event (or field) configured by the user (via | ||
| /// numberOfSampleSlots and numberOfIpcTracingSlots configuration properties) and required to support field Getter |
Contributor
There was a problem hiding this comment.
to support field Getter
to support field Getter and Setter ?
| "title": "Number of sample slots", | ||
| "description": "Mandatory LoLa specific provider/skeleton side setting, how many sample slots shall be allocated. Value needs to be between 1 and 65535!", | ||
| "minimum": 1, | ||
| "description": "Mandatory LoLa specific provider/skeleton side setting, how many sample slots shall be allocated. Value needs to be between 1 and 65535 wheth the field has Notifier enabled! For a field with Notifier disabled, this value denotes the number of SampleAllocateePtrs that can be held (from a call to Allocate() on the Skeleton Field) at one time while the maximum supported number of concurrent getter calls are being processed. A value of 0 can therefore be provided which means that Allocate() can not be called while the getter calls are being processed. A value of 1 is recommended for most use cases.", |
Contributor
There was a problem hiding this comment.
wheth the field ...
->
when the field
| `numberOfSampleSlots` is taken into account. The configured value should be calculated based on the total number of subscribers | ||
| and the max number of samples they subscribe with (see [here for details](#sizing-of-event_slots-and-state_slots-vectors)). | ||
| If `WithNotifier` is not present, then `WithGetter` must be present (otherwise a consumer has no way of getting a field value). | ||
| In this case, `numberOfSampleSlots` should be configured according to the number of SampleAllocateePtrs (can be allocated via a |
Contributor
There was a problem hiding this comment.
SampleAllocateePtrs -> SampleAllocateePtrs
... other than that: I really like your very concise documentation here about the slot-sizing!
| const SkeletonEventProperties event_properties) noexcept | ||
| { | ||
| bool inserted; | ||
| const auto total_number_of_slots = event_properties.GetTotalNumberOfSlots(); |
Contributor
There was a problem hiding this comment.
why this explicit extraction into a tmp variable? To be 100% sure to use the same value in EventControl and EventDataStorage?
| `numberOfSampleSlots`. I.e. The sum of all `maxSamples` values of all subscribing consumers must not exceed | ||
| `numberOfSampleSlots`. Otherwise, the subscribe call will be rejected. However, this check is not ASIL level | ||
| overarching. See explanation in the `maxSubscribers` section below. | ||
| **Note**: In case of a field without `WithNotifier` this property is rather the number the number of |
| `numberOfSampleSlots`. Otherwise, the subscribe call will be rejected. However, this check is not ASIL level | ||
| overarching. See explanation in the `maxSubscribers` section below. | ||
| **Note**: In case of a field without `WithNotifier` this property is rather the number the number of | ||
| SampleAllocateePtrs (can be allocated via a call to `Allocate()` on the `SkeletonField`) which can be made |
| EXPECT_TRUE(store.HasNotifier()); | ||
| EXPECT_TRUE(store.HasSetter()); | ||
| EXPECT_TRUE(store.HasGetter()); | ||
| } |
Contributor
There was a problem hiding this comment.
So:
auto store = FieldTagsStore::Create<int>();
would "work" ... and return a FieldTagsStore with all bools being set to false?
Do we want this behaviour?
Shall we explicitly test for it?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mw/com: Update event slot size based on field tags
The number of slots required for a field now takes into account the
slot requirements for different combinations of WithNotifier, WithGetter
and WithSetter. These are stored in SkeletonEventProperties which is
passed down to the lola binding. These values are used both to size the
event slot vectors and also by the lola::SkeletonEventCommon to
determine whether a getter was enabled or not. Further details are
documented in score/mw/com/design/events_fields/README.md.
Co-Authored-By: Krishna Satish Deshkulkarni Shankaranarayana krishna-satish.deshkulkarni-shankaranarayana@bmw.de