avoid adding notify waitable twice to events-executor collection

ros2 / rclcpp

rclcpp (ROS Client Library for C++)

Apache License 2.0

514 stars 410 forks source link

avoid adding notify waitable twice to events-executor collection #2564

Closed alsora closed 3 days ago

alsora commented 1 week ago

This PR fixes a bug noticed by @wjwwood as part of his PR here

We don't need to add the notify waitable to the current entities collection list there, because it has already been added in the constructor. The PR includes a unit-test that would detect bugs if the constructor insertion was removed.

alsora commented 1 week ago

CI:

Linux
Linux-aarch64
Linux-rhel
Windows

alsora commented 1 week ago

Linux
Linux-aarch64
Linux-rhel
Windows

alsora commented 1 week ago

Linux-rhel

alsora commented 4 days ago

The linux-rhel failure in test_tracetools.test_tracetools.test.test_generic_pub_sub.TestGenericPubSub.test_all (from pytest) seems unrelated. But triggering another time just in case.

Linux-rhel

EDIT: try again, although this seems very flaky

Linux-rhel

alsora commented 3 days ago

Merging with green CI. The instability of Linux Rhel seems unrelated to the changes here