iox-eclipse-iceoryx#1104 Integrate non-nullable function into posh

Signed-off-by: Christian Eltzschig <[email protected]>
ApexAI · Sep 22, 2022 · 2223129 · 2223129
1 parent c73636f
commit 2223129
Show file tree

Hide file tree

Showing 24 changed files with 96 additions and 215 deletions.
diff --git a/iceoryx_binding_c/include/iceoryx_binding_c/enums.h b/iceoryx_binding_c/include/iceoryx_binding_c/enums.h
@@ -155,7 +155,6 @@ enum iox_ListenerResult
     ListenerResult_LISTENER_FULL,
     ListenerResult_EVENT_ALREADY_ATTACHED,
     ListenerResult_EMPTY_EVENT_CALLBACK,
-    ListenerResult_EMPTY_INVALIDATION_CALLBACK,
     ListenerResult_UNDEFINED_ERROR,
     ListenerResult_SUCCESS
 };

diff --git a/iceoryx_binding_c/source/cpp2c_enum_translation.cpp b/iceoryx_binding_c/source/cpp2c_enum_translation.cpp
@@ -123,8 +123,6 @@ iox_ListenerResult listenerResult(const iox::popo::ListenerError value) noexcept
         return ListenerResult_LISTENER_FULL;
     case ListenerError::EMPTY_EVENT_CALLBACK:
         return ListenerResult_EMPTY_EVENT_CALLBACK;
-    case ListenerError::EMPTY_INVALIDATION_CALLBACK:
-        return ListenerResult_EMPTY_INVALIDATION_CALLBACK;
     }
     return ListenerResult_UNDEFINED_ERROR;
 }

diff --git a/iceoryx_binding_c/source/cpp2c_subscriber.cpp b/iceoryx_binding_c/source/cpp2c_subscriber.cpp
@@ -82,10 +82,10 @@ cpp2c_Subscriber::getCallbackForIsStateConditionSatisfied(const SubscriberState
     switch (subscriberState)
     {
     case SubscriberState::HAS_DATA:
-        return {*this, &cpp2c_Subscriber::hasSamples};
+        return iox::popo::WaitSetIsConditionSatisfiedCallback(iox::cxx::in_place, *this, &cpp2c_Subscriber::hasSamples);
     }
 
-    return {};
+    return iox::cxx::nullopt;
 }
 
 void cpp2c_Subscriber::invalidateTrigger(const uint64_t uniqueTriggerId) noexcept

diff --git a/iceoryx_binding_c/test/moduletests/test_cpp2c_enum_translation.cpp b/iceoryx_binding_c/test/moduletests/test_cpp2c_enum_translation.cpp
@@ -236,8 +236,7 @@ TEST(cpp2c_enum_translation_test, ListenerResult)
     constexpr EnumMapping<iox::popo::ListenerError, iox_ListenerResult> LISTENER_ERRORS[]{
         {iox::popo::ListenerError::LISTENER_FULL, ListenerResult_LISTENER_FULL},
         {iox::popo::ListenerError::EVENT_ALREADY_ATTACHED, ListenerResult_EVENT_ALREADY_ATTACHED},
-        {iox::popo::ListenerError::EMPTY_EVENT_CALLBACK, ListenerResult_EMPTY_EVENT_CALLBACK},
-        {iox::popo::ListenerError::EMPTY_INVALIDATION_CALLBACK, ListenerResult_EMPTY_INVALIDATION_CALLBACK}};
+        {iox::popo::ListenerError::EMPTY_EVENT_CALLBACK, ListenerResult_EMPTY_EVENT_CALLBACK}};
 
     for (const auto listenerError : LISTENER_ERRORS)
     {
@@ -252,9 +251,6 @@ TEST(cpp2c_enum_translation_test, ListenerResult)
         case iox::popo::ListenerError::EMPTY_EVENT_CALLBACK:
             EXPECT_EQ(cpp2c::listenerResult(listenerError.cpp), listenerError.c);
             break;
-        case iox::popo::ListenerError::EMPTY_INVALIDATION_CALLBACK:
-            EXPECT_EQ(cpp2c::listenerResult(listenerError.cpp), listenerError.c);
-            break;
             // default intentionally left out in order to get a compiler warning if the enum gets extended and we forgot
             // to extend the test
         }

diff --git a/iceoryx_examples/icediscovery/include/discovery_monitor.hpp b/iceoryx_examples/icediscovery/include/discovery_monitor.hpp
@@ -45,7 +45,7 @@ class Discovery
     ///       but we could hide this from the user (by e.g. accessing ServiceRegistry via singleton
     ///       and passing it)
     template <typename Callback>
-    bool registerCallback(const Callback& callback);
+    void registerCallback(const Callback& callback);
 
     /// @brief deregister the active callback (if any)
     void deregisterCallback();
@@ -64,22 +64,17 @@ class Discovery
 
     /// @note currently only one callback can be active (and there is no need to have more
     /// as we only have one event at the ServiceDiscovery to attach to - SERVICE_REGISTRY_CHANGED)
-    callback_t m_callback;
+    iox::cxx::optional<callback_t> m_callback;
 
     static void invokeCallback(ServiceDiscovery* discovery, Discovery* self);
 };
 
 //! [registerCallback]
 template <typename Callback>
-bool Discovery::registerCallback(const Callback& callback)
+void Discovery::registerCallback(const Callback& callback)
 //! [registerCallback]
 {
-    if (m_callback)
-    {
-        return false;
-    }
-
-    m_callback = callback;
+    m_callback.emplace(callback);
     auto errorHandler = [](auto) {
         std::cerr << "failed to attach to listener" << std::endl;
         std::exit(EXIT_FAILURE);
@@ -90,8 +85,6 @@ bool Discovery::registerCallback(const Callback& callback)
     m_listener.attachEvent(*m_discovery, iox::runtime::ServiceDiscoveryEvent::SERVICE_REGISTRY_CHANGED, invoker)
         .or_else(errorHandler);
     //! [attach listener]
-
-    return true;
 }
 
 } // namespace discovery
diff --git a/iceoryx_examples/icediscovery/src/discovery_monitor.cpp b/iceoryx_examples/icediscovery/src/discovery_monitor.cpp
@@ -36,7 +36,7 @@ void Discovery::deregisterCallback()
     {
         m_listener.detachEvent(*m_discovery, iox::runtime::ServiceDiscoveryEvent::SERVICE_REGISTRY_CHANGED);
     }
-    m_callback = nullptr;
+    m_callback.reset();
 }
 //! [deregisterCallback]
 
@@ -54,8 +54,8 @@ ServiceContainer Discovery::findService(const iox::cxx::optional<iox::capro::IdS
 void Discovery::invokeCallback(ServiceDiscovery*, Discovery* self)
 {
     // discarded discovery argument is required by the listener
-    self->m_callback(*self);
+    (*self->m_callback)(*self);
 }
 //! [invokeCallback]
 
-} // namespace discovery
+} // namespace discovery
diff --git a/iceoryx_examples/waitset/ice_waitset_trigger.cpp b/iceoryx_examples/waitset/ice_waitset_trigger.cpp
@@ -218,11 +218,13 @@ class MyTriggerClass
         switch (event)
         {
         case MyTriggerClassStates::HAS_PERFORMED_ACTION:
-            return {*this, &MyTriggerClass::hasPerformedAction};
+            return iox::popo::WaitSetIsConditionSatisfiedCallback(
+                iox::cxx::in_place, *this, &MyTriggerClass::hasPerformedAction);
         case MyTriggerClassStates::IS_ACTIVATED:
-            return {*this, &MyTriggerClass::isActivated};
+            return iox::popo::WaitSetIsConditionSatisfiedCallback(
+                iox::cxx::in_place, *this, &MyTriggerClass::isActivated);
         }
-        return {};
+        return iox::cxx::nullopt;
     }
     //! [condition satisfied]
 

diff --git a/iceoryx_posh/include/iceoryx_posh/error_handling/error_handling.hpp b/iceoryx_posh/include/iceoryx_posh/error_handling/error_handling.hpp
@@ -102,9 +102,6 @@ namespace iox
     error(POPO__CONDITION_NOTIFIER_INDEX_TOO_LARGE) \
     error(POPO__CONDITION_NOTIFIER_SEMAPHORE_CORRUPT_IN_NOTIFY) \
     error(POPO__NOTIFICATION_INFO_TYPE_INCONSISTENCY_IN_GET_ORIGIN) \
