diff --git a/score/mw/com/impl/BUILD b/score/mw/com/impl/BUILD index 1f78b91aa..7bac46d8b 100644 --- a/score/mw/com/impl/BUILD +++ b/score/mw/com/impl/BUILD @@ -147,6 +147,8 @@ cc_library( deps = [ ":skeleton_event", ":skeleton_field_base", + "//score/mw/com/impl/methods:method_signature_element_ptr", + "//score/mw/com/impl/methods:skeleton_method", "//score/mw/com/impl/mocking:i_skeleton_field", "//score/mw/com/impl/plumbing:field", "//score/mw/com/impl/plumbing:sample_allocatee_ptr", @@ -246,6 +248,7 @@ cc_library( deps = [ ":error", ":skeleton_event_base", + "//score/mw/com/impl/methods:skeleton_method_base", "@score_baselibs//score/language/futurecpp", "@score_baselibs//score/result", "@score_logging//score/mw/log", diff --git a/score/mw/com/impl/skeleton_base_test.cpp b/score/mw/com/impl/skeleton_base_test.cpp index af6cbf069..abe542edf 100644 --- a/score/mw/com/impl/skeleton_base_test.cpp +++ b/score/mw/com/impl/skeleton_base_test.cpp @@ -667,6 +667,11 @@ class DummyField : public SkeletonFieldBase { return ResultBlank{}; }; + + bool IsSetHandlerRegistered() const noexcept override + { + return false; + } }; const auto kServiceIdentifier = make_ServiceIdentifierType("foo", 13, 37); const LolaServiceInstanceId kLolaInstanceId{23U}; diff --git a/score/mw/com/impl/skeleton_event.h b/score/mw/com/impl/skeleton_event.h index 2face4332..1a9245e89 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,8 @@ 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] - friend class SkeletonField; + template + friend class SkeletonField; // Empty struct that is used to make the second constructor only accessible to SkeletonEvent and SkeletonField (as // the latter is a friend). diff --git a/score/mw/com/impl/skeleton_field.h b/score/mw/com/impl/skeleton_field.h index b364d93f2..f8e80ae6c 100644 --- a/score/mw/com/impl/skeleton_field.h +++ b/score/mw/com/impl/skeleton_field.h @@ -13,6 +13,7 @@ #ifndef SCORE_MW_COM_IMPL_SKELETON_FIELD_H #define SCORE_MW_COM_IMPL_SKELETON_FIELD_H +#include "score/mw/com/impl/methods/skeleton_method.h" #include "score/mw/com/impl/plumbing/sample_allocatee_ptr.h" #include "score/mw/com/impl/plumbing/skeleton_field_binding_factory.h" #include "score/mw/com/impl/skeleton_event.h" @@ -27,18 +28,48 @@ #include #include +#include #include namespace score::mw::com::impl { -template +namespace detail +{ +/// Tag types for constructor overload disambiguation based on EnableSet template parameter. +struct EnableSetOnlyTag +{ +}; +struct EnableNeitherTag +{ +}; +} // namespace detail + +template class SkeletonField : public SkeletonFieldBase { public: using FieldType = SampleDataType; + using SetHandlerType = score::cpp::callback; - SkeletonField(SkeletonBase& parent, const std::string_view field_name); + /// \brief Constructs a SkeletonField with setter enabled. 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 > + SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableSetOnlyTag = {}); + + /// \brief Constructs a SkeletonField with no setter. 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 > + SkeletonField(SkeletonBase& parent, const std::string_view field_name, detail::EnableNeitherTag = {}); /// Constructor that allows to set the binding directly. /// @@ -92,6 +123,29 @@ class SkeletonField : public SkeletonFieldBase skeleton_field_mock_ = &skeleton_field_mock; } + // SFINAE-gated: only exists when EnableSet == true + template ::type = 0> + ResultBlank RegisterSetHandler(SetHandlerType handler) + { + auto wrapped_callback = [this, user_callback = std::move(handler)](FieldType new_value) -> Result { + // Allow user to validate/modify the value in-place + user_callback(new_value); + + // Store the (possibly modified) value as the latest field value + auto update_result = this->Update(new_value); + if (!update_result.has_value()) + { + return MakeUnexpected(update_result.error()); + } + + // Return the accepted value to the proxy + return new_value; + }; + + is_set_handler_registered_ = true; + return set_method_.get()->RegisterHandler(std::move(wrapped_callback)); + } + private: bool IsInitialValueSaved() const noexcept override { @@ -107,40 +161,130 @@ class SkeletonField : public SkeletonFieldBase std::unique_ptr initial_field_value_; ISkeletonField* skeleton_field_mock_; + + // Zero-cost conditional storage: unique_ptr when EnableSet=true, zero-size tag when false. + using SetMethodType = std::conditional_t(FieldType)>>, + detail::EnableSetOnlyTag>; + SetMethodType set_method_; + + // Tracks whether RegisterSetHandler() has been called. Zero-cost when EnableSet=false. + using IsSetHandlerRegisteredType = std::conditional_t; + IsSetHandlerRegisteredType is_set_handler_registered_{}; + + // EnableSet=true: checks the flag; EnableSet=false: no setter, no handler required. + bool IsSetHandlerRegistered() const noexcept override + { + if constexpr (EnableSet) + { + return is_set_handler_registered_; + } + return true; + } + + /// \brief Private delegating constructor used by the setter-enabled public ctor. + /// Receives already-constructed objects so that we can pass the event pointer to SkeletonFieldBase before + /// storing it here, mirroring the ProxyField pattern. + template > + SkeletonField(SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + std::unique_ptr(FieldType)>> set_method, + const std::string_view field_name); + + /// \brief Private delegating constructor used by the no-setter public ctor. + template > + SkeletonField(SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + const std::string_view field_name); }; -template -SkeletonField::SkeletonField(SkeletonBase& parent, const std::string_view field_name) - : SkeletonFieldBase{parent, - field_name, - std::make_unique>( - parent, - field_name, - SkeletonFieldBindingFactory::CreateEventBinding( - SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), - parent, - field_name), - typename SkeletonEvent::FieldOnlyConstructorEnabler{})}, +/// \brief Public ctor — EnableSet=true: delegates to the private ctor that also creates the set method. +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(FieldType)>>(parent, std::string(field_name) + "_Set"), + field_name} +{ +} + +/// \brief Public ctor — EnableSet=false: delegates to the private ctor with no set method. +template +template +SkeletonField::SkeletonField(SkeletonBase& parent, + const std::string_view field_name, + detail::EnableNeitherTag) + : SkeletonField{ + parent, + std::make_unique>(parent, + field_name, + SkeletonFieldBindingFactory::CreateEventBinding( + SkeletonBaseView{parent}.GetAssociatedInstanceIdentifier(), + parent, + field_name), + typename SkeletonEvent::FieldOnlyConstructorEnabler{}), + field_name} +{ +} + +/// \brief Testing ctor: binding is provided directly (used with mock bindings in tests). +template +SkeletonField::SkeletonField( + SkeletonBase& skeleton_base, + const std::string_view field_name, + std::unique_ptr> binding) + : SkeletonField{skeleton_base, + std::make_unique>(skeleton_base, field_name, std::move(binding)), + field_name} +{ +} + +/// \brief Private delegating ctor — setter enabled. Receives fully-constructed objects. +/// By constructing set_method_ here and passing the event pointer to SkeletonFieldBase first, +/// we guarantee the base class pointer is stable before we store set_method_. +template +template +SkeletonField::SkeletonField( + SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + std::unique_ptr(FieldType)>> set_method, + const std::string_view field_name) + : SkeletonFieldBase{parent, field_name, std::move(skeleton_event_dispatch)}, initial_field_value_{nullptr}, - skeleton_field_mock_{nullptr} + skeleton_field_mock_{nullptr}, + set_method_{std::move(set_method)} { SkeletonBaseView skeleton_base_view{parent}; skeleton_base_view.RegisterField(field_name, *this); } -template -SkeletonField::SkeletonField(SkeletonBase& skeleton_base, - const std::string_view field_name, - std::unique_ptr> binding) - : SkeletonFieldBase{skeleton_base, - field_name, - std::make_unique>(skeleton_base, field_name, std::move(binding))}, +/// \brief Private delegating ctor — no setter. Receives the already-constructed event. +template +template +SkeletonField::SkeletonField( + SkeletonBase& parent, + std::unique_ptr> skeleton_event_dispatch, + const std::string_view field_name) + : SkeletonFieldBase{parent, field_name, std::move(skeleton_event_dispatch)}, + initial_field_value_{nullptr}, skeleton_field_mock_{nullptr} { + SkeletonBaseView skeleton_base_view{parent}; + 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. @@ -148,15 +292,17 @@ SkeletonField::SkeletonField(SkeletonField&& other) noexcept // so it's still valid to access it here for moving into our own member. // coverity[autosar_cpp14_a12_8_3_violation] This is a false-positive. initial_field_value_{std::move(other.initial_field_value_)}, - skeleton_field_mock_{other.skeleton_field_mock_} + skeleton_field_mock_{other.skeleton_field_mock_}, + set_method_{std::move(other.set_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); } -template -auto SkeletonField::operator=(SkeletonField&& other) & noexcept -> SkeletonField& +template +auto SkeletonField::operator=(SkeletonField&& other) & noexcept + -> SkeletonField& { if (this != &other) { @@ -164,6 +310,7 @@ auto SkeletonField::operator=(SkeletonField&& other) & noexcept initial_field_value_ = std::move(other.initial_field_value_); skeleton_field_mock_ = std::move(other.skeleton_field_mock_); + set_method_ = std::move(other.set_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()}; @@ -179,8 +326,8 @@ auto SkeletonField::operator=(SkeletonField&& other) & noexcept /// 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 -ResultBlank SkeletonField::Update(const FieldType& sample_value) noexcept +template +ResultBlank SkeletonField::Update(const FieldType& sample_value) noexcept { if (skeleton_field_mock_ != nullptr) { @@ -197,8 +344,9 @@ ResultBlank SkeletonField::Update(const FieldType& sample_value) /// \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 -ResultBlank SkeletonField::Update(SampleAllocateePtr sample) noexcept +template +ResultBlank SkeletonField::Update( + SampleAllocateePtr sample) noexcept { if (skeleton_field_mock_ != nullptr) { @@ -213,8 +361,8 @@ ResultBlank SkeletonField::Update(SampleAllocateePtr /// /// 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) { @@ -232,12 +380,12 @@ Result> SkeletonField::Alloca return GetTypedEvent()->Allocate(); } -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] -ResultBlank SkeletonField::DoDeferredUpdate() noexcept +ResultBlank SkeletonField::DoDeferredUpdate() noexcept { SCORE_LANGUAGE_FUTURECPP_ASSERT_MESSAGE( initial_field_value_ != nullptr, @@ -253,14 +401,15 @@ ResultBlank SkeletonField::DoDeferredUpdate() noexcept return ResultBlank{}; } -template -ResultBlank SkeletonField::UpdateImpl(const FieldType& sample_value) noexcept +template +ResultBlank SkeletonField::UpdateImpl(const FieldType& sample_value) noexcept { return GetTypedEvent()->Send(sample_value); } -template -auto SkeletonField::GetTypedEvent() const noexcept -> SkeletonEvent* +template +auto SkeletonField::GetTypedEvent() const noexcept + -> SkeletonEvent* { auto* const typed_event = dynamic_cast*>(skeleton_event_dispatch_.get()); SCORE_LANGUAGE_FUTURECPP_ASSERT_PRD_MESSAGE(typed_event != nullptr, "Downcast to SkeletonEvent failed!"); diff --git a/score/mw/com/impl/skeleton_field_base.h b/score/mw/com/impl/skeleton_field_base.h index b38974f13..b481c7a37 100644 --- a/score/mw/com/impl/skeleton_field_base.h +++ b/score/mw/com/impl/skeleton_field_base.h @@ -14,6 +14,7 @@ #define SCORE_MW_COM_IMPL_SKELETON_FIELD_BASE_H #include "score/mw/com/impl/com_error.h" +#include "score/mw/com/impl/methods/skeleton_method_base.h" #include "score/mw/com/impl/skeleton_event_base.h" #include "score/mw/log/logging.h" @@ -61,6 +62,15 @@ class SkeletonFieldBase // after calling PrepareOffer() on the binding if (!was_prepare_offer_called_) { + // If the field is configured with a setter, the application must register + // a set handler before calling OfferService(), otherwise Offer() shall fail. + if (!IsSetHandlerRegistered()) + { + score::mw::log::LogWarn("lola") + << "Set handler must be registered before offering field: " << field_name_; + return MakeUnexpected(ComErrc::kSetHandlerNotSet); + } + if (!IsInitialValueSaved()) { score::mw::log::LogWarn("lola") << "Initial value must be set before offering field: " << field_name_; @@ -121,6 +131,9 @@ class SkeletonFieldBase /// \brief Returns whether the initial value has been saved by the user to be used by DoDeferredUpdate virtual bool IsInitialValueSaved() const noexcept = 0; + /// \brief Returns whether a set handler has been registered. + virtual bool IsSetHandlerRegistered() const noexcept = 0; + /// \brief Sets the initial value of the field. /// /// The existence of the value is a precondition of this function, so IsInitialValueSaved() should be checked before diff --git a/score/mw/com/impl/skeleton_field_base_test.cpp b/score/mw/com/impl/skeleton_field_base_test.cpp index 93ac7bfac..229cb01af 100644 --- a/score/mw/com/impl/skeleton_field_base_test.cpp +++ b/score/mw/com/impl/skeleton_field_base_test.cpp @@ -83,6 +83,11 @@ class MyDummyField : public SkeletonFieldBase return {}; } + bool IsSetHandlerRegistered() const noexcept override + { + return true; + } + bool was_deferred_update_called_{false}; bool is_initial_value_saved_{true}; }; @@ -94,6 +99,11 @@ class MyDummyFieldFailingDeferredUpdate final : public MyDummyField { return MakeUnexpected(ComErrc::kCommunicationLinkError); } + + bool IsSetHandlerRegistered() const noexcept override + { + return true; + } }; class SkeletonFieldBaseFixture : public ::testing::Test diff --git a/score/mw/com/impl/tracing/skeleton_tracing_test.cpp b/score/mw/com/impl/tracing/skeleton_tracing_test.cpp index 2947995c8..ac38df911 100644 --- a/score/mw/com/impl/tracing/skeleton_tracing_test.cpp +++ b/score/mw/com/impl/tracing/skeleton_tracing_test.cpp @@ -76,6 +76,11 @@ class MyDummyField : public SkeletonFieldBase { return {}; } + + bool IsSetHandlerRegistered() const noexcept override + { + return true; + } }; class SkeletonTracingFixture : public ::testing::Test diff --git a/score/mw/com/impl/traits.h b/score/mw/com/impl/traits.h index 5f2ce1b94..5727d3111 100644 --- a/score/mw/com/impl/traits.h +++ b/score/mw/com/impl/traits.h @@ -150,8 +150,8 @@ class SkeletonTrait template using Event = SkeletonEvent; - template - using Field = SkeletonField; + template + using Field = SkeletonField; template using Method = SkeletonMethod;