Add get and set and notification support to the fields in the configuration by sahithi-nukala · Pull Request #405 · eclipse-score/communication

sahithi-nukala · 2026-05-11T12:16:04Z

Add useGetIfAvailable/useSetIfAvailable to LoLa field deployment

Introduce dedicated LolaFieldInstanceDeployment class replacing the former LolaEventInstanceDeployment type alias, adding field-specific members use_get_if_available_ and use_set_if_available_
Fix mw_com_config_schema.json: move useGetIfAvailable and useSetIfAvailable into the field "properties" object so they are correctly validated when additionalProperties is false.
Adapt existing test and add new tests for LolaFieldInstanceDeployment
Update PUML diagram and readme document

Issue: SWP-250429

bemerybmw · 2026-05-22T05:23:59Z

+class LolaFieldInstanceDeployment
+{
+  public:
+    using SampleSlotCountType = std::uint16_t;


These aliases are part of LolaEventInstanceDeployment so should be removed. If you need them here, then you should access them via LolaEventInstanceDeployment.

I have removed -
using SampleSlotCountType = std::uint16_t; using SubscriberCountType = std::uint8_t; using TracingSlotSizeType = std::uint8_t;
and updated with the "LolaEventInstanceDeployment". example like below -
void SetNumberOfSampleSlots(LolaEventInstanceDeployment::SampleSlotCountType number_of_sample_slots) noexcept;

bemerybmw · 2026-05-22T05:33:40Z

                        "type": "string",
                        "title": "Service Instance Specifier",
-                        "description": "Represents the InstanceSpecifier value. This is itself an AUTOSAR short-name-path (representing a port prototype of a software component instance). For comfort reasons, we allow also the usage of abbreviated short-name-paths (short-name-paths with a number of components starting from root being removed) in case they are still unique within an executable (this json/configuration is a per executable config!). From AUTOSAR concept perspective an executable contains a 'root sw-component', which itself can be a nested composition of AdaptiveApplicationSwComponentTypes). Because of this 'compositing feature' it could be the case, that the direct short-name of a port prototype isn’t unique within the composition making up the executable. In reality however the port names are 95% unique and forcing the usage of lengthy short-name-paths can be annoying! So this translates to the following rules:\n1. A config file with two “instanceSpecifiers”, where one is only a trailing substring of the other is INVALID!\n2. If user code makes a resolve/lookup providing an 'instanceSpecifier' in the form of an abbreviated short-name-path, which turns out NOT being unique with respect to the config, this is a hard programming/configuration fault, which shall lead to an abort."
+                        "description": "Represents the InstanceSpecifier value. This is itself an AUTOSAR short-name-path (representing a port prototype of a software component instance). For comfort reasons, we allow also the usage of abbreviated short-name-paths (short-name-paths with a number of components starting from root being removed) in case they are still unique within an executable (this json/configuration is a per executable config!). From AUTOSAR concept perspective an executable contains a 'root sw-component', which itself can be a nested composition of AdaptiveApplicationSwComponentTypes). Because of this 'compositing feature' it could be the case, that the direct short-name of a port prototype isn\u2019t unique within the composition making up the executable. In reality however the port names are 95% unique and forcing the usage of lengthy short-name-paths can be annoying! So this translates to the following rules:\n1. A config file with two \u201cinstanceSpecifiers\u201d, where one is only a trailing substring of the other is INVALID!\n2. If user code makes a resolve/lookup providing an 'instanceSpecifier' in the form of an abbreviated short-name-path, which turns out NOT being unique with respect to the config, this is a hard programming/configuration fault, which shall lead to an abort."


Why did this change?

bemerybmw · 2026-05-22T05:53:19Z


-            const auto& service_instance_map = [service_element_type, &lola_service_instance_deployment]() {
-                if (service_element_type == ServiceElementType::EVENT)
+            const auto service_element_name = service_element.service_element_name.data();


To minimize the duplicated code, maybe we can change the lambda to be get_number_of_tracing_slots which accepts auto service_elements_map (which can be events_ or fields_) and do the map lookup within the lambda?

Updated to lambda that takes service_elements_map (which can be events_ or fields_) to avoid the duplicated code.

bemerybmw · 2026-05-22T05:57:30Z

+    EXPECT_EQ(secondDeploymentInfo.fields_.at("CurrentTemperatureFrontLeft")
+                  .lola_event_instance_deployment_.max_concurrent_allocations_.value(),
+              1);
+    EXPECT_FALSE(secondDeploymentInfo.fields_.at("CurrentTemperatureFrontLeft").use_get_if_available_);


The values should be changed so that they're true. Makes it more likely to catch if we're not correctly parsing the value (since defualt is false).

Done. And updated the mw_com_config.json file also accordingly

bemerybmw · 2026-05-22T06:06:04Z

+    {static} + CreateFromJson(json_object : const score::json::Object&) : LolaFieldInstanceDeployment
+    + Serialize() const : score::json::Object
+    --
+    <u>Type Aliases:</u>


Aliases should be removed since they're part of the event deployment.

bemerybmw · 2026-05-22T06:06:46Z

-    + is_set_configured: bool
-    + is_get_configured: bool
+    {static} + constexpr serializationVersion : std::uint32_t
+    + max_subscribers_ : std::optional<SubscriberCountType>


We now compose a LolaEventInstanceDeployment.

Done. I have removed the max_subscribers_, max_concurrent_allocations_, enforce_max_samples_
and updated with LolaEventInstanceDeployment.

bemerybmw · 2026-05-22T07:32:55Z

+    // coverity[autosar_cpp14_m11_0_1_violation]
+    LolaEventInstanceDeployment lola_event_instance_deployment_;
+    // coverity[autosar_cpp14_m11_0_1_violation]
+    bool use_get_if_available_;


Should be an optional because it's mandatory on proxy side and not used on skeleton side. Doesn't make sense to have a value on skeleton side.

crimson11 · 2026-05-26T08:24:13Z

        kDefaultLolaInstanceId,
        {},
-        {{test::kFooEventName, LolaFieldInstanceDeployment{test::kMaxSlots, 10U, 1U, true, 0}}},
+        {{test::kFooEventName,


hm - unexpected to me ... why don't you use kFooFieldName here?

Updated with kFooFieldName

crimson11 · 2026-05-26T08:24:47Z

        kDefaultLolaInstanceId,
        {},
-        {{test::kFooEventName, LolaFieldInstanceDeployment{test::kMaxSlots, 10U, 1U, true, 0}}},
+        {{test::kFooEventName,


see above ... why not kFooFieldName?

crimson11 · 2026-05-26T08:26:24Z


        lola_field_inst_depls.push_back(
-            {test::kFooEventName, LolaFieldInstanceDeployment{number_of_slots, 10U, 1U, true, 0}});
+            {test::kFooEventName,


see above ... kFooFieldName ?

crimson11 · 2026-05-26T08:26:59Z

+                 LolaEventInstanceDeployment{number_of_slots, 10U, 1U, true, 0}, false, false}});
        lola_field_inst_depls.push_back(
-            {test::kDumbEventName, LolaFieldInstanceDeployment{number_of_slots, 10U, 1U, true, 0}});
+            {test::kDumbEventName,


see above ... kDumbFieldName ?

crimson11 · 2026-05-26T08:46:02Z

 }

-TEST_F(LolaFieldInstanceDeploymentFixture, UseSetIfAvailableIsTrueAfterRoundTripSerialisation)
+INSTANTIATE_TEST_SUITE_P(UseGetIfAvailable, LolaFieldInstanceDeploymentUseGetParamFixture, ::testing::Values(true));


Hm? You are only instantiating the parametrized test with true here? Shouldn't you do:
INSTANTIATE_TEST_SUITE_P(UseGetIfAvailable, LolaFieldInstanceDeploymentUseGetParamFixture, ::testing::Values(true, false)); or
INSTANTIATE_TEST_SUITE_P(UseGetIfAvailable, LolaFieldInstanceDeploymentUseGetParamFixture, ::testing::Bool());

... and I think you still have a lot duplication and don't exploit parametrized-tests fully!
Your parametrization should consist of bool-pairs (1st element is enableGet, 2nd element is enableSet).
So your testing-values should be:

std::pair<bool, bool>{true, true} std::pair<bool, bool>{true, false} std::pair<bool, bool>{false, true} std::pair<bool, bool>{false, false}

I would even go one step further and make it something like:

enum class Getter { ENABLED, DISABLED }; std::pair<Getter, Setter>{Getter::ENABLED, Setter::ENABLED}; std::pair<Getter, Setter>{Getter::ENABLED, Setter::DISABLED};

crimson11 · 2026-05-26T09:56:57Z

 namespace detail
 {

+template <typename LolaServiceElementInstanceDeployment>


This pre-decl seems unneeded? Just shift the definition in line 50 below the GetSkeletonEventProperties() func template beginning in line 57?

crimson11 · 2026-05-26T10:10:19Z

OK. I guess our plumbing code now is "a mess"!?
CreateSkeletonServiceElement and the template arg element_type imply, that this func-template can deal with all the element types (Event/Field/Method). But this is dead-wrong! If you look at the code (namely line 119), where explicitly event-properties are set up ... it is obvious, that we woud get compile-time errors, when trying to instantiate this func-template for a METHOD. (Right now we are obviously NOT doing it - the plumbing code for methods doesn't use this.)

Bottom line:

at least rename the func-template to CreateSkeletonEventOrField()

since we still might need the template arg element_type, we should static_assert here, that it is either FIELD or EVENT.

eventually shift the whole code from skeleton_service_element_binding_factory_impl.h to skeleton_event_binding_factory_impl.h ... and "re-use" the functionality from there in case of a FIELD. skeleton_service_element_binding_factory_impl.h can then be removed ...

@bemerybmw : Please add your view on this!

From memory, Krishna has a pr somewhere making a similar change. But I agree that it should be CreateSkeletonEventOrField for the moment. In the future, I don't have a strong preference about having skeleton_service_element_binding_factory_impl (although renamed) or calling the event function directly from the field factory.

crimson11 · 2026-05-26T10:56:28Z

-                if (service_element_type == ServiceElementType::FIELD)
+            const auto service_element_name = service_element.service_element_name.data();
+            const auto get_number_of_tracing_slots =
+                [&](const auto& service_elements_map) noexcept -> LolaEventInstanceDeployment::TracingSlotSizeType {


Nitpicky ;) - You don't need to (globally) capture here (this you should typically avoid) ... why not hand over service_element_name as a call-arg to the lambda?

bemerybmw · 2026-05-27T06:15:20Z

                                         lola_service_element_instance_deployment.enforce_max_samples_};
 }

+inline lola::SkeletonEventProperties GetSkeletonEventProperties(


Why have this intermediate function?

removed the inline function and directly updated the GetSkeletonEventProperties

bemerybmw · 2026-05-27T06:28:30Z

From memory, Krishna has a pr somewhere making a similar change. But I agree that it should be CreateSkeletonEventOrField for the moment. In the future, I don't have a strong preference about having skeleton_service_element_binding_factory_impl (although renamed) or calling the event function directly from the field factory.

- Introduce dedicated LolaFieldInstanceDeployment class replacing the former LolaEventInstanceDeployment type alias, adding field-specific members use_get_if_available_ and use_set_if_available_. - Fix mw_com_config_schema.json: move useGetIfAvailable and useSetIfAvailable into the field "properties" object so they are correctly validated when additionalProperties is false. Issue: SWP-250429

- Update config_parser.cpp to read optional field values useGetIfAvailable/useSetIfAvailable - Fix existing unit test of loloa field instance deployment. Issue: SWP-250429

Update existing tests which refer LolaFieldInstanceDeployment to support additional optional useGetIfAvailable and useSetIfAvailable Issue: SWP-250429

- Add tests to verfiy behaviour of option field values useGetIfAvailable/useSetIfAvailable Issue: SWP-250429

- Update ReadMe and UML diagram to refelct updated changes optional field values useGetIfAvailable/useSetIfAvailable Issue: SWP-250429

- Remove the redundant unit tests and update the exsiting test for default field setter/getter value - Update the default values of useGetIfAvailable/useSetIfAvailable from default false to true - Update the plant UML diagram to retain only necessary information Issue: SWP-250429

… LolaEventInstanceDeployment members - Delegate GetNumberOfSampleSlots(), GetNumberOfTracingSlots(), and SetNumberOfSampleSlots() to the inner lola_event_instance_deployment_ member - Delegate CreateFromJson() event parsing and Serialize() event part to LolaEventInstanceDeployment - Add a non-template GetSkeletonEventProperties() overload in skeleton_service_element_binding_factory_impl.h that delegates to the event deployment overload - Update all call sites: config_parser.cpp, configuration_test_resources.cpp, and all affected test files

sahithi-nukala requested review from LittleHuba, bemerybmw, castler, crimson11, hoe-jo and limdor as code owners May 11, 2026 12:16

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch 2 times, most recently from 65d3943 to 7c3e519 Compare May 11, 2026 13:11

sahithi-nukala marked this pull request as draft May 18, 2026 12:45

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch from 7c3e519 to 4ccfdb8 Compare May 18, 2026 12:46

sahithi-nukala marked this pull request as ready for review May 18, 2026 13:14

sahithi-nukala marked this pull request as draft May 20, 2026 13:11

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch 3 times, most recently from 4ccfdb8 to eef7275 Compare May 20, 2026 13:34

sahithi-nukala marked this pull request as ready for review May 20, 2026 13:34

bemerybmw requested changes May 22, 2026

View reviewed changes

bemerybmw reviewed May 22, 2026

View reviewed changes

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch from eef7275 to b89ebe4 Compare May 25, 2026 11:31

sahithi-nukala marked this pull request as draft May 25, 2026 12:23

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch from 7287d32 to 70409f5 Compare May 26, 2026 04:04

sahithi-nukala requested a review from bemerybmw May 26, 2026 05:19

sahithi-nukala marked this pull request as ready for review May 26, 2026 07:11

crimson11 requested changes May 26, 2026

View reviewed changes

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch from 70409f5 to b4d8a2a Compare May 27, 2026 04:25

sahithi-nukala marked this pull request as draft May 27, 2026 04:25

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch 2 times, most recently from 6ee5667 to 5457da1 Compare May 27, 2026 06:07

bemerybmw reviewed May 27, 2026

View reviewed changes

soldier-sky and others added 7 commits May 29, 2026 12:46

Update config parser to read useGetIfAvailable/useSetIfAvailable

e02353f

- Update config_parser.cpp to read optional field values useGetIfAvailable/useSetIfAvailable - Fix existing unit test of loloa field instance deployment. Issue: SWP-250429

Adapt existing tests to support fields with optional setter/getter

dc4223e

Update existing tests which refer LolaFieldInstanceDeployment to support additional optional useGetIfAvailable and useSetIfAvailable Issue: SWP-250429

Add unit tests to verify optional setter and getter field values

e3180bf

- Add tests to verfiy behaviour of option field values useGetIfAvailable/useSetIfAvailable Issue: SWP-250429

Update configuration documentation

a5f9c82

- Update ReadMe and UML diagram to refelct updated changes optional field values useGetIfAvailable/useSetIfAvailable Issue: SWP-250429

Fixing the review comments

0f14d65

sahithi-nukala force-pushed the suya_Add_get_and_set_and_notification_support_to_the_fields_in_the_configuration branch from 0677f55 to 0f14d65 Compare May 29, 2026 07:16

sahithi-nukala marked this pull request as ready for review May 29, 2026 11:06

sahithi-nukala marked this pull request as draft May 29, 2026 12:21

Conversation

sahithi-nukala commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sahithi-nukala May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sahithi-nukala May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bemerybmw May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

sahithi-nukala commented May 11, 2026 •

edited

Loading

sahithi-nukala May 25, 2026 •

edited

Loading

sahithi-nukala May 25, 2026 •

edited

Loading

bemerybmw May 27, 2026 •

edited

Loading