-    error(POPO__TRIGGER_INVALID_RESET_CALLBACK) \
-    error(POPO__TRIGGER_INVALID_HAS_TRIGGERED_CALLBACK) \
-    error(POPO__TRIGGER_HANDLE_INVALID_RESET_CALLBACK) \
     error(POPO__TYPED_UNIQUE_ID_ROUDI_HAS_ALREADY_DEFINED_CUSTOM_UNIQUE_ID) \
     error(POPO__TYPED_UNIQUE_ID_OVERFLOW) \
     error(MEPOO__MEMPOOL_CONFIG_MUST_BE_ORDERED_BY_INCREASING_SIZE) \

diff --git a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp
@@ -97,7 +97,7 @@ constexpr uint32_t MAX_REQUEST_QUEUE_CAPACITY = 1024;
 // Waitset
 namespace popo
 {
-using WaitSetIsConditionSatisfiedCallback = cxx::function<bool()>;
+using WaitSetIsConditionSatisfiedCallback = cxx::optional<cxx::function<bool()>>;
 }
 constexpr uint32_t MAX_NUMBER_OF_CONDITION_VARIABLES = 1024U;
 

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/base_client.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/base_client.inl
@@ -129,9 +129,9 @@ BaseClient<PortT, TriggerHandleT>::getCallbackForIsStateConditionSatisfied(const
     switch (clientState)
     {
     case ClientState::HAS_RESPONSE:
-        return {*this, &SelfType::hasResponses};
+        return WaitSetIsConditionSatisfiedCallback(cxx::in_place, *this, &SelfType::hasResponses);
     }
-    return {};
+    return cxx::nullopt;
 }
 
 template <typename PortT, typename TriggerHandleT>

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/base_server.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/base_server.inl
@@ -133,9 +133,9 @@ BaseServer<PortT, TriggerHandleT>::getCallbackForIsStateConditionSatisfied(const
     switch (serverState)
     {
     case ServerState::HAS_REQUEST:
-        return {*this, &SelfType::hasRequests};
+        return WaitSetIsConditionSatisfiedCallback(cxx::in_place, *this, &SelfType::hasRequests);
     }
-    return {};
+    return cxx::nullopt;
 }
 
 template <typename PortT, typename TriggerHandleT>

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/base_subscriber.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/base_subscriber.inl
@@ -142,9 +142,9 @@ BaseSubscriber<port_t>::getCallbackForIsStateConditionSatisfied(const Subscriber
     switch (subscriberState)
     {
     case SubscriberState::HAS_DATA:
-        return {*this, &SelfType::hasData};
+        return WaitSetIsConditionSatisfiedCallback(cxx::in_place, *this, &SelfType::hasData);
     }
-    return {};
+    return cxx::nullopt;
 }
 
 template <typename port_t>

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/listener.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/listener.inl
@@ -144,11 +144,8 @@ ListenerImpl<Capacity>::addEvent(void* const origin,
         return cxx::error<ListenerError>(ListenerError::LISTENER_FULL);
     }
 
