ros-industrial / kuka_experimental

Experimental packages for KUKA manipulators within ROS-Industrial (http://wiki.ros.org/kuka_experimental)
Apache License 2.0
277 stars 218 forks source link

Implement RT priority assignment #257

Open balandbal opened 1 year ago

balandbal commented 1 year ago

The README for the (KRC4) RSI hardware interface recommends setting the PC real-time priority for it if the connection to RSI is sporadically dropped. The hardware interface for Universal Robots arms sets this priority when it can; I thought it sensible to adopt that part of the code into the KUKA interface.

As far as I could determine at a glance, the implementation should not conflict with anything already in the KUKA interface, though my experience with C++ and the source code of the hardware interface is limited.

Once our cell is built completely in the following weeks, I will be using the patch more extensively. If there are any specific tests that would be great to run, I am willing to try and set them up.

simonschmeisser commented 1 year ago

Thanks for your PR! This looks fine now from the formal side. I have no experience with RSI (yet) so I would like to wait for either you or me to test this patch before merging and ideally for someone with real time experience to also review it.

gavanderhoorn commented 1 year ago

I believe, as this was copied from the/a UR driver, that the provenance of this code should be clearly stated, including the license it is covered by, and the original author(s).

IIRC, the code actually comes from the Franka Emika driver, so perhaps the UR driver authors already didn't abide by the license(s) there, but that doesn't mean we can do the same here.

balandbal commented 1 year ago

I believe, as this was copied from the/a UR driver, that the provenance of this code should be clearly stated, including the license it is covered by, and the original author(s).

IIRC, the code actually comes from the Franka Emika driver, so perhaps the UR driver authors already didn't abide by the license(s) there, but that doesn't mean we can do the same here.

I added the license, copyright and author information in c51e1b4.

Adding a notice about the mixed licensing seemed excessive for kuka_hardware_interface.cpp, though it also includes the header with the priority assignment through kuka_hardware_interface.h.

gavanderhoorn commented 1 year ago

thanks for the update.

However, calling a function does not change the copyright on the file that calls it. Neither does including a .h. So the changes to the files that just call the new function or include the header would not be needed (kuka_rsi_hw_interface/include/kuka_rsi_hw_interface/kuka_hardware_interface.h fi).

I'm also not sure whether Felix would have to be added to the manifest: many people have contributed to these packages, and we don't list all of them. Only the main contributors are listed.