From 5715f13bbfd3e5d6c7c187eb73c336f0124b11b2 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 1 Jul 2024 13:48:53 -0400 Subject: [PATCH] Move the event_map out of the header file. (#226) The main reason for this is that putting it in the header file means that it is duplicated into every compilation unit that includes it. This is slow, and in theory can cause issues if a pointer to the map is passed between compilation units. Instead, hide the map inside of the events.cpp file, and add a free function that converts between an RMW event type and a Zenoh event type. Signed-off-by: Chris Lalancette --- rmw_zenoh_cpp/src/detail/event.cpp | 30 ++++++++++++++++++ rmw_zenoh_cpp/src/detail/event.hpp | 14 +-------- rmw_zenoh_cpp/src/rmw_event.cpp | 45 +++++++++++++-------------- rmw_zenoh_cpp/src/rmw_zenoh.cpp | 50 +++++++++++++++++------------- 4 files changed, 81 insertions(+), 58 deletions(-) diff --git a/rmw_zenoh_cpp/src/detail/event.cpp b/rmw_zenoh_cpp/src/detail/event.cpp index 03bec0ec..4c2b0101 100644 --- a/rmw_zenoh_cpp/src/detail/event.cpp +++ b/rmw_zenoh_cpp/src/detail/event.cpp @@ -12,6 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include +#include +#include +#include #include #include "event.hpp" @@ -20,8 +24,34 @@ #include "rmw/error_handling.h" +namespace +{ +// RMW Event types that we support in rmw_zenoh. +static const std::unordered_map event_map{ + {RMW_EVENT_REQUESTED_QOS_INCOMPATIBLE, rmw_zenoh_cpp::ZENOH_EVENT_REQUESTED_QOS_INCOMPATIBLE}, + {RMW_EVENT_OFFERED_QOS_INCOMPATIBLE, rmw_zenoh_cpp::ZENOH_EVENT_OFFERED_QOS_INCOMPATIBLE}, + {RMW_EVENT_MESSAGE_LOST, rmw_zenoh_cpp::ZENOH_EVENT_MESSAGE_LOST}, + {RMW_EVENT_SUBSCRIPTION_MATCHED, rmw_zenoh_cpp::ZENOH_EVENT_SUBSCRIPTION_MATCHED}, + {RMW_EVENT_PUBLICATION_MATCHED, rmw_zenoh_cpp::ZENOH_EVENT_PUBLICATION_MATCHED}, + {RMW_EVENT_SUBSCRIPTION_INCOMPATIBLE_TYPE, + rmw_zenoh_cpp::ZENOH_EVENT_SUBSCRIPTION_INCOMPATIBLE_TYPE}, + {RMW_EVENT_PUBLISHER_INCOMPATIBLE_TYPE, rmw_zenoh_cpp::ZENOH_EVENT_PUBLISHER_INCOMPATIBLE_TYPE} + // TODO(clalancette): Implement remaining events +}; +} // namespace + namespace rmw_zenoh_cpp { +rmw_zenoh_event_type_t zenoh_event_from_rmw_event(rmw_event_type_t rmw_event_type) +{ + auto zenoh_event_it = event_map.find(rmw_event_type); + if (zenoh_event_it != event_map.end()) { + return zenoh_event_it->second; + } + + return ZENOH_EVENT_INVALID; +} + ///============================================================================= void DataCallbackManager::set_callback( const void * user_data, rmw_event_callback_t callback) diff --git a/rmw_zenoh_cpp/src/detail/event.hpp b/rmw_zenoh_cpp/src/detail/event.hpp index 96f8629a..9710711a 100644 --- a/rmw_zenoh_cpp/src/detail/event.hpp +++ b/rmw_zenoh_cpp/src/detail/event.hpp @@ -15,12 +15,10 @@ #ifndef DETAIL__EVENT_HPP_ #define DETAIL__EVENT_HPP_ -#include #include #include #include #include -#include #include "rmw/event.h" #include "rmw/event_callback_type.h" @@ -53,17 +51,7 @@ enum rmw_zenoh_event_type_t /// Helper value to indicate the maximum index of events supported. #define ZENOH_EVENT_ID_MAX rmw_zenoh_event_type_t::ZENOH_EVENT_PUBLICATION_MATCHED -// RMW Event types that we support in rmw_zenoh. -static const std::unordered_map event_map{ - {RMW_EVENT_REQUESTED_QOS_INCOMPATIBLE, ZENOH_EVENT_REQUESTED_QOS_INCOMPATIBLE}, - {RMW_EVENT_OFFERED_QOS_INCOMPATIBLE, ZENOH_EVENT_OFFERED_QOS_INCOMPATIBLE}, - {RMW_EVENT_MESSAGE_LOST, ZENOH_EVENT_MESSAGE_LOST}, - {RMW_EVENT_SUBSCRIPTION_MATCHED, ZENOH_EVENT_SUBSCRIPTION_MATCHED}, - {RMW_EVENT_PUBLICATION_MATCHED, ZENOH_EVENT_PUBLICATION_MATCHED}, - {RMW_EVENT_SUBSCRIPTION_INCOMPATIBLE_TYPE, ZENOH_EVENT_SUBSCRIPTION_INCOMPATIBLE_TYPE}, - {RMW_EVENT_PUBLISHER_INCOMPATIBLE_TYPE, ZENOH_EVENT_PUBLISHER_INCOMPATIBLE_TYPE} - // TODO(clalancette): Implement remaining events -}; +rmw_zenoh_event_type_t zenoh_event_from_rmw_event(rmw_event_type_t rmw_event_type); ///============================================================================= /// A struct to store status changes which can be mapped to rmw event statuses. diff --git a/rmw_zenoh_cpp/src/rmw_event.cpp b/rmw_zenoh_cpp/src/rmw_event.cpp index 33a950aa..964118ee 100644 --- a/rmw_zenoh_cpp/src/rmw_event.cpp +++ b/rmw_zenoh_cpp/src/rmw_event.cpp @@ -48,8 +48,9 @@ rmw_publisher_event_init( return RMW_RET_INCORRECT_RMW_IMPLEMENTATION; } - auto rmw_event_it = rmw_zenoh_cpp::event_map.find(event_type); - if (rmw_event_it == rmw_zenoh_cpp::event_map.end()) { + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "provided event_type %d is not supported by rmw_zenoh_cpp", event_type); return RMW_RET_UNSUPPORTED; @@ -62,16 +63,15 @@ rmw_publisher_event_init( // Register the event with graph cache. pub_data->context->impl->graph_cache->set_qos_event_callback( pub_data->entity, - rmw_event_it->second, + zenoh_event_type, [pub_data, - event_id = - rmw_event_it->second](std::unique_ptr zenoh_event) + zenoh_event_type](std::unique_ptr zenoh_event) { if (pub_data == nullptr) { return; } pub_data->events_mgr.add_new_event( - event_id, + zenoh_event_type, std::move(zenoh_event)); } ); @@ -103,8 +103,9 @@ rmw_subscription_event_init( return RMW_RET_INCORRECT_RMW_IMPLEMENTATION; } - auto rmw_event_it = rmw_zenoh_cpp::event_map.find(event_type); - if (rmw_event_it == rmw_zenoh_cpp::event_map.end()) { + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "provided event_type %d is not supported by rmw_zenoh_cpp", event_type); return RMW_RET_UNSUPPORTED; @@ -116,22 +117,21 @@ rmw_subscription_event_init( // Register the event with graph cache if the event is not ZENOH_EVENT_MESSAGE_LOST // since this is checked for in the subscription callback. - if (rmw_event_it->second == rmw_zenoh_cpp::ZENOH_EVENT_MESSAGE_LOST) { + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_MESSAGE_LOST) { return RMW_RET_OK; } sub_data->context->impl->graph_cache->set_qos_event_callback( sub_data->entity, - rmw_event_it->second, + zenoh_event_type, [sub_data, - event_id = - rmw_event_it->second](std::unique_ptr zenoh_event) + zenoh_event_type](std::unique_ptr zenoh_event) { if (sub_data == nullptr) { return; } sub_data->events_mgr.add_new_event( - event_id, + zenoh_event_type, std::move(zenoh_event)); } ); @@ -150,8 +150,9 @@ rmw_event_set_callback( RMW_CHECK_ARGUMENT_FOR_NULL(rmw_event, RMW_RET_INVALID_ARGUMENT); RMW_CHECK_ARGUMENT_FOR_NULL(rmw_event->data, RMW_RET_INVALID_ARGUMENT); - auto zenoh_event_it = rmw_zenoh_cpp::event_map.find(rmw_event->event_type); - if (zenoh_event_it == rmw_zenoh_cpp::event_map.end()) { + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(rmw_event->event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "RMW Zenoh does not support event [%d]", rmw_event->event_type); @@ -162,10 +163,7 @@ rmw_event_set_callback( rmw_zenoh_cpp::EventsManager * event_data = static_cast(rmw_event->data); RMW_CHECK_ARGUMENT_FOR_NULL(event_data, RMW_RET_INVALID_ARGUMENT); - event_data->event_set_callback( - zenoh_event_it->second, - callback, - user_data); + event_data->event_set_callback(zenoh_event_type, callback, user_data); return RMW_RET_OK; } @@ -189,8 +187,9 @@ rmw_take_event( return RMW_RET_INCORRECT_RMW_IMPLEMENTATION; } - auto zenoh_event_it = rmw_zenoh_cpp::event_map.find(event_handle->event_type); - if (zenoh_event_it == rmw_zenoh_cpp::event_map.end()) { + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(event_handle->event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "RMW Zenoh does not support event [%d]", event_handle->event_type); @@ -201,7 +200,7 @@ rmw_take_event( static_cast(event_handle->data); RMW_CHECK_ARGUMENT_FOR_NULL(event_data, RMW_RET_INVALID_ARGUMENT); std::unique_ptr st = event_data->pop_next_event( - zenoh_event_it->second); + zenoh_event_type); if (st == nullptr) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "rmw_take_event called when event queue for event type [%d] is empty", @@ -210,7 +209,7 @@ rmw_take_event( } // Now depending on the event, populate the rmw event status. - switch (zenoh_event_it->second) { + switch (zenoh_event_type) { case rmw_zenoh_cpp::ZENOH_EVENT_REQUESTED_QOS_INCOMPATIBLE: { auto ei = static_cast(event_info); RMW_CHECK_ARGUMENT_FOR_NULL(ei, RMW_RET_INVALID_ARGUMENT); diff --git a/rmw_zenoh_cpp/src/rmw_zenoh.cpp b/rmw_zenoh_cpp/src/rmw_zenoh.cpp index 8aa972b9..a4e20215 100644 --- a/rmw_zenoh_cpp/src/rmw_zenoh.cpp +++ b/rmw_zenoh_cpp/src/rmw_zenoh.cpp @@ -3385,21 +3385,22 @@ static bool check_and_attach_condition( if (events) { for (size_t i = 0; i < events->event_count; ++i) { auto event = static_cast(events->events[i]); - auto zenoh_event_it = rmw_zenoh_cpp::event_map.find(event->event_type); - if (zenoh_event_it != rmw_zenoh_cpp::event_map.end()) { - auto event_data = static_cast(event->data); - if (event_data != nullptr) { - if (event_data->queue_has_data_and_attach_condition_if_not( - zenoh_event_it->second, - wait_set_data)) - { - return true; - } - } - } else { + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(event->event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { RMW_SET_ERROR_MSG_WITH_FORMAT_STRING( "has_triggered_condition() called with unknown event %u. Report this bug.", event->event_type); + continue; + } + + auto event_data = static_cast(event->data); + if (event_data == nullptr) { + continue; + } + + if (event_data->queue_has_data_and_attach_condition_if_not(zenoh_event_type, wait_set_data)) { + return true; } } } @@ -3538,16 +3539,21 @@ rmw_wait( for (size_t i = 0; i < events->event_count; ++i) { auto event = static_cast(events->events[i]); auto event_data = static_cast(event->data); - if (event_data != nullptr) { - auto zenoh_event_it = rmw_zenoh_cpp::event_map.find(event->event_type); - if (zenoh_event_it != rmw_zenoh_cpp::event_map.end()) { - if (event_data->detach_condition_and_event_queue_is_empty(zenoh_event_it->second)) { - // Setting to nullptr lets rcl know that this subscription is not ready - events->events[i] = nullptr; - } else { - wait_result = true; - } - } + if (event_data == nullptr) { + continue; + } + + rmw_zenoh_cpp::rmw_zenoh_event_type_t zenoh_event_type = + rmw_zenoh_cpp::zenoh_event_from_rmw_event(event->event_type); + if (zenoh_event_type == rmw_zenoh_cpp::ZENOH_EVENT_INVALID) { + continue; + } + + if (event_data->detach_condition_and_event_queue_is_empty(zenoh_event_type)) { + // Setting to nullptr lets rcl know that this subscription is not ready + events->events[i] = nullptr; + } else { + wait_result = true; } } }