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

Dependency on JSC and JTC missing? #103

Open
gavanderhoorn opened this issue Jan 25, 2018 · 7 comments
Open

Dependency on JSC and JTC missing? #103

gavanderhoorn opened this issue Jan 25, 2018 · 7 comments
Labels

Comments

@gavanderhoorn
Copy link
Member

As confirmed in #101 (comment), none of the packages in kuka_experimental have a dependency on joint_state_controller or joint_trajectory_controller.

This can lead to users - with a minimal ROS install (ie: no ros_control packages installed yet - running into "Could not load controller X" errors while trying to start the launch file(s) mentioned in the readmes.

It's unclear whether - and where - these dependencies should be added: the hw interface can be used without those two controllers and, apart from the launch and config files in test (which should probably not be there), kuka_rsi_hw_interface does not use those packages itself.

Only if users of kuka_rsi_hw_interface configure their systems to load those controllers is a dependency created.

@gavanderhoorn
Copy link
Member Author

Technically, the responsibility to bring in those packages falls onto the users of kuka_rsi_hw_interface.

Realistically though, most users will want to use joint_state_controller and joint_trajectory_controller with just about every configuration of the hw interface, so we could add those as dependencies to the hw interface.

Alternatively we could consider adding launch and config files to the robot support packages that provide an initial setup and tie the hw interface, ros_control and kuka_rsi_hw_interface together. Then the support packages would declare the dependency.

We do run into point 1 of ros-industrial/ros_industrial_issues#49 then though: RSI is not the only way to interface with these robots, and the packages can be used without any driver whatsoever as well, which would make hardcoding the dependency a bit unfortunate.

@BrettHemes
Copy link
Member

I still really like your suggestions in ros-industrial/ros_industrial_issues#49 and therefore would vote against putting the dependencies in the support packages. I am also leaning towards them not being included in kuka_rsi_hw_interface. You commented on the test launch files maybe not belonging? I think fixing/removing test however you might have in mind and/or (my favorite) working on improving the documentation all around. If/when we split out kuka_rsi_hw_interface as a "driver" it should have solid documentation anyway.

@gavanderhoorn
Copy link
Member Author

gavanderhoorn commented Jan 25, 2018

We could introduce a separate kuka_rsi_hw_interface_test package that contains a minimal 6-axis urdf (no meshes, just kinematic structure, so generic for all robots) and a launch file that configures enough parameters for the hw interface to work (controller_joint_names and the ros_control parameters). The hw iface itself doesn't care about the specific variant, and neither does RSI, nor the rqt_joint_trajectory_controller that is typically used to test. This would make testing a new RSI installation much easier and the readmes could refer to that package as well.

That package should have dependencies on JSC and JTC.

As to the rest: I also don't want to add the dependencies to the support packages, but I only see two other options:

  1. documentation (ie: tell people to install them manually)
  2. introduce additional packages that depend on both 'a' driver and (in the case the driver is kuka_rsi_hw_interface) the JSC and JTC

Ad 1: we can document all we want, but in my experience we won't avoid users not following instructions and posting support requests about 'crashing' launch files because of missing packages.

Ad 2: these could be packages like: kuka_kr6_rsi_hw_iface(_bringup). Not too happy about the name, but you get the idea. That would mean very small pkgs, with just a bit of configuration and some re-usable launch files, that provide everything needed to use kuka_kr6_support together with kuka_rsi_hw_interface (ie: RSI).

With the support pkgs we currently have, that would mean 6 additional packages. That is assuming fully specialised convenience launch files which are specifically created for one specific variant.

Alternatively we could parameterise just about everything and create a single kuka_rsi_hw_iface_support package (or something) which does the same, but needs a series and variant parameter (ie: kr6 and kr6r900sixx respectively), and would then load the proper xacros and other parameters/resources.

@BrettHemes
Copy link
Member

👍 to a kuka_rsi_iface_test

So to clarify, assuming we add the rsi test package with ros-controller dependencies there, the remaining issue then is how to handle the adoption of ros-industrial/ros_industrial_issues#49 and how to manage different driver-robot combinations?

@gavanderhoorn
Copy link
Member Author

I'm not too worried about ros-industrial/ros_industrial_issues#49, I've split up a nr of repositories already. Using git-filter-branch it's relatively trivial.

the remaining issue then is [..] how to manage different driver-robot combinations?

well .. that was the reason I opened this issue ;)

