search

magazino / move_base_flex

Move Base Flex: a backwards-compatible replacement for move_base
https://uos.github.io/mbf_docs/
BSD 3-Clause "New" or "Revised" License
422 stars 154 forks source link

add rosparam to accept node name change #333

Closed knorth55 closed 7 months ago

knorth55 commented 7 months ago

this PR accept to change the node name for legacy relay. this PR is useful when you want to keep the node name as move_base, because some rviz assumes the move_base node name. I use as below and I don't need to change rviz config which I used for ros-planning/navigation.

corot commented 7 months ago

Thanks @knorth55 for the contribution, but these kind of things are much better handled with remappings Using params to configure topic names is considered an anti-pattern and so strongly discouraged Moreover, changing RViz config is way easier, as it's all handled internally by RViz, and so much harder to make mistakes

knorth55 commented 7 months ago

i strongly think it is not a good idea to configure rviz again, because we want to have two different rviz configure for move_base and move_base_flex. at least, i dont want to change plenty of rviz configs we already use.

remapping for simple action server is troublesome so i use rosparam, but it is an anti pattern. you are right.

corot commented 7 months ago

remapping for simple action server is troublesome so i use rosparam, but it is an anti pattern. you are right.

not 100% sure, but I think it's possible to remap actions :thinking:

knorth55 commented 7 months ago

yes im sure i can. but remapping goal, feedback, status, cancel result is kind of troublesome to me. anyway this PR is trivial, so it is ok not to be merged.

corot commented 7 months ago

yes im sure i can. but remapping goal, feedback, status, cancel result is kind of troublesome to me. anyway this PR is trivial, so it is ok not to be merged.

no no,,, I mean, remap the action name, not the individual topics (ie move_base --> navigate)

knorth55 commented 7 months ago

Oh really, can I do that? In that case, I don't need this PR.