-    if (!m_events[index]->init(
-            index, origin, userType, eventType, eventTypeHash, callback, translationCallback, invalidationCallback))
-    {
-        return cxx::error<ListenerError>(ListenerError::EMPTY_INVALIDATION_CALLBACK);
-    }
+    m_events[index]->init(
+        index, origin, userType, eventType, eventTypeHash, callback, translationCallback, invalidationCallback);
     return cxx::success<uint32_t>(index);
 }
 

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/trigger.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/trigger.inl
@@ -43,11 +43,6 @@ inline Trigger::Trigger(T* const notificationOrigin,
     , m_originTriggerType(originTriggerType)
     , m_originTriggerTypeHash(originTriggerTypeHash)
 {
-    if (!resetCallback)
-    {
-        errorHandler(PoshError::POPO__TRIGGER_INVALID_RESET_CALLBACK, ErrorLevel::FATAL);
-        invalidate();
-    }
 }
 
 template <typename T, typename ContextDataType>
@@ -70,11 +65,6 @@ inline Trigger::Trigger(StateBasedTrigger_t,
               stateType,
               stateTypeHash)
 {
-    if (!hasTriggeredCallback)
-    {
-        errorHandler(PoshError::POPO__TRIGGER_INVALID_HAS_TRIGGERED_CALLBACK, ErrorLevel::FATAL);
-        invalidate();
-    }
 }
 
 template <typename T, typename ContextDataType>
@@ -86,15 +76,16 @@ inline Trigger::Trigger(EventBasedTrigger_t,
                         const uint64_t uniqueId,
                         const uint64_t notificationType,
                         const uint64_t notificationTypeHash) noexcept
-    : Trigger(notificationOrigin,
-              cxx::function<bool()>(),
-              resetCallback,
-              notificationId,
-              callback,
-              uniqueId,
-              TriggerType::EVENT_BASED,
-              notificationType,
-              notificationTypeHash)
+    : Trigger(
+        notificationOrigin,
+        []() { return false; },
+        resetCallback,
+        notificationId,
+        callback,
+        uniqueId,
+        TriggerType::EVENT_BASED,
+        notificationType,
+        notificationTypeHash)
 {
 }
 } // namespace popo

diff --git a/iceoryx_posh/include/iceoryx_posh/internal/popo/wait_set.inl b/iceoryx_posh/include/iceoryx_posh/internal/popo/wait_set.inl
@@ -17,6 +17,8 @@
 #ifndef IOX_POSH_POPO_WAIT_SET_INL
 #define IOX_POSH_POPO_WAIT_SET_INL
 
+#include "iceoryx_posh/popo/wait_set.hpp"
+
 namespace iox
 {
 namespace popo
@@ -81,7 +83,7 @@ WaitSet<Capacity>::attachImpl(T& eventOrigin,
     {
         m_triggerArray[*index].emplace(StateBasedTrigger,
                                        &eventOrigin,
-                                       hasTriggeredCallback,
+                                       *hasTriggeredCallback,
                                        invalidationCallback,
                                        eventId,
                                        eventCallback,
@@ -115,7 +117,7 @@ WaitSet<Capacity>::attachEvent(T& eventOrigin,
     static_assert(IS_EVENT_ENUM<EventType>, "Only enums with an underlying EventEnumIdentifier are allowed.");
 
     return attachImpl(eventOrigin,
-                      WaitSetIsConditionSatisfiedCallback(),
+                      cxx::nullopt,
                       eventId,
                       eventCallback,
                       static_cast<uint64_t>(eventType),
@@ -142,7 +144,7 @@ inline cxx::expected<WaitSetError> WaitSet<Capacity>::attachEvent(
     T& eventOrigin, const uint64_t eventId, const NotificationCallback<T, ContextDataType>& eventCallback) noexcept
 {
     return attachImpl(eventOrigin,
-                      WaitSetIsConditionSatisfiedCallback(),
+                      cxx::nullopt,
                       eventId,
                       eventCallback,
                       static_cast<uint64_t>(NoEventEnumUsed::PLACEHOLDER),

diff --git a/iceoryx_posh/include/iceoryx_posh/popo/listener.hpp b/iceoryx_posh/include/iceoryx_posh/popo/listener.hpp
@@ -44,7 +44,7 @@ class Event_t
 
     bool isEqualTo(const void* const origin, const uint64_t eventType, const uint64_t eventTypeHash) const noexcept;
     bool reset() noexcept;
-    bool init(const uint64_t eventId,
+    void init(const uint64_t eventId,
               void* const origin,
               void* const userType,
               const uint64_t eventType,
@@ -67,7 +67,7 @@ class Event_t
     void* m_userType = nullptr;
 
     uint64_t m_eventId = INVALID_ID;
-    cxx::function<void(uint64_t)> m_invalidationCallback;
+    cxx::function<void(uint64_t)> m_invalidationCallback = [](auto) {};
 };
 } // namespace internal
 
@@ -76,7 +76,6 @@ enum class ListenerError
     LISTENER_FULL,
     EVENT_ALREADY_ATTACHED,
     EMPTY_EVENT_CALLBACK,
-    EMPTY_INVALIDATION_CALLBACK
 };
 
 /// @brief The Listener is a class which reacts to registered events by

diff --git a/iceoryx_posh/include/iceoryx_posh/popo/trigger_handle.hpp b/iceoryx_posh/include/iceoryx_posh/popo/trigger_handle.hpp
@@ -37,16 +37,15 @@ namespace popo
 class TriggerHandle
 {
   public:
-    /// @note explicitly implemented for MSVC and QNX
-    TriggerHandle() noexcept;
+    TriggerHandle() noexcept = default;
 
     /// @brief Creates a TriggerHandle
     /// @param[in] conditionVariableDataRef reference to a condition variable data struct
     /// @param[in] resetCallback callback which will be called it goes out of scope or reset is called
     /// @param[in] uniqueTriggerId the unique trigger id of the Trigger which corresponds to the TriggerHandle. Usually
     /// stored in a Notifyable. It is required for the resetCallback
     TriggerHandle(ConditionVariableData& conditionVariableData,
-                  const cxx::function<void(uint64_t)> resetCallback,
+                  const cxx::function<void(uint64_t)>& resetCallback,
                   const uint64_t uniqueTriggerId) noexcept;
     TriggerHandle(const TriggerHandle&) = delete;
     TriggerHandle& operator=(const TriggerHandle&) = delete;
@@ -86,7 +85,7 @@ class TriggerHandle
 
   private:
     ConditionVariableData* m_conditionVariableDataPtr = nullptr;
-    cxx::function<void(uint64_t)> m_resetCallback;
+    cxx::function<void(uint64_t)> m_resetCallback = [](auto) {};
     uint64_t m_uniqueTriggerId = Trigger::INVALID_TRIGGER_ID;
     mutable std::recursive_mutex m_mutex;
 };

diff --git a/iceoryx_posh/source/popo/listener.cpp b/iceoryx_posh/source/popo/listener.cpp
@@ -48,7 +48,7 @@ void Event_t::executeCallback() noexcept
     m_translationCallback(m_origin, m_userType, m_callback);
 }
 
-bool Event_t::init(const uint64_t eventId,
+void Event_t::init(const uint64_t eventId,
                    void* const origin,
                    void* const userType,
                    const uint64_t eventType,
@@ -57,20 +57,14 @@ bool Event_t::init(const uint64_t eventId,
                    internal::TranslationCallbackRef_t translationCallback,
                    const cxx::function<void(uint64_t)> invalidationCallback) noexcept
 {
-    if (invalidationCallback)
-    {
-        m_eventId = eventId;
-        m_origin = origin;
-        m_userType = userType;
-        m_eventType = eventType;
-        m_eventTypeHash = eventTypeHash;
-        m_callback = &callback;
-        m_translationCallback = &translationCallback;
-        m_invalidationCallback = invalidationCallback;
-        return true;
-    }
-
-    return false;
+    m_eventId = eventId;
+    m_origin = origin;
+    m_userType = userType;
+    m_eventType = eventType;
+    m_eventTypeHash = eventTypeHash;
+    m_callback = &callback;
+    m_translationCallback = &translationCallback;
+    m_invalidationCallback = invalidationCallback;
 }
 
 bool Event_t::isEqualTo(const void* const origin, const uint64_t eventType, const uint64_t eventTypeHash) const noexcept
@@ -90,7 +84,7 @@ bool Event_t::reset() noexcept
         m_eventTypeHash = INVALID_ID;
         m_callback = nullptr;
         m_translationCallback = nullptr;
-        m_invalidationCallback = cxx::function<void(uint64_t)>();
+        m_invalidationCallback = [](auto) {};
 
         return true;
     }
@@ -100,7 +94,7 @@ bool Event_t::reset() noexcept
 bool Event_t::isInitialized() const noexcept
 {
     return m_origin != nullptr && m_eventId != INVALID_ID && m_eventType != INVALID_ID && m_eventTypeHash != INVALID_ID
-           && m_callback != nullptr && m_translationCallback != nullptr && m_invalidationCallback;
+           && m_callback != nullptr && m_translationCallback != nullptr;
 }
 } // namespace internal