Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin name proposal #31

Closed
traversaro opened this issue Nov 8, 2013 · 11 comments
Closed

Plugin name proposal #31

traversaro opened this issue Nov 8, 2013 · 11 comments
Assignees

Comments

@traversaro
Copy link
Member

As already mentioned in #25, if we are refactoring the code It could be a good time also to change the name of plugin/device/classes to reflect their actual meaning and increase the readability of the repository for new users.
I expose a proposal that I came up with (inspired mainly from https://github.com/ros-simulation/gazebo_ros_pkgs/tree/hydro-devel/gazebo_plugins), but please make any comment on this.

https://github.com/robotology/gazebo_yarp_plugins/wiki/Proposal-for-names-conventions

(I am not sure about the yarp device driver naming convention, perhaps @lornat75 can provide a more informed opinion).

If we will be ok with this changes and when the times of the changes will come, I can do the dirty work of changing the names and updating all the uses across the repo (Possibly during an italian night).

@MirkoFerrati
Copy link
Contributor

The names...so long!
Why do we need to put Plugin in the end of each class name?
Also, we should remove DeviceDriver and substitute with something shorter...Driver?

@traversaro
Copy link
Member Author

Ok for me, for the DeviceDriver suffix was added in consistency with other yarp devices that I saw in yarp and iCub, but I guess we can go for simply driver.

@traversaro
Copy link
Member Author

@EnricoMingo what do you think?

@EnricoMingo
Copy link
Member

Long Names are more understandable in my opinion

@traversaro
Copy link
Member Author

Ok, I have updated the proposal with the two options (the only difference is in the Plugin postfix in the Gazebo plugin class names).
Can we reach a consensus between this two proposal @EnricoMingo @MirkoFerrati @alessandrosettimi @arocchi ? (Obviously other observations/criticism/improvements are always welcome).

@alessandrosettimi
Copy link
Contributor

I think short names are better.

We're already writing the code inside a folder with a "plugin" in the name :).

This convention is also used in the gazebo_ros_pkgs: https://github.com/ros-simulation/gazebo_ros_pkgs/tree/hydro-devel/gazebo_plugins/src

@traversaro
Copy link
Member Author

Ok, in the next few days we'll get a couple of users/developers interested in gazebo_yarp_plugins looking at the code and possibly contributing, so for starting improving the user friendliness of the repo this night I can prepare a pull request (for gazebo_yarp_plugins and iit-coman-ros-pkg) with the proposed changes (for now it seems that the class name without Plugin are winning).

@arocchi
Copy link
Contributor

arocchi commented Nov 12, 2013

What about using namespaces?
gazebo::yarp::ForceTorquePlugin
For the case of the Force/Torque sensor, we have both a plugin and a device driver:
GazeboYarpForceTorquePlugin extends gazebo ForceTorquePlugin and sets data inside the device driver GazeboYarpForceTorqueDeviceDriver, which in turn publishes this data by using an AnalogServer

@traversaro
Copy link
Member Author

Personally I don't like to use custom namespaces because we depart from the style of gazebo_ros_pkgs (the closest equivelent already existing) on a side and from the style of existing yarp devices from the other side, but I guess it can be a matter just of personal taste.

@traversaro
Copy link
Member Author

I have committed the modified version to:

If you give them a quicktest then we can merge them.

@traversaro
Copy link
Member Author

commited new naming scheme as proposed in hangout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants