fix: populate reception_sequence_number and advertise sequence number features#920
Conversation
|
Pulls: #920 |
|
@YuanYuYuan or @JEnoch should we backport this ? |
I think we could backport it. This feature has landed since Humble ros2/rmw#318, and the API change is distro-agnostic. |
|
https://github.com/Mergifyio backport kilted jazzy humble |
✅ Backports have been createdDetails
Cherry-pick of 3cbcffb has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Description
rmw_feature_supportedcurrently returnsfalsefor bothRMW_FEATURE_MESSAGE_INFO_PUBLICATION_SEQUENCE_NUMBERandRMW_FEATURE_MESSAGE_INFO_RECEPTION_SEQUENCE_NUMBER, causing rclcpp to skip populating these fields inrclcpp::MessageInfoentirely. Additionally,reception_sequence_numberis always set to 0 with aTODOcomment at both take sites.The fix:
rmw_feature_supportedto returntruefor both sequence number features.std::atomic<uint64_t> reception_sn_counter toSubscriptionData, initialized to 0. Each call totake_one_messageortake_serialized_messageincrements it withfetch_add(1, std::memory_order_relaxed)and assigns the pre-increment value tomessage_info->reception_sequence_number.publication_sequence_numberwas already populated from the attachment; no change needed there.The attachment wire format (
sequence_number | source_timestamp | source_gid) is unchanged — this fix is purely in the subscriber-side bookkeeping.Closes #907
Closes #906