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

Implement realtime Ruckig jerk-limited smoothing #2956

Merged
merged 11 commits into from
Aug 15, 2024

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Aug 7, 2024

Description

Implement jerk-limited smoothing in Servo. This should enable large industrial robots to be controlled in a realtime fashion.

Checklist

  • Extend the tutorials / documentation -- I'd like to make a new tutorial about these smoothing plugins since there are 3 of them now.

Test Instructions

Note this change in panda_simulated_config.yaml: smoothing_filter_plugin_name: "online_signal_smoothing::RuckigFilterPlugin"

That's the only change the user needs to make. Then run one of the Servo demos like normal, e.g.

ros2 launch moveit_servo demo_ros_api.launch.py

ros2 run moveit_servo servo_keyboard_input

@AndyZe AndyZe marked this pull request as draft August 7, 2024 22:00
@AndyZe AndyZe force-pushed the ruckig_smoothing branch from dd29b5f to aee05c7 Compare August 7, 2024 22:05
@AndyZe AndyZe marked this pull request as ready for review August 7, 2024 22:06
@AndyZe AndyZe force-pushed the ruckig_smoothing branch from aee05c7 to c8a26b6 Compare August 7, 2024 22:08
@AndyZe AndyZe requested review from pac48 and sjahr August 7, 2024 22:12
@AndyZe
Copy link
Member Author

AndyZe commented Aug 7, 2024

@ibrahiminfinite

@AndyZe AndyZe force-pushed the ruckig_smoothing branch from c8a26b6 to 5c50516 Compare August 7, 2024 22:18
@sea-bass
Copy link
Contributor

Thanks for trying out the optional jerk limits! Hopefully the issue you raised there will go somewhere in future.

There seems to be a failing clang-tidy check. Once that is clear (and if you're ready), request review and I'll give it another look.

@AndyZe AndyZe force-pushed the ruckig_smoothing branch 5 times, most recently from da91bd3 to 2278b2c Compare August 11, 2024 18:57
@AndyZe AndyZe requested a review from sea-bass August 11, 2024 19:17
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Requesting changes because I think those CI / clang-format changes should not be part of this PR.


Also, I see this PR has components of #2954 -- do you want to merge that in first, or is the plan to combine both of these into this PR?

Either way, my comment on that other PR stands, whether the changes go in here or there first. I'd like to see validation that the fixes in the smoother logic don't break the process of switching between free-space motion planning and servo, without sudden jumps due to old smoother state.


Finally, if you do plan to switch the default panda_simulated_config.yaml to use the Ruckig plugin, all the launch files in the demos and integration tests need to update their parameter definitions (or maybe not, if they have sensible defaults)

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
.github/workflows/format.yaml Outdated Show resolved Hide resolved
@AndyZe AndyZe force-pushed the ruckig_smoothing branch 2 times, most recently from 6bc8dbf to e69636d Compare August 12, 2024 14:04
@AndyZe AndyZe requested a review from sea-bass August 14, 2024 17:03
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Just one tiny line to fix, LGTM

.github/workflows/format.yaml Outdated Show resolved Hide resolved
@AndyZe
Copy link
Member Author

AndyZe commented Aug 14, 2024

We both weren't really satisfied with the bit of code below so I tried 4 different ways to handle it. There is some info on how Ruckig handles this tracking scenario here: https://github.com/pantor/ruckig?tab=readme-ov-file#tracking-interface

  1. My first implementation: Assume zero vel/accel at target state. Seems very smooth but the direction of motion of the robot is not completely correct. (It should be vertical.) In short I think this is due to longer intervals before the target state would be reached. In the given control period (0.01s) the target state wasn't reached so the direction of motion changed.
  // Update Ruckig target state
  // This assumes stationary at the target (zero vel, zero accel)
  ruckig_input_->target_position = std::vector<double>(positions.data(), positions.data() + positions.size());
  // target velocities and accelerations are zero
stationary_at_target.webm
  1. The Ruckig way, assume the current acceleration continues through the next target state. Seemed jerky. This method doesn't make sense to me.
ruckig_way_maintain_current_accel.webm
  1. This seemed the best. Smooth and the direction of motion was good.
  ruckig_input_->target_position = std::vector<double>(positions.data(), positions.data() + positions.size());
  // We don't know what the next command will be. Assume velocity continues forward based on current state,
  // target_acceleration is zero.
  const size_t num_joints = ruckig_input_->current_acceleration.size();
  for (size_t i = 0; i < num_joints; ++i)
  {
    ruckig_input_->target_velocity.at(i) =
        ruckig_input_->current_velocity.at(i) + ruckig_input_->current_acceleration.at(i) * params_.update_period;
  }
  // target_acceleration remains a vector of zeroes
zero_accel_target.webm
  1. This was noticeably jerky and got stuck at singularities (overshot the target position, I assume)
  ruckig_input_->target_position = std::vector<double>(positions.data(), positions.data() + positions.size());
  // We don't know what the next command will be, so assume acceleration tends to zero
  const size_t num_joints = ruckig_input_->current_acceleration.size();
  for (size_t i = 0; i < num_joints; ++i)
  {
    ruckig_input_->target_velocity.at(i) =
        ruckig_input_->current_velocity.at(i) + ruckig_input_->current_acceleration.at(i) * params_.update_period;
    if (ruckig_input_->current_acceleration.at(i) > 0)
    {
      // Positive acceleration, takes a step toward zero
      ruckig_input_->target_acceleration.at(i) =
          ruckig_input_->current_acceleration.at(i) - ruckig_input_->max_jerk.at(i) * params_.update_period;
    }
    else
    {
      // Negative acceleration, takes a step toward zero
      ruckig_input_->target_acceleration.at(i) =
          ruckig_input_->current_acceleration.at(i) + ruckig_input_->max_jerk.at(i) * params_.update_period;
    }
  }
step_toward_zero_accel_target.webm

So i'm going with number 3

@AndyZe AndyZe enabled auto-merge August 14, 2024 18:27
@AndyZe AndyZe added this pull request to the merge queue Aug 14, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 14, 2024
@sea-bass sea-bass added this pull request to the merge queue Aug 15, 2024
Merged via the queue into moveit:main with commit eecacae Aug 15, 2024
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants