search

micro-ROS / system_modes

System modes for ROS 2 and micro-ROS
Apache License 2.0
43 stars 9 forks source link

Redundant service call parameter "node_name" for system_modes services #24

Closed chcorbato closed 4 years ago

chcorbato commented 4 years ago

Currently, system_modes/srv/GetAvailableModes, system_modes/srv/ChangeMode and system_modes/srv/GetMode require as parameter the node_name. However, the similar services from lifecycle_msgs/srv/* do not require to pass that parameter, since the service name/scope already includes it. For example:

ros2 service call /actuation/get_available_modes system_modes/srv/GetAvailableModes "{node_name: actuation}"}"

Maybe that service parameter could be optional, also for system_modes to have a more similar interface to lifecycle?

norro commented 4 years ago

Thanks, @chcorbato, this is a very valuable observation. This redundancy was indeed also present in the according lifecycle_msgs/srv/ when we first started this project (before pushing it to github), but was since removed: https://github.com/ros2/rcl_interfaces/pull/39 (October 2018).

So you are right, this should be removed from the system mode services as well.

norro commented 4 years ago

@chcorbato please have a look at PR https://github.com/micro-ROS/system_modes/pull/31

marioney commented 3 years ago

Hi @norro

This issue is still present in the /feature/rules branch, I'm not sure if a new issue is needed, but It'll be good to apply the solution to that branch as well.

norro commented 3 years ago

Very good point, I will port it there as well.

norro commented 3 years ago

@marioney should be ported now. If it causes any issues, please don't hesitate to open further bug reports.