From 433e358033fce7c02b44525574b04e4b3b00b4d9 Mon Sep 17 00:00:00 2001 From: muhseth Date: Tue, 17 Mar 2026 14:56:31 +0100 Subject: [PATCH 1/3] mw/com/impl: introduces EnableGet flag - SkeletonField now includes a new template parameter, EnableGet. - When EnableGet is true, a skeleton Get method attribute is added to SkeletonField. - Constructors were updated to support all relevant EnableSet and EnableGet combinations. Issue: SWP-249517 --- score/mw/com/impl/methods/skeleton_method.h | 4 +- score/mw/com/impl/skeleton_event.h | 4 +- score/mw/com/impl/skeleton_field.h | 150 +++++++++++++++----- score/mw/com/impl/skeleton_field_test.cpp | 74 +++++++--- score/mw/com/impl/traits.h | 4 +- 5 files changed, 173 insertions(+), 63 deletions(-) diff --git a/score/mw/com/impl/methods/skeleton_method.h b/score/mw/com/impl/methods/skeleton_method.h index 8164f2761..8f294c3cb 100644 --- a/score/mw/com/impl/methods/skeleton_method.h +++ b/score/mw/com/impl/methods/skeleton_method.h @@ -31,7 +31,7 @@ namespace score::mw::com::impl { -template +template class SkeletonField; template @@ -53,7 +53,7 @@ class SkeletonMethod final : public SkeletonMethodBase static_assert(return_value_is_not_a_pointer, "Return value can not be a pointer, since we can not put them in shared memory."); - template + template // coverity[autosar_cpp14_a11_3_1_violation] friend class SkeletonField; diff --git a/score/mw/com/impl/skeleton_event.h b/score/mw/com/impl/skeleton_event.h index ac50d016a..c18b0728a 100644 --- a/score/mw/com/impl/skeleton_event.h +++ b/score/mw/com/impl/skeleton_event.h @@ -36,7 +36,7 @@ namespace score::mw::com::impl // False Positive: this is a normal forward declaration. // coverity[autosar_cpp14_m3_2_3_violation] -template +template class SkeletonField; template @@ -51,7 +51,7 @@ class SkeletonEvent : public SkeletonEventBase // SkeletonField uses composition pattern to reuse code from SkeletonEvent. These two classes also have shared // private APIs which necessitates the use of the friend keyword. // coverity[autosar_cpp14_a11_3_1_violation] - template + template friend class SkeletonField; // Empty struct that is used to make the second constructor only accessible to SkeletonEvent and SkeletonField (as diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index da961010c..afd0c367e 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -35,29 +35,49 @@ namespace score::mw::com::impl { -template +template class SkeletonField : public SkeletonFieldBase { public: using FieldType = SampleDataType; - /// \brief Constructs a SkeletonField with setter enabled. Normal ctor used in production code. + /// \brief Constructs a SkeletonField for EnableSet=true, EnableGet=true. Normal ctor used in production code. + /// + /// \param parent Skeleton that contains this field. + /// \param field_name Field name of the field. + /// \param detail::EnableBothTag This parameter is only used for constructor overload disambiguation and + /// has no semantic meaning. + template > + SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableBothTag = {}); + + /// \brief Constructs a SkeletonField for EnableSet=false, EnableGet=true. Normal ctor used in production code. + /// + /// \param parent Skeleton that contains this field. + /// \param field_name Field name of the field. + /// \param detail::EnableGetOnlyTag This parameter is only used for constructor overload disambiguation and + /// has no semantic meaning. + template > + SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableGetOnlyTag = {}); + + /// \brief Constructs a SkeletonField for EnableSet=true, EnableGet=false. Normal ctor used in production code. /// /// \param parent Skeleton that contains this field. /// \param field_name Field name of the field. /// \param detail::EnableSetOnlyTag This parameter is only used for constructor overload disambiguation and - /// has no semantic meaning. The tag disambiguates the setter-enabled ctor from the no-setter ctor, as - /// otherwise both would have the same signature. - template > + /// has no semantic meaning. + template > SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableSetOnlyTag = {}); - /// \brief Constructs a SkeletonField with no setter. Normal ctor used in production code. + /// \brief Constructs a SkeletonField for EnableSet=false, EnableGet=false. Normal ctor used in production code. /// /// \param parent Skeleton that contains this field. /// \param field_name Field name of the field. /// \param detail::EnableNeitherTag This parameter is only used for constructor overload disambiguation and /// has no semantic meaning. - template > + template > SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableNeitherTag = {}); /// Constructor that allows to set the binding directly. @@ -210,12 +230,12 @@ class SkeletonField : public SkeletonFieldBase std::unique_ptr> get_method_; }; -/// \brief Public ctor — EnableSet=true: delegates to the private ctor that also creates the set method. -template -template -SkeletonField::SkeletonField(SkeletonBase& parent, +/// \brief Public ctor — EnableSet=true, EnableGet=true: delegates to the private ctor. +template +template +SkeletonField::SkeletonField(SkeletonBase& parent, const std::string_view field_name, - detail::EnableSetOnlyTag) + detail::EnableBothTag) : SkeletonField{ parent, std::make_unique>(parent, @@ -230,8 +250,6 @@ SkeletonField::SkeletonField(Skeleton field_name, ::score::mw::com::impl::MethodType::kSet, typename SkeletonMethod::FieldOnlyConstructorEnabler{}), - // TODO: Move get_method_ initialization into the delegating constructors (like set_method_) once the - // Get handler is implemented. std::make_unique>( parent, field_name, @@ -241,10 +259,60 @@ SkeletonField::SkeletonField(Skeleton { } -/// \brief Public ctor — EnableSet=false: delegates to the private ctor with no set method. -template -template -SkeletonField::SkeletonField(SkeletonBase& parent, +/// \brief Public ctor — EnableSet=false, EnableGet=true: delegates to the private ctor. +template +template +SkeletonField::SkeletonField(SkeletonBase& parent, + const std::string_view field_name, + detail::EnableGetOnlyTag) + : SkeletonField{ + parent, + std::make_unique>(parent, + field_name, + SkeletonFieldBindingFactory::CreateEventBinding( + SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), + parent, + field_name), + typename SkeletonEvent::FieldOnlyConstructorEnabler{}), + nullptr, + std::make_unique>( + parent, + field_name, + ::score::mw::com::impl::MethodType::kGet, + typename SkeletonMethod::FieldOnlyConstructorEnabler{}), + field_name} +{ +} + +/// \brief Public ctor — EnableSet=true, EnableGet=false: delegates to the private ctor. +template +template +SkeletonField::SkeletonField(SkeletonBase& parent, + const std::string_view field_name, + detail::EnableSetOnlyTag) + : SkeletonField{ + parent, + std::make_unique>(parent, + field_name, + SkeletonFieldBindingFactory::CreateEventBinding( + SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), + parent, + field_name), + typename SkeletonEvent::FieldOnlyConstructorEnabler{}), + std::make_unique>( + parent, + field_name, + ::score::mw::com::impl::MethodType::kSet, + typename SkeletonMethod::FieldOnlyConstructorEnabler{}), + nullptr, + field_name} +{ +} + +/// \brief Public ctor — EnableSet=false, EnableGet=false: delegates to the private ctor with no methods. +template +template +SkeletonField::SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableNeitherTag) : SkeletonField{ @@ -263,8 +331,8 @@ SkeletonField::SkeletonField(Skeleton } /// \brief Testing ctor: binding is provided directly (used with mock bindings in tests). -template -SkeletonField::SkeletonField( +template +SkeletonField::SkeletonField( SkeletonBase& skeleton_base, const std::string_view field_name, std::unique_ptr> binding) @@ -276,8 +344,8 @@ SkeletonField::SkeletonField( { } -template -SkeletonField::SkeletonField( +template +SkeletonField::SkeletonField( SkeletonBase& parent, std::unique_ptr> skeleton_event_dispatch, std::unique_ptr> skeleton_set_method_dispatch, @@ -294,8 +362,8 @@ SkeletonField::SkeletonField( skeleton_base_view.RegisterField(field_name, *this); } -template -SkeletonField::SkeletonField(SkeletonField&& other) noexcept +template +SkeletonField::SkeletonField(SkeletonField&& other) noexcept : SkeletonFieldBase(static_cast(other)), // known llvm bug (https://github.com/llvm/llvm-project/issues/63202) // This usage is safe because the previous line only moves the base class portion via static_cast. @@ -312,9 +380,9 @@ SkeletonField::SkeletonField(Skeleton skeleton_base_view.UpdateField(field_name_, *this); } -template -auto SkeletonField::operator=(SkeletonField&& other) & noexcept - -> SkeletonField& +template +auto SkeletonField::operator=(SkeletonField&& other) & noexcept + -> SkeletonField& { if (this != &other) { @@ -325,6 +393,8 @@ auto SkeletonField::operator=(Skeleto is_set_handler_registered_ = std::move(other.is_set_handler_registered_); set_method_ = std::move(other.set_method_); get_method_ = std::move(other.get_method_); + + // Since the address of this event has changed, we need update the address stored in the parent skeleton. SkeletonBaseView skeleton_base_view{skeleton_base_.get()}; skeleton_base_view.UpdateField(field_name_, *this); } @@ -338,8 +408,9 @@ auto SkeletonField::operator=(Skeleto /// field cannot be set until the Skeleton has been set up via Skeleton::OfferService(). Therefore, we create a /// callback that will update the field value with sample_value which will be called in the first call to /// SkeletonFieldBase::PrepareOffer(). -template -Result SkeletonField::Update(const FieldType& sample_value) noexcept +template +Result SkeletonField::Update( + const FieldType& sample_value) noexcept { if (skeleton_field_mock_ != nullptr) { @@ -356,8 +427,8 @@ Result SkeletonField::Update(co /// \brief FieldType is previously allocated by middleware and provided by the user to indicate that he is finished /// filling the provided pointer with live data. Dispatches to SkeletonEvent::Send() -template -Result SkeletonField::Update( +template +Result SkeletonField::Update( SampleAllocateePtr sample) noexcept { if (skeleton_field_mock_ != nullptr) @@ -373,8 +444,9 @@ Result SkeletonField::Update( /// /// This function cannot be currently called to set the initial value of a field as the shared memory must be first /// set up in the Skeleton::PrepareOffer() before the user can obtain / use a SampleAllocateePtr. -template -Result> SkeletonField::Allocate() noexcept +template +Result> SkeletonField:: + Allocate() noexcept { if (skeleton_field_mock_ != nullptr) { @@ -392,12 +464,12 @@ Result> SkeletonFieldAllocate(); } -template +template // Suppress "AUTOSAR C++14 A0-1-3" rule finding. This rule states: "Every function defined in an anonymous // namespace, or static function with internal linkage, or private member function shall be used.". // False-positive, method is used in the base class in PrepareOffer(). // coverity[autosar_cpp14_a0_1_3_violation : FALSE] -Result SkeletonField::DoDeferredUpdate() noexcept +Result SkeletonField::DoDeferredUpdate() noexcept { SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( initial_field_value_ != nullptr, @@ -413,15 +485,15 @@ Result SkeletonField::DoDeferre return Result{}; } -template -Result SkeletonField::UpdateImpl( +template +Result SkeletonField::UpdateImpl( const FieldType& sample_value) noexcept { return GetTypedEvent()->Send(sample_value); } -template -auto SkeletonField::GetTypedEvent() const noexcept +template +auto SkeletonField::GetTypedEvent() const noexcept -> SkeletonEvent* { auto* const typed_event = dynamic_cast*>(skeleton_event_dispatch_.get()); diff --git a/score/mw/com/impl/skeleton_field_test.cpp b/score/mw/com/impl/skeleton_field_test.cpp index 1255f5e12..a7b3f8d01 100644 --- a/score/mw/com/impl/skeleton_field_test.cpp +++ b/score/mw/com/impl/skeleton_field_test.cpp @@ -172,6 +172,62 @@ TEST(SkeletonFieldTest, SkeletonFieldContainsPublicSampleType) "Incorrect FieldType."); } +TEST(SkeletonFieldTest, CtorBothEnabledAcceptsEnableBothTag) +{ + using FieldType = SkeletonField; + static_assert(std::is_constructible_v, + "Constructor should accept EnableBothTag when ES=true and EG=true"); +} + +TEST(SkeletonFieldTest, CtorGetOnlyAcceptsEnableGetOnlyTag) +{ + using FieldType = SkeletonField; + static_assert(std::is_constructible_v, + "Constructor should accept EnableGetOnlyTag when ES=false and EG=true"); +} + +TEST(SkeletonFieldTest, CtorSetOnlyAcceptsEnableSetOnlyTag) +{ + using FieldType = SkeletonField; + static_assert(std::is_constructible_v, + "Constructor should accept EnableSetOnlyTag when ES=true and EG=false"); +} + +TEST(SkeletonFieldTest, CtorNeitherEnabledAcceptsEnableNeitherTag) +{ + using FieldType = SkeletonField; + static_assert(std::is_constructible_v, + "Constructor should accept EnableNeitherTag when ES=false and EG=false"); +} + +TEST(SkeletonFieldTest, CtorBothEnabledRejectsEnableGetOnlyTag) +{ + using FieldType = SkeletonField; + static_assert(!std::is_constructible_v, + "Constructor should reject EnableGetOnlyTag when ES=true and EG=true"); +} + +TEST(SkeletonFieldTest, CtorGetOnlyRejectsEnableSetOnlyTag) +{ + using FieldType = SkeletonField; + static_assert(!std::is_constructible_v, + "Constructor should reject EnableSetOnlyTag when ES=false and EG=true"); +} + +TEST(SkeletonFieldTest, CtorSetOnlyRejectsEnableBothTag) +{ + using FieldType = SkeletonField; + static_assert(!std::is_constructible_v, + "Constructor should reject EnableBothTag when ES=true and EG=false"); +} + +TEST(SkeletonFieldTest, CtorNeitherEnabledRejectsEnableBothTag) +{ + using FieldType = SkeletonField; + static_assert(!std::is_constructible_v, + "Constructor should reject EnableBothTag when ES=false and EG=false"); +} + // When Ticket-104261 is implemented, the Update call does not have to be deferred until OfferService is called. This // test can be reworked to remove the call to PrepareOffer() and simply test Update() before PrepareOffer() is called. using SkeletonFieldCopyUpdateTest = SkeletonFieldTestFixture; @@ -999,11 +1055,6 @@ TEST_F(SkeletonFieldSetHandlerTest, UserCallbackIsInvokedByWrappedHandler) EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(kInstanceIdWithLolaBinding, _, _, MethodType::kSet)) .WillOnce(Return(ByMove(std::move(capturing_binding)))); - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, - Create(kInstanceIdWithLolaBinding, _, _, MethodType::kGet)) - .WillOnce(InvokeWithoutArgs([]() { - return std::unique_ptr{}; - })); MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; @@ -1052,11 +1103,6 @@ TEST_F(SkeletonFieldSetHandlerTest, UserCallbackCanModifyValueInPlace) EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(kInstanceIdWithLolaBinding, _, _, MethodType::kSet)) .WillOnce(Return(ByMove(std::move(capturing_binding)))); - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, - Create(kInstanceIdWithLolaBinding, _, _, MethodType::kGet)) - .WillOnce(InvokeWithoutArgs([]() { - return std::unique_ptr{}; - })); MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; @@ -1102,11 +1148,6 @@ TEST_F(SkeletonFieldSetHandlerTest, WrappedHandlerLogsWhenUpdateFails) EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(kInstanceIdWithLolaBinding, _, _, MethodType::kSet)) .WillOnce(Return(ByMove(std::move(capturing_binding)))); - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, - Create(kInstanceIdWithLolaBinding, _, _, MethodType::kGet)) - .WillOnce(InvokeWithoutArgs([]() { - return std::unique_ptr{}; - })); MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; @@ -1186,9 +1227,6 @@ TEST_F(SkeletonFieldSetHandlerTest, SecondRegisterSetHandlerReplacesHandler) EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, Create(kInstanceIdWithLolaBinding, _, _, MethodType::kSet)) .WillOnce(Return(ByMove(std::move(capturing_binding)))); - EXPECT_CALL(skeleton_method_binding_factory_mock_guard_.factory_mock_, - Create(kInstanceIdWithLolaBinding, _, _, MethodType::kGet)) - .WillOnce(Return(ByMove(nullptr))); MySetterSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; diff --git a/score/mw/com/impl/traits.h b/score/mw/com/impl/traits.h index 1c51a0587..2578e2153 100644 --- a/score/mw/com/impl/traits.h +++ b/score/mw/com/impl/traits.h @@ -164,8 +164,8 @@ class SkeletonTrait template using Event = SkeletonEvent; - template - using Field = SkeletonField; + template + using Field = SkeletonField; template using Method = SkeletonMethod; From ad359c70ebf7a8d161f4a8746071af6fc6780899 Mon Sep 17 00:00:00 2001 From: muhseth Date: Tue, 17 Mar 2026 16:59:37 +0100 Subject: [PATCH 2/3] mw/com/impl: adds functionality for SkeletonField::Get - Automatically register a Get handler for fields with EnableGet. - Serve Get requests by reading the latest field value and returning it through the Get method. - Update the required slot calculation. - Add a test covering this behavior. Issue: SWP-249517 --- score/mw/com/impl/skeleton_field.h | 57 +++++++++++++++++------ score/mw/com/impl/skeleton_field_test.cpp | 32 +++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index afd0c367e..3e0e4a5ec 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -36,9 +37,9 @@ namespace score::mw::com::impl { template + const bool EnableSet = false, + const bool EnableNotifier = false, + const bool EnableGet = false> class SkeletonField : public SkeletonFieldBase { public: @@ -220,12 +221,38 @@ class SkeletonField : public SkeletonFieldBase return !is_set_handler_registered_; } + // Get handler registration + // SFINAE-gated: only exists when EnableGet == true + template ::type = 0> + void RegisterGetHandler() + { + if (get_method_ == nullptr) + { + return; + } + const auto result = get_method_.get()->RegisterHandler([this]() -> Result { + // need to serialize access to Get. In case of concurrent Get calls, + // we want to ensure that they are processed sequentially. + std::lock_guard lock{get_handler_mutex_}; + return SkeletonEventView{*GetTypedEvent()}.GetLatestSample(); + }); + if (!result.has_value()) + { + score::mw::log::LogError("lola") + << "RegisterGetHandler: failed to register get handler: " << result.error(); + } + } + std::unique_ptr initial_field_value_; ISkeletonField* skeleton_field_mock_; // Tracks whether RegisterSetHandler() has been called. bool is_set_handler_registered_; + // Zero-cost storage: only a real mutex when EnableGet=true, otherwise an empty zero-size type. + using GetHandlerMutexType = std::conditional_t; + GetHandlerMutexType get_handler_mutex_{}; + std::unique_ptr> set_method_; std::unique_ptr> get_method_; }; @@ -234,8 +261,8 @@ class SkeletonField : public SkeletonFieldBase template template SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableBothTag) + const std::string_view field_name, + detail::EnableBothTag) : SkeletonField{ parent, std::make_unique>(parent, @@ -263,8 +290,8 @@ SkeletonField::SkeletonFie template template SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableGetOnlyTag) + const std::string_view field_name, + detail::EnableGetOnlyTag) : SkeletonField{ parent, std::make_unique>(parent, @@ -288,8 +315,8 @@ SkeletonField::SkeletonFie template template SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableSetOnlyTag) + const std::string_view field_name, + detail::EnableSetOnlyTag) : SkeletonField{ parent, std::make_unique>(parent, @@ -313,8 +340,8 @@ SkeletonField::SkeletonFie template template SkeletonField::SkeletonField(SkeletonBase& parent, - const std::string_view field_name, - detail::EnableNeitherTag) + const std::string_view field_name, + detail::EnableNeitherTag) : SkeletonField{ parent, std::make_unique>(parent, @@ -360,6 +387,10 @@ SkeletonField::SkeletonFie { SkeletonBaseView skeleton_base_view{parent}; skeleton_base_view.RegisterField(field_name, *this); + if constexpr (EnableGet) + { + RegisterGetHandler(); + } } template @@ -445,8 +476,8 @@ Result SkeletonField /// This function cannot be currently called to set the initial value of a field as the shared memory must be first /// set up in the Skeleton::PrepareOffer() before the user can obtain / use a SampleAllocateePtr. template -Result> SkeletonField:: - Allocate() noexcept +Result> +SkeletonField::Allocate() noexcept { if (skeleton_field_mock_ != nullptr) { diff --git a/score/mw/com/impl/skeleton_field_test.cpp b/score/mw/com/impl/skeleton_field_test.cpp index a7b3f8d01..878a66ba4 100644 --- a/score/mw/com/impl/skeleton_field_test.cpp +++ b/score/mw/com/impl/skeleton_field_test.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -127,6 +128,14 @@ class SkeletonFieldTestFixture : public ::testing::Test std::optional return_storage_{}; }; +class MyGetEnabledSkeleton : public SkeletonBase +{ + public: + using SkeletonBase::SkeletonBase; + + SkeletonField my_get_field_{*this, kFieldName}; +}; + TEST(SkeletonFieldTest, NotCopyable) { RecordProperty("Verifies", "SCR-18221574"); @@ -228,6 +237,29 @@ TEST(SkeletonFieldTest, CtorNeitherEnabledRejectsEnableBothTag) "Constructor should reject EnableBothTag when ES=false and EG=false"); } +TEST(SkeletonFieldGetHandlerTest, RegisterGetHandlerInvokesGetLatestSampleOnBinding) +{ + RuntimeMockGuard runtime_mock_guard{}; + ON_CALL(runtime_mock_guard.runtime_mock_, GetTracingFilterConfig()).WillByDefault(Return(nullptr)); + + // Set up event binding mock + SkeletonFieldBindingFactoryMockGuard skeleton_field_binding_factory_mock_guard{}; + auto skeleton_field_binding_mock_ptr = std::make_unique>(); + EXPECT_CALL(skeleton_field_binding_factory_mock_guard.factory_mock_, + CreateEventBinding(kInstanceIdWithLolaBinding, _, kFieldName)) + .WillOnce(Return(ByMove(std::move(skeleton_field_binding_mock_ptr)))); + + // Set up method binding mock to verify RegisterHandler is called during construction + SkeletonMethodBindingFactoryMockGuard skeleton_method_binding_factory_mock_guard{}; + mock_binding::SkeletonMethod skeleton_method_binding_mock{}; + EXPECT_CALL(skeleton_method_binding_mock, RegisterHandler(_)).WillOnce(Return(ResultBlank{})); + EXPECT_CALL(skeleton_method_binding_factory_mock_guard.factory_mock_, Create(kInstanceIdWithLolaBinding, _, _, _)) + .WillOnce(Return(ByMove(std::make_unique(skeleton_method_binding_mock)))); + + // When a skeleton with EnableGet=true field is constructed, RegisterHandler is called automatically + MyGetEnabledSkeleton unit{std::make_unique(), kInstanceIdWithLolaBinding}; +} + // When Ticket-104261 is implemented, the Update call does not have to be deferred until OfferService is called. This // test can be reworked to remove the call to PrepareOffer() and simply test Update() before PrepareOffer() is called. using SkeletonFieldCopyUpdateTest = SkeletonFieldTestFixture; From 2c34b1ef6b7792644bf8a8f190ec3574d42228b8 Mon Sep 17 00:00:00 2001 From: muhseth Date: Thu, 19 Mar 2026 12:58:22 +0100 Subject: [PATCH 3/3] mw/com/impl: adds extra field slot when getter/setter is enabled - pass additional_slots through field binding creation - compute additional_slots from EnableSet || EnableGet - use configured_slots + additional_slots in event properties - add unit tests for +1 and default (unchanged) slot count --- .../i_skeleton_field_binding_factory.h | 9 ++++--- .../plumbing/skeleton_field_binding_factory.h | 6 +++-- .../skeleton_field_binding_factory_impl.h | 20 +++++++++------ .../skeleton_field_binding_factory_mock.h | 14 ++++++++++- ...ton_service_element_binding_factory_impl.h | 18 +++++++++---- ...n_service_element_binding_factory_test.cpp | 25 +++++++++++++++++++ score/mw/com/impl/skeleton_field.h | 12 ++++++--- 7 files changed, 82 insertions(+), 22 deletions(-) diff --git a/score/mw/com/impl/plumbing/i_skeleton_field_binding_factory.h b/score/mw/com/impl/plumbing/i_skeleton_field_binding_factory.h index 053898253..063ab2164 100644 --- a/score/mw/com/impl/plumbing/i_skeleton_field_binding_factory.h +++ b/score/mw/com/impl/plumbing/i_skeleton_field_binding_factory.h @@ -18,6 +18,7 @@ #include "score/mw/com/impl/skeleton_base.h" #include "score/mw/com/impl/skeleton_event_binding.h" +#include #include #include @@ -44,11 +45,13 @@ class ISkeletonFieldBindingFactory /// \param identifier The instance identifier containing the binding information. /// \param parent A reference to the Skeleton which owns this event. /// \param field_name The binding unspecific name of the field inside the skeleton denoted by instance identifier. + /// \param additional_slots_for_field_get_set Additional slots to reserve on top of configured sample slots. /// \return An instance of SkeletonEventBinding or nullptr in case of an error. virtual auto CreateEventBinding(const InstanceIdentifier& identifier, - SkeletonBase& parent, - const std::string_view field_name) noexcept - -> std::unique_ptr> = 0; + SkeletonBase& parent, + const std::string_view field_name, + std::size_t additional_slots_for_field_get_set = 0U) noexcept + -> std::unique_ptr> = 0; }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/plumbing/skeleton_field_binding_factory.h b/score/mw/com/impl/plumbing/skeleton_field_binding_factory.h index 1473748cd..d50aa06df 100644 --- a/score/mw/com/impl/plumbing/skeleton_field_binding_factory.h +++ b/score/mw/com/impl/plumbing/skeleton_field_binding_factory.h @@ -23,6 +23,7 @@ #include "score/mw/com/impl/skeleton_event_binding.h" #include +#include #include #include #include @@ -39,9 +40,10 @@ class SkeletonFieldBindingFactory final /// \brief See documentation in ISkeletonFieldBindingFactory. static std::unique_ptr> CreateEventBinding(const InstanceIdentifier& identifier, SkeletonBase& parent, - const std::string_view field_name) + const std::string_view field_name, + std::size_t additional_slots_for_field_get_set = 0U) { - return instance().CreateEventBinding(identifier, parent, field_name); + return instance().CreateEventBinding(identifier, parent, field_name, additional_slots_for_field_get_set); } /// \brief Inject a mock ISkeletonFieldBindingFactory. If a mock is injected, then all calls on diff --git a/score/mw/com/impl/plumbing/skeleton_field_binding_factory_impl.h b/score/mw/com/impl/plumbing/skeleton_field_binding_factory_impl.h index 34dd2b44a..4edc06c14 100644 --- a/score/mw/com/impl/plumbing/skeleton_field_binding_factory_impl.h +++ b/score/mw/com/impl/plumbing/skeleton_field_binding_factory_impl.h @@ -21,6 +21,7 @@ #include "score/mw/com/impl/skeleton_base.h" #include "score/mw/com/impl/skeleton_event_binding.h" +#include #include #include @@ -36,7 +37,8 @@ class SkeletonFieldBindingFactoryImpl : public ISkeletonFieldBindingFactory> CreateEventBinding( const InstanceIdentifier& identifier, SkeletonBase& parent, - const std::string_view field_name) noexcept override; + const std::string_view field_name, + std::size_t additional_slots_for_field_get_set = 0U) noexcept override; }; template @@ -49,13 +51,17 @@ template // This suppression should be removed after fixing [Ticket-173043](broken_link_j/Ticket-173043) // coverity[autosar_cpp14_a15_5_3_violation : FALSE] auto SkeletonFieldBindingFactoryImpl::CreateEventBinding(const InstanceIdentifier& identifier, - SkeletonBase& parent, - const std::string_view field_name) noexcept - -> std::unique_ptr> + SkeletonBase& parent, + const std::string_view field_name, + std::size_t additional_slots_for_field_get_set) noexcept + -> std::unique_ptr> { - return CreateSkeletonServiceElement, - lola::SkeletonEvent, - ServiceElementType::FIELD>(identifier, parent, field_name); + return CreateSkeletonServiceElement, + lola::SkeletonEvent, + ServiceElementType::FIELD>(identifier, + parent, + field_name, + additional_slots_for_field_get_set); } } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/plumbing/skeleton_field_binding_factory_mock.h b/score/mw/com/impl/plumbing/skeleton_field_binding_factory_mock.h index 6e9197cb7..1ab93a8ca 100644 --- a/score/mw/com/impl/plumbing/skeleton_field_binding_factory_mock.h +++ b/score/mw/com/impl/plumbing/skeleton_field_binding_factory_mock.h @@ -17,6 +17,8 @@ #include +#include + namespace score::mw::com::impl { @@ -27,7 +29,17 @@ class SkeletonFieldBindingFactoryMock : public ISkeletonFieldBindingFactory>, CreateEventBinding, (const InstanceIdentifier&, SkeletonBase&, const std::string_view), - (noexcept, override)); + (noexcept)); + + auto CreateEventBinding(const InstanceIdentifier& identifier, + SkeletonBase& parent, + const std::string_view field_name, + std::size_t additional_slots_for_field_get_set) noexcept + -> std::unique_ptr> override + { + static_cast(additional_slots_for_field_get_set); + return CreateEventBinding(identifier, parent, field_name); + } }; } // namespace score::mw::com::impl diff --git a/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_impl.h b/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_impl.h index 7614683b2..8ed32a050 100644 --- a/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_impl.h +++ b/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_impl.h @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -46,7 +47,8 @@ namespace detail template lola::SkeletonEventProperties GetSkeletonEventProperties( - const LolaServiceElementInstanceDeployment& lola_service_element_instance_deployment) + const LolaServiceElementInstanceDeployment& lola_service_element_instance_deployment, + std::size_t additional_slots_for_field_get_set = 0U) { if (!lola_service_element_instance_deployment.GetNumberOfSampleSlots().has_value()) { @@ -63,7 +65,11 @@ lola::SkeletonEventProperties GetSkeletonEventProperties( "not specified in the configuration. Terminating."; std::terminate(); } - return lola::SkeletonEventProperties{lola_service_element_instance_deployment.GetNumberOfSampleSlots().value(), + const auto number_of_slots = static_cast( + lola_service_element_instance_deployment.GetNumberOfSampleSlots().value()) + + additional_slots_for_field_get_set; + + return lola::SkeletonEventProperties{number_of_slots, lola_service_element_instance_deployment.max_subscribers_.value(), lola_service_element_instance_deployment.enforce_max_samples_}; } @@ -81,7 +87,8 @@ template std::unique_ptr { static_assert(element_type != ServiceElementType::INVALID); @@ -90,7 +97,7 @@ auto CreateSkeletonServiceElement(const InstanceIdentifier& identifier, using ReturnType = std::unique_ptr; auto visitor = score::cpp::overload( - [identifier_view, &parent, &service_element_name]( + [identifier_view, &parent, &service_element_name, additional_slots_for_field_get_set]( const LolaServiceTypeDeployment& lola_service_type_deployment) -> ReturnType { auto* const lola_parent = dynamic_cast(SkeletonBaseView{parent}.GetBinding()); if (lola_parent == nullptr) @@ -108,7 +115,8 @@ auto CreateSkeletonServiceElement(const InstanceIdentifier& identifier, const auto& lola_service_element_instance_deployment = GetServiceElementInstanceDeployment( lola_service_instance_deployment, service_element_name_str); const auto skeleton_event_properties = - detail::GetSkeletonEventProperties(lola_service_element_instance_deployment); + detail::GetSkeletonEventProperties(lola_service_element_instance_deployment, + additional_slots_for_field_get_set); const auto lola_service_element_id = GetServiceElementId(lola_service_type_deployment, service_element_name_str); diff --git a/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_test.cpp b/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_test.cpp index 218192c5b..bd4692ed3 100644 --- a/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_test.cpp +++ b/score/mw/com/impl/plumbing/skeleton_service_element_binding_factory_test.cpp @@ -138,6 +138,31 @@ INSTANTIATE_TEST_CASE_P(SkeletonServiceElementBindingFactoryParamaterisedDeathTe TEST_P(SkeletonServiceElementBindingFactoryParamaterisedFixture, CanConstructFixture) {} +class SkeletonServiceElementBindingFactoryFixture : public ::testing::Test +{ + protected: + const LolaFieldInstanceDeployment field_deployment_{{2U}, {3U}, 1U, true, 0U}; + + auto GetSkeletonEventProperties(const std::size_t additional_slots_for_field_get_set = 0U) const + { + return detail::GetSkeletonEventProperties(field_deployment_, additional_slots_for_field_get_set); + } +}; + +TEST_F(SkeletonServiceElementBindingFactoryFixture, AdditionalSlotsAreAddedToConfiguredNumberOfSlots) +{ + const auto skeleton_event_properties = GetSkeletonEventProperties(1U); + + EXPECT_EQ(skeleton_event_properties.number_of_slots, 3U); +} + +TEST_F(SkeletonServiceElementBindingFactoryFixture, DefaultAdditionalSlotsKeepsConfiguredNumberOfSlots) +{ + const auto skeleton_event_properties = GetSkeletonEventProperties(); + + EXPECT_EQ(skeleton_event_properties.number_of_slots, 2U); +} + TEST_P(SkeletonServiceElementBindingFactoryParamaterisedFixture, CanConstructServiceElement) { RecordProperty("Verifies", "SCR-21803701, SCR-21803702, SCR-5898925"); diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index 3e0e4a5ec..030caa15e 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -270,7 +270,8 @@ SkeletonField::SkeletonFie SkeletonFieldBindingFactory::CreateEventBinding( SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), parent, - field_name), + field_name, + (EnableSet || EnableGet) ? 1U : 0U), typename SkeletonEvent::FieldOnlyConstructorEnabler{}), std::make_unique>( parent, @@ -299,7 +300,8 @@ SkeletonField::SkeletonFie SkeletonFieldBindingFactory::CreateEventBinding( SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), parent, - field_name), + field_name, + (EnableSet || EnableGet) ? 1U : 0U), typename SkeletonEvent::FieldOnlyConstructorEnabler{}), nullptr, std::make_unique>( @@ -324,7 +326,8 @@ SkeletonField::SkeletonFie SkeletonFieldBindingFactory::CreateEventBinding( SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), parent, - field_name), + field_name, + (EnableSet || EnableGet) ? 1U : 0U), typename SkeletonEvent::FieldOnlyConstructorEnabler{}), std::make_unique>( parent, @@ -349,7 +352,8 @@ SkeletonField::SkeletonFie SkeletonFieldBindingFactory::CreateEventBinding( SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), parent, - field_name), + field_name, + (EnableSet || EnableGet) ? 1U : 0U), typename SkeletonEvent::FieldOnlyConstructorEnabler{}), nullptr, nullptr,