stereolabs / zed-ros-wrapper

ROS wrapper for the ZED SDK
https://www.stereolabs.com/docs/ros/
MIT License
447 stars 391 forks source link

[Question] Custom vs Standard service types #624

Open bjsowa opened 4 years ago

bjsowa commented 4 years ago

I noticed that you define a custom service type for each service in the zed_interfaces package. Why not just use a standard service types like std_srvs/SetBool and std_srvs/Trigger for most of the services? This way, other clients in the ROS network won't need to build the zed_interfaces package to, for example, reset the odometry.

I know that changing the ROS API can break some applications that use the current one, but I would like you to at least consider this in the future.

Myzhar commented 4 years ago

Hi @bjsowa can you explain better what you mean and how you'd make such a feature? SetBool and Trigger do not have extended information, how can the node understand what service must be enabled/disabled/triggered?

bjsowa commented 4 years ago

To give you an example:

You define zed_interfaces/reset_odometry type for the reset_odometry service. Instead, you could use the std_srvs/Trigger type and return the bool value in the success variable.

Myzhar commented 4 years ago

Ah ok, that's clear. Yes, you are right. They are defined in the same way. However you cannot avoid using the zed_interfaces package because there are custom messages for the Object Detection module and the ZED2 sensors. But it could be useful to call ZED services from other nodes without adding the zed_interfaces dependency

Myzhar commented 4 years ago

Tested this suggestion writing the new ROS2 wrapper for ROS2 Eloquent (https://github.com/stereolabs/zed-ros2-wrapper/commit/61ff87e4fe8ea634687676f7617e0ada28701b9f , https://github.com/stereolabs/zed-ros2-wrapper/commit/7dd59707a505d0d142edf47a60ac6292dc9ea8a6), it's a good suggestion and it will be replicated here with the new version that will be released with the new ZED SDK