I currently have a slight preference for option 2, but with the 'single package with series & variant launch file parameter' approach.

That would mean one additional package, which allows to use the xacros from the robot support packages with the RSI hw iface. And that pkg should then have the dependencies (I never intended for the new test pkg to have the responsibility of bringing those in: users could opt not to install it, and we'd be back where we started).

@BrettHemes
Copy link
Member

Gotcha!

I currently have a slight preference for option 2, but with the 'single package with series & variant launch file parameter' approach.

Agreed 👍 This is my favorite as well.

@gavanderhoorn
Copy link
Member Author

I've worked on this a bit, and so far have concluded (for myself) that a separate test package doesn't make much sense:

  • the launch and yaml files would be almost identical to those of the 'option 2' pkg
  • loading a 'default, simple robot' doesn't make much sense: there is no way to know what limits would be safe / dummy proof, so might as well put the burden of selecting / procuring / creating the correct xacro on the user

Without a test pkg, but with an 'option 2' pkg, testing the rsi_hw_iface (as is done in the readme) would come down to loading the correct urdf and starting the main launch file from the 'option 2' pkg. That doesn't seem like a lot of work to me.

The 'option 2' pkg is worth looking into more I feel, but I'm running into some things that need decisions and I can't really make up my mind.

I'd like us to be able to release the 'option 2' pkg, which means that we have to come up with some sort of reusable infrastructure (ie: launch and config files) that don't need editing to be useful, at least in the common case / initially (editing will be impossible, as they will be in non world-writable locations).

My goal would be to make the introduced infrastructure as re-usable as possible, unless you have a really complex setup. A standard, 6-axes robot should be able to launch an instance of the hw iface with a single command, only providing the bare minimum of information (ip address, port, urdf).

If we make the user responsible for loading a urdf, a base launch file for rsi_hw_iface needs to:

  1. configure the rsi/listen_address and rsi/listen_port parameters
  2. set ros_control parameters (JSC and JTC, joints to control)
  3. start the rsi_hw_iface node
  4. spawn the controllers

For 1 and 2, we could either put everything in (a) yaml file(s) or use rosparam directly and embed in the launch file.

Launch files with arguments are parameterisable, which increases their reusability, especially when they are read-only. Having everything in the launch file (rosparam) also makes it easier to keep things consistent and not have various settings distributed across multiple files (controller and joint names fi). However: you always include an entire launch file, so unless the rosparam parts are in a separate file, reuse of the launch file in bigger setups could be as cumbersome as with the all-config-in-a-yaml-file option below.

Yaml files are static, so not reusable (unless you happen to need the exact same content). A user with a different setup will need to copy the file to some writable location, update it and then somehow get the base launch file to load the copied file, not the original (could use an arg for the yaml here). Doing everything in the yaml file (so ad 1 and 2) is possible, so that would keep all settings together, but the user would be responsible for keeping the controller names in the yaml and those passed to the controller_manager/spawner in sync (one more potential pof).


I've prototyped the "everything in a single launch file"-approach before, see kr6r900sixx_moveit_rsi_convenience (ignore the issue with the robot_ip and robot_port). I never merged that as I wasn't convinced it was the best way to do this. I'm still not, but the 'single launch file' I describe above is essentially the same.

With the new setup though, instead of creating something like that for all variants, we could add a single instance to the 'option 2' pkg and add robot_series and robot_variant arguments.

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

No branches or pull requests

2 participants