Skip to content

Conversation

@exMachina316
Copy link
Contributor

Fixes #983

  1. Added a conditional statement to set the raw flag when creating subscriptions for topics based on the value of filter_expr. If the --filter flag is used, filter_expr is not None and deserializing subscriptions are created. Otherwise filter_expr is None and 'raw' subscriptions are created.

  2. As for the bw verb, it already uses 'raw' subscriptions and there is no case where desrialization of topics is required. Hence no changes made there.

Manual Testing

  1. Run an example publisher:
$ ros2 run examples_rclcpp_minimal_publisher publisher_member_function
[INFO] [1744209999.094889429] [minimal_publisher]: Publishing: 'Hello, world! 0'
[INFO] [1744209999.594917828] [minimal_publisher]: Publishing: 'Hello, world! 1'
[INFO] [1744210000.094935314] [minimal_publisher]: Publishing: 'Hello, world! 2'
[INFO] [1744210000.594921529] [minimal_publisher]: Publishing: 'Hello, world! 3'
[INFO] [1744210001.094927741] [minimal_publisher]: Publishing: 'Hello, world! 4'
  1. Run simple topic hz:
$ ros2 topic hz /topic
average rate: 2.002
        min: 0.500s max: 0.500s std dev: 0.00000s window: 1
average rate: 2.000
        min: 0.499s max: 0.501s std dev: 0.00105s window: 3
average rate: 2.000
        min: 0.499s max: 0.501s std dev: 0.00084s window: 5
  1. Run topic hz with filter that only selects messages with even numbers:
$ ros2 topic hz /topic --filter 'int(m.data.rpartition(\"! \")[-1]) % 2 == 0'
average rate: 1.000
        min: 1.000s max: 1.000s std dev: 0.00000s window: 1
average rate: 1.000
        min: 0.999s max: 1.000s std dev: 0.00024s window: 2
average rate: 1.000
        min: 0.999s max: 1.001s std dev: 0.00065s window: 3
  1. Verifying the message deserialization directly with:
$ ros2 topic hz /topic --filter 'print(type(m.data))'
<class 'str'>
<class 'str'>
<class 'str'>

…ter` argument is provided.

Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with green CI.

@fujitatomoya
Copy link
Collaborator

@exMachina316 thank you very much for taking care of this 👍

@fujitatomoya
Copy link
Collaborator

Pulls: #1005
Gist: https://gist.githubusercontent.com/fujitatomoya/a2ae64630c17539709bfd70cb33aa03f/raw/1b8427275e2a5d4f7e879db78453fa17425e10e8/ros2.repos
BUILD args: --packages-above-and-dependencies ros2topic
TEST args: --packages-above ros2topic
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15639

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

@fujitatomoya
Copy link
Collaborator

IMO, this enhancement would be nice to backport to humble and jazzy.

@christophebedard
Copy link
Member

christophebedard commented Apr 9, 2025

I wonder how this affects/improves performance, especially for #843.

Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
@christophebedard
Copy link
Member

I retriggered the same ci_launcher job:

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

@christophebedard
Copy link
Member

@ros-pull-request-builder retest this please

@christophebedard christophebedard merged commit bfc5245 into ros2:rolling Apr 9, 2025
2 of 3 checks passed
@christophebedard
Copy link
Member

@Mergifyio backport jazzy humble

@mergify
Copy link

mergify bot commented Apr 9, 2025

backport jazzy humble

✅ Backports have been created

Details

mergify bot pushed a commit that referenced this pull request Apr 9, 2025
Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
(cherry picked from commit bfc5245)

# Conflicts:
#	ros2topic/ros2topic/verb/hz.py
mergify bot pushed a commit that referenced this pull request Apr 9, 2025
Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
(cherry picked from commit bfc5245)

# Conflicts:
#	ros2topic/ros2topic/verb/hz.py
christophebedard added a commit that referenced this pull request Apr 9, 2025
…1005) (#1006)

Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
(cherry picked from commit bfc5245)

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Co-authored-by: Kostubh Khandelwal <123073764+exMachina316@users.noreply.github.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
christophebedard added a commit that referenced this pull request Apr 9, 2025
…1005) (#1007)

Signed-off-by: Kostubh Khandelwal <khandelwalkostubh@gmail.com>
(cherry picked from commit bfc5245)

Signed-off-by: Christophe Bedard <bedard.christophe@gmail.com>
Co-authored-by: Kostubh Khandelwal <123073764+exMachina316@users.noreply.github.com>
Co-authored-by: Christophe Bedard <bedard.christophe@gmail.com>
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.

No need to deserialize the message via hz and bw unless necessary

3 participants