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

Sudden stop for critical issues, filtered deceleration otherwise #1468

Merged
merged 6 commits into from
Aug 7, 2019
Merged

Sudden stop for critical issues, filtered deceleration otherwise #1468

merged 6 commits into from
Aug 7, 2019

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented May 16, 2019

Description

A suddenHalt() function for critical issues like joint limits. Otherwise, decelerations are low-pass filtered.

Checklist

@AndyZe
Copy link
Member Author

AndyZe commented May 16, 2019

@jschleicher give it a review, if you please

@jschleicher
Copy link
Contributor

jschleicher commented May 20, 2019

What about the check in the main loop, if a zero velocity command is received? https://github.com/ros-planning/moveit/blob/7f142291ccd8c7f203a4427e6353e3a950d09314/moveit_experimental/jog_arm/src/jog_arm/jog_calcs.cpp#L135

Recording a bag file shows several unexpected behaviours:

  1. while filtering looks good for negative joint velocities, it is still hard-reset to zero for positive velocities:
    jog_arm-screenshot-2019-05-20 16-17-16

  2. during stop sometimes there is an overshoot with movement into the opposite direction:
    jog_arm-screenshot-2019-05-20 16-19-33

@AndyZe
Copy link
Member Author

AndyZe commented May 20, 2019

Good catch on resetVelocityFilters() being out-of-place.

After moving it to a better spot, I'm not seeing anything unexpected from the filters. There is a tiny bit of overshoot at low_pass_filter_coefficient == 1. You can damp that down with a higher low_pass_filter_coefficient, if you want (compare 4th plot to 3rd plot).

I just discovered PlotJuggler for capturing this data easily. It's fantastic.
position0_c_10
position1_c_10
velocity0_c_1
velocity_c_10

@AndyZe
Copy link
Member Author

AndyZe commented May 20, 2019

The filter type used here is known to overshoot. It could be simplified to a first-order filter that apparently doesn't have overshoot.

https://dankelley.github.io/r/2014/01/15/butterworth-filters.html

@SansoneG
Copy link
Contributor

Maybe a new ros parameter could be used in the config.yaml to select the order of the butterworth filter that should be used.

@AndyZe
Copy link
Member Author

AndyZe commented May 21, 2019

I'll make an issue for that.

@jschleicher
Copy link
Contributor

Just a pedantic comment: If the incoming command_timeout is lower than the time to reach zero velocity, the robot keeps moving now:
stop_missing

But a comment to increase the incoming_command_timeout for very high low_pass_filter_coeff (we have tested with low_pass_filter_coeff=30) in the documentation would be fine for me.

@AndyZe
Copy link
Member Author

AndyZe commented May 24, 2019

If I understand right, the last velocity command is sometimes non-zero if incoming_command_timeout is really short.

Here's a fix for that. (I did test it)

@jschleicher
Copy link
Contributor

If I understand right, the last velocity command is sometimes non-zero if incoming_command_timeout is really short.

Exactly. Your fix works for me.

@AndyZe
Copy link
Member Author

AndyZe commented Jun 26, 2019

@henningkayser @BryceStevenWilley @felixvd Can I get a review please? Not sure where my 'Assign Reviewers' button went

Copy link
Contributor

@BryceStevenWilley BryceStevenWilley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though this isn't my expertise.

@BryceStevenWilley BryceStevenWilley added the awaits 2nd review one maintainer approved this request label Jul 8, 2019
@jschleicher
Copy link
Contributor

@henningkayser @felixvd Ping

I'd appreciate, if this could be merged.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving formally, assuming that functionality was approved by @jschleicher.

@rhaschke rhaschke merged commit 5199813 into moveit:master Aug 7, 2019
@tylerjw tylerjw mentioned this pull request May 8, 2020
20 tasks
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 12, 2020
…eit#1468)

Use special suddenHalt() function to stop at critical issues.
Otherwise, e.g. if no new commands are received, use filtered deceleration.
tylerjw pushed a commit to PickNikRobotics/moveit that referenced this pull request May 20, 2020
…eit#1468)

Use special suddenHalt() function to stop at critical issues.
Otherwise, e.g. if no new commands are received, use filtered deceleration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaits 2nd review one maintainer approved this request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants