Skip to content

Conversation

@reinzor
Copy link
Contributor

@reinzor reinzor commented Sep 24, 2024

Backport of #4675

@mergify
Copy link
Contributor

mergify bot commented Sep 24, 2024

@reinzor, all pull requests must be targeted towards the main development branch.
Once merged into main, it is possible to backport to @jazzy, but it must be in main
to have these changes reflected into new distributions.

@reinzor reinzor changed the title feat(controller-server): publish_zero_velocity parameter (#4675) feat(controller-server): publish_zero_velocity parameter jazzy backport Sep 24, 2024
…ation#4675)

Backport of ros-navigation#4675

Signed-off-by: Rein Appeldoorn <rein.appeldoorn@nobleo.nl>
@reinzor reinzor force-pushed the backport/publish-zero-velocity-jazzy branch from f633103 to f72b994 Compare September 24, 2024 12:41
velocity.header.frame_id = costmap_ros_->getBaseFrameID();
velocity.header.stamp = now();
publishVelocity(velocity);
if (get_parameter("publish_zero_velocity").as_bool()) {
Copy link
Member

Choose a reason for hiding this comment

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

You can store the variable in the class, that's OK

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://www.ros.org/reps/rep-0009.html:

You cannot...
  Add new non-static data members, even if they are protected or private

Also the get_parameter is a map lookup so should be fast.

Copy link
Member

Choose a reason for hiding this comment

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

I suppose I haven't enforced that part of ABI, but perhaps I should be -- it would just make direct backporting any new feature virtually impossible since they would likely have some parameter set tied to them. I enforce API stability and behavior stability (i.e. not changing defaults or fundamentally changing the behavior on a module mid-distribution), but I think there's an argument that I should by enforcing stricter ABI as well. Though, its never come up and no one's had an issue.

This is fine.

Co-authored-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: Rein Appeldoorn <reinzor@gmail.com>
@SteveMacenski SteveMacenski merged commit ef1330f into ros-navigation:jazzy Sep 25, 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