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

Remove velocity override code (pushMotion) #1

Closed
marshallpowell97 opened this issue Jan 10, 2020 · 7 comments · Fixed by #15
Closed

Remove velocity override code (pushMotion) #1

marshallpowell97 opened this issue Jan 10, 2020 · 7 comments · Fixed by #15
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@marshallpowell97
Copy link
Contributor

marshallpowell97 commented Jan 10, 2020

// check whether velocity value should be overwritten with value set on teach pendant
if (bOverwriteVel == true)
// overwrite velocity (and acceleration - ?)
l_mTrajPt.mDesc.vel = getMonitorSpeed()
// explicitly setting accel and decel to VAL3 default values
l_mTrajPt.mDesc.accel = 9999
l_mTrajPt.mDesc.decel = 9999
endIf

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
Copy link
Member

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
Copy link

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 ...

  • or somewhere else external to the driver

@gavanderhoorn gavanderhoorn transferred this issue from ros-industrial/staubli_experimental Jan 16, 2020
@gavanderhoorn
Copy link
Member

As I wrote in #8 (comment), 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.

@gavanderhoorn gavanderhoorn changed the title Velocity Override does not have correct effect Remove velocity override code (pushMotion) Mar 12, 2020
@gavanderhoorn gavanderhoorn added enhancement New feature or request good first issue Good for newcomers labels Mar 12, 2020
@marshallpowell97
Copy link
Contributor Author

marshallpowell97 commented Mar 12, 2020

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

@gavanderhoorn
Copy link
Member

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

@gavanderhoorn gavanderhoorn linked a pull request Mar 30, 2020 that will close this issue
@gavanderhoorn
Copy link
Member

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

@gavanderhoorn
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Development

Successfully merging a pull request may close this issue.

3 participants