Skip to content

fix: populate reception_sequence_number and advertise sequence number features (backport #920)#922

Open
mergify[bot] wants to merge 1 commit intokiltedfrom
mergify/bp/kilted/pr-920
Open

fix: populate reception_sequence_number and advertise sequence number features (backport #920)#922
mergify[bot] wants to merge 1 commit intokiltedfrom
mergify/bp/kilted/pr-920

Conversation

@mergify
Copy link

@mergify mergify bot commented Mar 11, 2026

Description

rmw_feature_supported currently returns false for both RMW_FEATURE_MESSAGE_INFO_PUBLICATION_SEQUENCE_NUMBER and RMW_FEATURE_MESSAGE_INFO_RECEPTION_SEQUENCE_NUMBER, causing rclcpp to skip populating these fields in rclcpp::MessageInfo entirely. Additionally, reception_sequence_number is always set to 0 with a TODO comment at both take sites.

The fix:

  • Flips rmw_feature_supported to return true for both sequence number features.
  • Adds a std::atomic<uint64_t> reception_sn_ counter to SubscriptionData, initialized to 0. Each call to take_one_message or take_serialized_message increments it with fetch_add(1, std::memory_order_relaxed) and assigns the pre-increment value to message_info->reception_sequence_number.
  • publication_sequence_number was 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


This is an automatic backport of pull request #920 done by [Mergify](https://mergify.com).

@ahcorde
Copy link
Contributor

ahcorde commented Mar 11, 2026

Pulls: #922
Gist: https://gist.githubusercontent.com/ahcorde/c8db749b6b3c290eac49feaa2034f7bd/raw/2a1455dc25221a63a3a673d5e8d09e447e8164a9/ros2.repos
BUILD args: --packages-above-and-dependencies rmw_zenoh_cpp
TEST args: --packages-above rmw_zenoh_cpp
ROS Distro: kilted
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18436

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants