-
Notifications
You must be signed in to change notification settings - Fork 81
Description
Overview
rmw_zenoh assigns a constant value of 0 to the reception_sequence_number field of rmw_message_info_t, but it should be using RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED.
rmw_zenoh/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp
Lines 425 to 426 in 4dcd184
| // TODO(clalancette): fill in reception_sequence_number | |
| message_info->reception_sequence_number = 0; |
rmw_zenoh/rmw_zenoh_cpp/src/detail/rmw_subscription_data.cpp
Lines 482 to 483 in 4dcd184
| // TODO(clalancette): fill in reception_sequence_number | |
| message_info->reception_sequence_number = 0; |
Background
rmw implementations which do not support a notion of publication/reception sequence numbers should the respective sequence number field value to the designated RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED constant.
Per rmw library documentation (emphasis mine):
uint64_t reception_sequence_number - Sequence number of the received message set by the subscription.
- [...]
- If the rmw implementation doesn’t support sequence numbers, its value will be RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED.
Where RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED is defined in rmw as:
#define RMW_MESSAGE_INFO_SEQUENCE_NUMBER_UNSUPPORTED UINT64_MAXThe other Tier 1 rmw implementations currently set this value as specified when recipient sequence numbers are not supported:
This is field sees extremely sparse actual use in the wild, as the only rmw implementation I'm aware of that supports it is rmw_connextdds. Further, the rmw_feature_supported function (correctly) returning false for RMW_FEATURE_MESSAGE_INFO_RECEPTION_SEQUENCE_NUMBER provides some indication that the value stored in that field should not be used, but the field value should still be made to conform to the rmw interface docs.