Remove `prefix_is_for_params` from PID_Ros

[bmagyar]

https://github.com/ros-controls/control_toolbox/pull/129#pullrequestreview-1371661251

[christophfroehlich]

@bmagyar why to remove that, and what should be the default behavior?

[github-actions[bot]]

This issue is being labeled as stale because it has been open 45 days with no activity. It will be automatically closed after another 45 days without follow-ups.

[github-actions[bot]]

This issue was closed because it has been stalled for 45 days with no activity.

[christophfroehlich]

quoting @bmagyar

yes, I think the idea was to move away from that parameter and maybe even entirely disable that functionality or have one default and not allow different ones

[christophfroehlich]

proposal together with @saikishor

• new two constructors with arguments
  • param_prefix -> no topic publisher per default
  • param_prefix, topic_namespace_prefix -> topic publisher is on per default
• no string manipulation for both prefixes -> user has to be explicit
• topic uses the namespace of the node (i.e, "~/" + topic_namespace_prefix)
• ROS Parameter for opt-in/opt-out the publisher with default from above

[saikishor]

Thank you for summarising

[destogl]

I agree on "no string manipulation" as this is the issue here. We were removing "/" from parameters and then the interface names of the following controllers that habe prefix with "/" in it were invalidated.

If we use the local topic names, it means with the node prefix "~/", do we require parameter prefix? I am not sure if we need this for embedding PID to other controllers

[ViktorCVS]

@christophfroehlich Since this issue has been resolved, do you think it’s a good time to make those changes? https://github.com/ros-controls/ros2_controllers/pull/1585#issuecomment-2985693108

[christophfroehlich]

yess, please!