Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add collision detector documentation #451

Merged
merged 11 commits into from Aug 28, 2023

Conversation

tonynajjar
Copy link
Contributor

@tonynajjar tonynajjar commented Aug 3, 2023

@tonynajjar tonynajjar changed the title Add collision detector docu Add collision detector documentation Aug 3, 2023
@tonynajjar
Copy link
Contributor Author

Done I believe. Sorry if it's not as verbose as you would wish but I really need to move on with this contribution; it took much more time than expected

@SteveMacenski
Copy link
Member

otherwise LGTM in general, but I'll let @AlexeyMerzlyakov review as well when he's back from vacation

Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

I'll let Alexey approve the content of the subpages with the changes, but LGTM structurally

configuration/packages/configuring-collision-monitor.rst Outdated Show resolved Hide resolved
configuration/packages/configuring-collision-monitor.rst Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

One last thing from me - add an entry in the migration guide page for the new node to show off to the users!

@tonynajjar
Copy link
Contributor Author

One last thing from me - add an entry in the migration guide page for the new node to show off to the users!

Added for Jazzy and Iron, so now we have to backport to Iron 😄

migration/Humble.rst Outdated Show resolved Hide resolved
migration/Iron.rst Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

Just @AlexeyMerzlyakov 's comments left but I'm happy when he is

Copy link
Collaborator

@AlexeyMerzlyakov AlexeyMerzlyakov left a comment

Choose a reason for hiding this comment

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

One nitpick comment and it is ready to-go

******************************************************

In this `PR #3693 <https://github.com/ros-planning/navigation2/pull/3500>`_ A new node was introduced in the nav2_collision_monitor: Collision Detector.
It works similarly to the Collision Monitor, but does not affect the robot's velocity. It will only inform that data from the configured sources has been detected within the configured polygons via message to the ``collision_detector_state`` topic.
Copy link
Collaborator

Choose a reason for hiding this comment

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

... that might be used by any external module (e.g. switching LED or sound alarm in case of collision).

******************************************************

In this `PR #3693 <https://github.com/ros-planning/navigation2/pull/3500>`_ A new node was introduced in the nav2_collision_monitor: Collision Detector.
It works similarly to the Collision Monitor, but does not affect the robot's velocity. It will only inform that data from the configured sources has been detected within the configured polygons via message to the ``collision_detector_state`` topic.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
It works similarly to the Collision Monitor, but does not affect the robot's velocity. It will only inform that data from the configured sources has been detected within the configured polygons via message to the ``collision_detector_state`` topic.
It works similarly to the Collision Monitor, but does not affect the robot's velocity. It will only inform that data from the configured sources has been detected within the configured polygons via message to the ``collision_detector_state`` topic that might be used by any external module (e.g. switching LED or sound alarm in case of collision).

@SteveMacenski SteveMacenski merged commit fa2cae6 into ros-navigation:master Aug 28, 2023
1 check passed
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.

None yet

3 participants