Skip to content

Conversation

@kaichie
Copy link
Contributor

@kaichie kaichie commented Jan 20, 2024

@SteveMacenski
Copy link
Member

I think also this comment wasn't documented:

Perhaps this is just a documentation item that users should set specific over general polygons in the list since the first that applies is used (so backups at the end)

Missing docs that the first polygon, if many cover a range, is used first. That should be addressed in the tutorial, configuration guide, and readme

Signed-off-by: nelson <[email protected]>
@SteveMacenski
Copy link
Member

SteveMacenski commented Jan 29, 2024


VelocityPolygon parameters
==========================

Copy link
Member

Choose a reason for hiding this comment

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

Add a line here that all previous parameters apply, in addition to the following unique parameters for velocity polygons

Add VelocityPolygon in Collision Monitor
****************************************

`PR #3708 <https://github.com/ros-planning/navigation2/pull/3708>`_ adds ``VelocityPolgon`` type in Collision Monitor. This allows the user to setup multiple polygons to cover the range of the robot's velocity limits. For example, the user can configure different polygons for rotation, moving forward, or moving backward. The Collision Monitor will check the robot's velocity against each sub polygon to determine the approriate polygon to be used for collision checking.
Copy link
Member

Choose a reason for hiding this comment

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

Add :ref: to the tutorial update on it

@kaichie
Copy link
Contributor Author

kaichie commented Jan 31, 2024

@kaichie look at the rendering: there's something wrong with the fformatting or your new section https://output.circle-artifacts.com/output/job/2288c871-ef24-4297-8327-cddd9214fe45/artifacts/0/html/configuration/packages/collision_monitor/configuring-collision-monitor-node.html

I could not see anything obvious that is wrong with the table formatting. Looks like it is affected by the length of the parameter name on the left. Not familiar with this but will continue looking later, appreciate any tips here.

@SteveMacenski
Copy link
Member

I have no particular suggestion. given that this is already a problem, I don't expect you to fix it (though that would be ideal), I would however expect you to make it less bad. It seems like reducing the <velocity_polygon_name>.<sub_polygon_name> length would do alot of good. Perhaps <vel_poly>.<subpoly>

If that's not quite enough, seeing what formatting is not properly IDing these tables max width

@kaichie
Copy link
Contributor Author

kaichie commented Jan 31, 2024

Attempted to fix it in this commit, looks okay on the collision monitor page, but causes the table on other pages to look a bit funny, for example, here. Reverted and updated the parameter length to <vel_poly>.<subpoly>.

@SteveMacenski
Copy link
Member

OK, works for me for now

@SteveMacenski SteveMacenski merged commit 402905e into ros-navigation:master Feb 5, 2024
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