From 64c21ad42827b00e6a53c0ddf80266d272ecfa63 Mon Sep 17 00:00:00 2001 From: hskrieg Date: Mon, 14 Nov 2022 11:17:22 -0500 Subject: [PATCH] uorb: allow for more than 255 uORB messages Increased size for ORB_ID from uint8_t to uint16_t Created a type: orb_id_size_t = uint16_t. There are still a couple of places where the size of the ORB_ID is assumed to be less than 16-bits. The places that I have found are commented regarding this and can be found with a search on orb_id_size_t. --- Tools/msg/templates/uorb/msg.cpp.em | 2 +- Tools/msg/templates/uorb/uORBTopics.cpp.em | 2 +- Tools/msg/templates/uorb/uORBTopics.hpp.em | 2 +- platforms/common/uORB/uORB.h | 11 ++++++----- platforms/common/uORB/uORBDeviceMaster.cpp | 2 +- platforms/common/uORB/uORBDeviceMaster.hpp | 2 +- src/modules/logger/logged_topics.h | 2 +- src/modules/logger/logger.h | 2 +- src/modules/uxrce_dds_client/utilities.hpp | 17 +++++++++++++---- 9 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Tools/msg/templates/uorb/msg.cpp.em b/Tools/msg/templates/uorb/msg.cpp.em index e613d9511f..c3a82cc014 100644 --- a/Tools/msg/templates/uorb/msg.cpp.em +++ b/Tools/msg/templates/uorb/msg.cpp.em @@ -77,7 +77,7 @@ topic_fields = ["%s %s" % (convert_type(field.type, True), field.name) for field constexpr char __orb_@(name_snake_case)_fields[] = "@( ";".join(topic_fields) );"; @[for topic in topics]@ -ORB_DEFINE(@topic, struct @uorb_struct, @(struct_size-padding_end_size), __orb_@(name_snake_case)_fields, static_cast(ORB_ID::@topic)); +ORB_DEFINE(@topic, struct @uorb_struct, @(struct_size-padding_end_size), __orb_@(name_snake_case)_fields, static_cast(ORB_ID::@topic)); @[end for] void print_message(const orb_metadata *meta, const @uorb_struct& message) diff --git a/Tools/msg/templates/uorb/uORBTopics.cpp.em b/Tools/msg/templates/uorb/uORBTopics.cpp.em index c8b1c88397..9a087416e4 100644 --- a/Tools/msg/templates/uorb/uORBTopics.cpp.em +++ b/Tools/msg/templates/uorb/uORBTopics.cpp.em @@ -76,5 +76,5 @@ const struct orb_metadata *get_orb_meta(ORB_ID id) return nullptr; } - return uorb_topics_list[static_cast(id)]; + return uorb_topics_list[static_cast(id)]; } diff --git a/Tools/msg/templates/uorb/uORBTopics.hpp.em b/Tools/msg/templates/uorb/uORBTopics.hpp.em index aea7015cc7..425661b2fb 100644 --- a/Tools/msg/templates/uorb/uORBTopics.hpp.em +++ b/Tools/msg/templates/uorb/uORBTopics.hpp.em @@ -62,7 +62,7 @@ static constexpr size_t orb_topics_count() { return ORB_TOPICS_COUNT; } */ extern const struct orb_metadata *const *orb_get_topics() __EXPORT; -enum class ORB_ID : uint8_t { +enum class ORB_ID : orb_id_size_t { @[for idx, topic_name in enumerate(topic_names_all)]@ @(topic_name) = @(idx), @[end for] diff --git a/platforms/common/uORB/uORB.h b/platforms/common/uORB/uORB.h index 7fdbb699d1..0e2abb2bcf 100644 --- a/platforms/common/uORB/uORB.h +++ b/platforms/common/uORB/uORB.h @@ -42,16 +42,17 @@ #include #include +typedef uint16_t orb_id_size_t; /** * Object metadata. */ struct orb_metadata { - const char *o_name; /**< unique object name */ - const uint16_t o_size; /**< object size */ - const uint16_t o_size_no_padding; /**< object size w/o padding at the end (for logger) */ - const char *o_fields; /**< semicolon separated list of fields (with type) */ - uint8_t o_id; /**< ORB_ID enum */ + const char *o_name; /**< unique object name */ + const uint16_t o_size; /**< object size */ + const uint16_t o_size_no_padding; /**< object size w/o padding at the end (for logger) */ + const char *o_fields; /**< semicolon separated list of fields (with type) */ + orb_id_size_t o_id; /**< ORB_ID enum */ }; typedef const struct orb_metadata *orb_id_t; diff --git a/platforms/common/uORB/uORBDeviceMaster.cpp b/platforms/common/uORB/uORBDeviceMaster.cpp index c39e4572db..bd3877f13b 100644 --- a/platforms/common/uORB/uORBDeviceMaster.cpp +++ b/platforms/common/uORB/uORBDeviceMaster.cpp @@ -150,7 +150,7 @@ int uORB::DeviceMaster::advertise(const struct orb_metadata *meta, bool is_adver // add to the node map. _node_list.add(node); - _node_exists[node->get_instance()].set((uint8_t)node->id(), true); + _node_exists[node->get_instance()].set((orb_id_size_t)node->id(), true); } group_tries++; diff --git a/platforms/common/uORB/uORBDeviceMaster.hpp b/platforms/common/uORB/uORBDeviceMaster.hpp index 94c97e61df..ccda58f8d3 100644 --- a/platforms/common/uORB/uORBDeviceMaster.hpp +++ b/platforms/common/uORB/uORBDeviceMaster.hpp @@ -98,7 +98,7 @@ public: return false; } - return _node_exists[instance][(uint8_t)id]; + return _node_exists[instance][(orb_id_size_t)id]; } /** diff --git a/src/modules/logger/logged_topics.h b/src/modules/logger/logged_topics.h index 4a990d5ba7..de27574ce8 100644 --- a/src/modules/logger/logged_topics.h +++ b/src/modules/logger/logged_topics.h @@ -88,7 +88,7 @@ public: RequestedSubscription sub[MAX_TOPICS_NUM]; int count{0}; - uint8_t excluded_optional_topic_ids[MAX_EXCLUDED_OPTIONAL_TOPICS_NUM]; + orb_id_size_t excluded_optional_topic_ids[MAX_EXCLUDED_OPTIONAL_TOPICS_NUM]; int num_excluded_optional_topic_ids{0}; }; diff --git a/src/modules/logger/logger.h b/src/modules/logger/logger.h index 57b324d0c0..1531ac8b73 100644 --- a/src/modules/logger/logger.h +++ b/src/modules/logger/logger.h @@ -365,7 +365,7 @@ private: uint16_t _event_sequence_offset{0}; ///< event sequence offset to account for skipped (not logged) messages uint16_t _event_sequence_offset_mission{0}; - uint8_t _excluded_optional_topic_ids[LoggedTopics::MAX_EXCLUDED_OPTIONAL_TOPICS_NUM]; + orb_id_size_t _excluded_optional_topic_ids[LoggedTopics::MAX_EXCLUDED_OPTIONAL_TOPICS_NUM]; int _num_excluded_optional_topic_ids{0}; LogWriter _writer; diff --git a/src/modules/uxrce_dds_client/utilities.hpp b/src/modules/uxrce_dds_client/utilities.hpp index 91e1996fb8..f20ae1d428 100644 --- a/src/modules/uxrce_dds_client/utilities.hpp +++ b/src/modules/uxrce_dds_client/utilities.hpp @@ -10,8 +10,16 @@ uxrObjectId topic_id_from_orb(ORB_ID orb_id, uint8_t instance = 0) { - if (orb_id != ORB_ID::INVALID) { - uint16_t id = static_cast(orb_id) + (instance * UINT8_MAX); + // Note that the uxrObjectId.id is a uint16_t so we need to cap the ID. + // orb_id_size_t is currently uint16_t (MAX = 65535) and a max # of instances as either 4 or 10. + // We want to use half of the available id's for writers and half for readers, + // so this works as long as orb_id < 3276. + // Unfortunately, limits does not appear to be available. So hard-coding for uint16_t. + if (orb_id != ORB_ID::INVALID && + //(orb_id_size_t) orb_id < (std::numeric_limits / (2*ORB_MULTI_MAX_INSTANCES) ) + (orb_id_size_t) orb_id < (65535U / (2U * (uint16_t)ORB_MULTI_MAX_INSTANCES)) + ) { + uint16_t id = static_cast(orb_id) + (instance * ORB_TOPICS_COUNT); uxrObjectId topic_id = uxr_object_id(id, UXR_TOPIC_ID); return topic_id; } @@ -94,8 +102,9 @@ static bool create_data_reader(uxrSession *session, uxrStreamId reliable_out_str return false; } - uint16_t id = index + 1000; - + // Use the second half of the available ID space. + // Add 1 so that we get a nice hex starting number: 0x8000 instead of 0x7fff. + uint16_t id = index + (65535U / 2U) + 1; uxrObjectId topic_id = uxr_object_id(id, UXR_TOPIC_ID); uint16_t topic_req = uxr_buffer_create_topic_bin(session, reliable_out_stream_id, topic_id, participant_id, topic_name,