Implement inconsistent topic. #431
Conversation
or maybe the tests could live in |
There was a problem hiding this comment.
The change looks good to me, but there is the issue that I am not so sure Cyclone will raise "inconsistent topic" based on reader/writer discovery. It should (and will) do it based on topic discovery.
The reason I am not sure is because I do think there's a good argument that a reader/writer pair that fails to match on type name (possibly also assignability, but there other QoS settings also come into play) implies that an "inconsistent topic" notification would have been raised if topic discovery had been enabled, and that it therefore makes sense to raise it at that point if topic discovery is disabled.
I'm approving this as a Cyclone's not raising "inconsistent topic" today even when topic discovery is enabled is simply a bug, and because I believe this code will work fine when that bug is fixed.
clalancette
force-pushed
the
clalancette/inconsistent-topic-event
branch
from
dc1f1c2
to
984065a
Compare
Note that this is not hooked up inside of CycloneDDS, so this will never fire currently. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
clalancette
force-pushed
the
clalancette/inconsistent-topic-event
branch
from
984065a
to
6b04608
Compare
delete-merged-branch
bot
deleted the
clalancette/inconsistent-topic-event
branch
Implement inconsistent topic. (ros2#431)
|
It seems this PR somehow makes I ran 6 builds, building Building rclcpp and launch_testing_ros:
One more time:
Building launch_testing_ros only:
I used the following ros2.repos files: FYI:
|
This reverts commit 76572eb.
Note that this is not hooked up inside of CycloneDDS, so this will never fire currently.
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
Part of ros2/ros2#1361
Depends on ros2/rmw#339