Skip to content

Commit

Permalink
Move the event_map out of the header file. (#226)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
clalancette authored Jul 1, 2024
1 parent bc761dd commit 5715f13
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 58 deletions.
30 changes: 30 additions & 0 deletions rmw_zenoh_cpp/src/detail/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include <deque>
#include <memory>
#include <mutex>
#include <unordered_map>
#include <utility>

#include "event.hpp"
Expand All @@ -20,8 +24,34 @@

#include "rmw/error_handling.h"

namespace
{
// RMW Event types that we support in rmw_zenoh.
static const std::unordered_map<rmw_event_type_t, rmw_zenoh_cpp::rmw_zenoh_event_type_t> 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)
Expand Down
14 changes: 1 addition & 13 deletions rmw_zenoh_cpp/src/detail/event.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
#ifndef DETAIL__EVENT_HPP_
#define DETAIL__EVENT_HPP_

#include <condition_variable>
#include <deque>
#include <memory>
#include <mutex>
#include <string>
#include <unordered_map>

#include "rmw/event.h"
#include "rmw/event_callback_type.h"
Expand Down Expand Up @@ -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<rmw_event_type_t, rmw_zenoh_event_type_t> 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.
Expand Down
45 changes: 22 additions & 23 deletions rmw_zenoh_cpp/src/rmw_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
zenoh_event_type](std::unique_ptr<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
{
if (pub_data == nullptr) {
return;
}
pub_data->events_mgr.add_new_event(
event_id,
zenoh_event_type,
std::move(zenoh_event));
}
);
Expand Down Expand Up @@ -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;
Expand All @@ -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<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
zenoh_event_type](std::unique_ptr<rmw_zenoh_cpp::rmw_zenoh_event_status_t> zenoh_event)
{
if (sub_data == nullptr) {
return;
}
sub_data->events_mgr.add_new_event(
event_id,
zenoh_event_type,
std::move(zenoh_event));
}
);
Expand All @@ -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);
Expand All @@ -162,10 +163,7 @@ rmw_event_set_callback(
rmw_zenoh_cpp::EventsManager * event_data =
static_cast<rmw_zenoh_cpp::EventsManager *>(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;
}
Expand All @@ -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);
Expand All @@ -201,7 +200,7 @@ rmw_take_event(
static_cast<rmw_zenoh_cpp::EventsManager *>(event_handle->data);
RMW_CHECK_ARGUMENT_FOR_NULL(event_data, RMW_RET_INVALID_ARGUMENT);
std::unique_ptr<rmw_zenoh_cpp::rmw_zenoh_event_status_t> 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",
Expand All @@ -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<rmw_requested_qos_incompatible_event_status_t *>(event_info);
RMW_CHECK_ARGUMENT_FOR_NULL(ei, RMW_RET_INVALID_ARGUMENT);
Expand Down
50 changes: 28 additions & 22 deletions rmw_zenoh_cpp/src/rmw_zenoh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<rmw_event_t *>(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<rmw_zenoh_cpp::EventsManager *>(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<rmw_zenoh_cpp::EventsManager *>(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;
}
}
}
Expand Down Expand Up @@ -3538,16 +3539,21 @@ rmw_wait(
for (size_t i = 0; i < events->event_count; ++i) {
auto event = static_cast<rmw_event_t *>(events->events[i]);
auto event_data = static_cast<rmw_zenoh_cpp::EventsManager *>(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;
}
}
}
Expand Down

0 comments on commit 5715f13

Please sign in to comment.