frankaemika / franka_ros

ROS integration for Franka research robots
https://frankaemika.github.io
Apache License 2.0
350 stars 307 forks source link

[feature_request] Change relevant FrankaHWSim members from 'private' to 'protected' #223

Closed rtkg closed 2 years ago

rtkg commented 2 years ago

In our application we run custom Gazebo hardware interfaces for, e.g., adding a F/T sensor to the simulation. Right now, this means to completely re-implement the FrankaHWSim class because relevant members like robot_state_ or arm_id_ are private. Changing the access specifier to protected would allow to derive from FrankaHWSim and only implement the desired additional features.

gollth commented 2 years ago

Hello @rtkg and thanks for the input. Make sense to me. Are we talking only about robot_state_ and arm_id_ or do you require other members as well? Do you need to overwrite methods from the class potentially as well?

rtkg commented 2 years ago

There are others as well, for example I'd need to override the writeSim(.) method for customized simulator interaction. To this end, joints_ and joint_names_ would need to be accessible also. One could also think about interfacing different simulators such as MuJoCo (which is free now and much more stable than Gazebo). Does anything speak against simply changing the current access specifier to protected or do you see members that should be strictly private?

gollth commented 2 years ago

Hi @rtkg

we discussed a bit internally about this topic and came to the conclusion to not make the members protected. Doing this would make the members of the FrankaHWSim and FrankaGripperSim a public interface which we have to maintain. At the current time we cannot guarantee, that these members will keep their signature. Offering them to everybody would restrict future development of franka_gazebo and limit the time we can spend on other features/bug fixes.

I hope you understand. But of course feel free to fork our repo and adjust the code as you need. Best