robotology / gz-sim-yarp-plugins

YARP plugins for Modern Gazebo (gz-sim).
BSD 3-Clause "New" or "Revised" License
8 stars 0 forks source link

Rename robotinterface singleton with clear name #154

Closed xela-95 closed 2 months ago

xela-95 commented 2 months ago

Closes #69

The renaming includes:

traversaro commented 2 months ago

Sorry about the bikeshedding, but I am not sure the gz-sim-yarp-robotinterface name. In particular, I do not like it for two reasons:

A possible suggestion for alternative name is have something that describes that the class is doing, that is "saving" the device pointer, so we could call it DeviceRegistry or similar. In that case the renaming can be:

Feel free to do the decision you prefer, stick to the name you chosen, use my suggestion or even a new name scheme if you prefer, thanks!

xela-95 commented 2 months ago

Sorry about the bikeshedding, but I am not sure the gz-sim-yarp-robotinterface name. In particular, I do not like it for two reasons:

* This singleton class is _used_ by the `gz-sim-yarp-robotinterface-system` plugin, but by itself does not contain anything that is particularly specific to the `robotinterface`. Instead, it is a point were we register/store the pointers to all the YARP devices opened by the various gz-sim plugins.

* `gz-sim-yarp-robotinterface` can be easily confused with `gz-sim-yarp-robotinterface-system`.

A possible suggestion for alternative name is have something that describes that the class is doing, that is "saving" the device pointer, so we could call it DeviceRegistry or similar. In that case the renaming can be:

* the class name: `gzyarp::Handler` --> `gzyarp::DeviceRegistry`

* the library name: `gz-sim-yarp-handler` --> `gz-sim-yarp-device-registry`

* the folder containing the library: `singleton-devices` --> `device-registry`

Feel free to do the decision you prefer, stick to the name you chosen, use my suggestion or even a new name scheme if you prefer, thanks!

Thanks @traversaro for the clarification, it makes sense to me.

I was convinced that this library belongs to the responsibilities of the robotinterface plugin (also I was a bit fooled by the name of #69 issue), hence the name I gave to the library (that is however confusing with the gz-sim-yarp-robotinterface-system).

I like your proposal, I will use that convention.

xela-95 commented 2 months ago

Update done. CC @traversaro

xela-95 commented 2 months ago

Merging 🚀