ros-industrial / staubli_val3_driver

ROS-Industrial (simple message) driver for Stäubli CS8 and CS9 robot controllers (VAL 3 application)
Apache License 2.0
26 stars 21 forks source link

Add support for CS9 controller #15

Closed marshallpowell97 closed 4 years ago

marshallpowell97 commented 4 years ago

Separate out the UI code for CS8 and CS9 into libraries. Library is loaded at runtime depending on version of controller (done automatically). Same code base can still be used for both controllers.

Relevant: https://github.com/ros-industrial/staubli_val3_driver/issues/4

gavanderhoorn commented 4 years ago

This is quite a nice PR. Thanks for this @marshallpowell97 :+1:

@gonzalocasas: would you happen to see any possibility to test this? Perhaps in SRS? It's OK if you don't. I only have a (rather) old version of it which doesn't support CS9 controllers.

simonschmeisser commented 4 years ago

I unfortunately don't have access to a Stäubli myself and I think our customer doesn't have one either at the moment. Maybe @marshallpowell97 could motivate someone at Stäubli to update your license for SRS?

gonzalocasas commented 4 years ago

I will test it, I received the SRS license dongle from our office, so, I can now test at home.

gavanderhoorn commented 4 years ago

@gonzalocasas wrote:

I will test it, I received the SRS license dongle from our office, so, I can now test at home.

awesome, thanks @gonzalocasas :+1:

gavanderhoorn commented 4 years ago

@gonzalocasas: I hate to be pushy, but I don't want to keep @marshallpowell97 waiting too long.

Would you happen to have some time next week to test this PR?

gonzalocasas commented 4 years ago

I just tried on my SRS, but despite being CS9, it still complains about trying to load CS8 stuff:

image

@marshallpowell97: do I need to do something additional to get it to ignore these incompatible data types?


EDIT: It seems to be ignoring the autoload=false flag defined here: https://github.com/ros-industrial/staubli_val3_driver/pull/15/files#diff-088b39fc58afb792039b8d0d7c01fb43

gonzalocasas commented 4 years ago

Also, unless I'm looking at the wrong branch, ros_server.dtx still contains the scDebug, and scMonitor variables that are of the incompatible data type, which was the problem in the first place.

gonzalocasas commented 4 years ago

@marshallpowell97 @gavanderhoorn if I remove the two variables leftover screen type variables from ros_server.dtx and I update the path of the library in ros_server.pjx to point to interfaceCS9 then everything else seems to work fine.

gavanderhoorn commented 4 years ago

Thanks for testing @gonzalocasas.

The idea would certainly be for this to all be detected and switched automatically, so what you describe seems like a problem with the current code.

gavanderhoorn commented 4 years ago

Friendly ping @marshallpowell97.

marshallpowell97 commented 4 years ago

ros_server.dtx still contains the scDebug, and scMonitor variables that are of the incompatible data type, which was the problem in the first place.

This was an oversight on my part, I removed them. They are now only in interfaceCS8.dtx where they should be.

I also noticed another issue that didn't show up on a real controller (since I don't have access to any of our robots at the moment). Even when the 'auto load' option for the library libInterface is deselected, it still causes a runtime error when using the emulator in SRS. I added another interface library interfaceDefault which is empty and loads fine on either controller.

gavanderhoorn commented 4 years ago

@gonzalocasas: would you perhaps happen to have some time to test this?

I'd like to merge it, but it would be good to have a second pair of eyes on the functionality @marshallpowell97 has added.

gonzalocasas commented 4 years ago

@gavanderhoorn yes, I had it in my radar already, but haven't had the time so far. But I will test it in the coming days!

gavanderhoorn commented 4 years ago

@gonzalocasas: have you had any chance to test this already?

gavanderhoorn commented 4 years ago

Ping @gonzalocasas ?

gavanderhoorn commented 4 years ago

I'm going to merge this to not delay @marshallpowell97 any longer.

We'll deal with problems with this change in follow-up PRs.