search

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

Remove velocity override code (pushMotion) #1

Closed marshallpowell97 closed 4 years ago

marshallpowell97 commented 4 years ago

https://github.com/ros-industrial/staubli_val3_driver/blob/55b4fe6d3fbc60fa7a89139aa0f195c5d3c62072/staubli_val3_driver/val3/ros_server/pushMotion.pgx#L72-L79

Sets the velocity to the value set on the teach pendant if 'velocity override' is enabled. This does not seem to work as intended since the velocity set on the pendant already "overrides" whatever value is set in the Val3 code. The result is that the "overridden" velocity ends up being much smaller than intended.

I.E. if 50% velocity is set by the pendant, the actual speed of the robot will be 50% (result from getMonitorSpeed() ) multiplied by another 50% (set by the pendant) = 25%

gavanderhoorn commented 4 years ago

I'm not entirely sure what this code was supposed to achieve either.

This was all added in ros-industrial/staubli_experimental#10 and unfortunately the contributor of that is no longer around.

I suggest we remove this, and the associated code in the UI files.

@nilsmelchert @simonschmeisser: have you ever used this and did it work for you as expected?

simonschmeisser commented 4 years ago

Unfortunately we currently don't have access to a Stäubli and I went on summer holidays after getting the thing to run so I haven't collected that much experience but ...

I image the original intention, which might have been achieved or not, was to actually speed up execution of trajectories? At least that would be problem we keep pushing ahead and not addressing. Basically if you set velocity levels and acceleration levels too low and/or your time parametrization is suboptimal you end up with a trajectory containing values smaller than the maximum achievable velocity. Then the controller will interpret those as upper limits, reducing velocity further. Now thinking about it it is however not clear to me why we would not "solve"* this at the MoveIt level ...

gavanderhoorn commented 4 years ago

As I wrote in https://github.com/ros-industrial/staubli_val3_driver/pull/8#discussion_r369798953, I believe it'd be best to remove the override: it reportedly doesn't work as intended.

I'll change the title of this issue to convert it into an action item.

marshallpowell97 commented 4 years ago

I have this removed in a branch here ready to make a pull request once the previous one is merged.

gavanderhoorn commented 4 years ago

It would probably be good to rebase your new branch once #8 gets merged.

gavanderhoorn commented 4 years ago

I've linked this to #9, as it (starts to) remove(s) the velocity override.

gavanderhoorn commented 4 years ago

Seeing issues like #2 (and the attempts to fix this in #8 and then #18), perhaps the rationale behind the velocity override functionality was exactly the problems we're encountering now?