gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Ackermann Steering Plugin: separate linear and angular

Open Bac0nEater opened this issue 1 year ago • 4 comments

🎉 New feature

Summary

This separate fields between linear and angular in Ackermann Steering Plugin, such as linear, acceleration and jerk.

Linear velocity, acceleration and jerk fields stay the same. However, we moved the angular velocity, acceleration and jerk fields, so we can specify the velocity, acceleration and jerk differently for linear and angular.

Explanation: Screenshot from 2023-06-15 14-57-15

We also added P control to normal mode (not SteeringOnly mode).

The plugin and integration tests are written based on the original velocity and acceleration of Ackermann Steering.

Test it

The plugin can be tested using examples/worlds/ackermann_steering_with_angular.sdf

Checklist

  • [x] Signed all commits for DCO
  • [x] Added tests
  • [x] Added example and/or tutorial
  • [x] Updated documentation (as needed)
  • [ ] Updated migration guide (as needed)
  • [ ] Consider updating Python bindings (if the library has them)
  • [ ] codecheck passed (See contributing)
  • [x] All tests passed (See test coverage)
  • [x] While waiting for a review on your PR, please help review another open pull request to support the maintainers

Bac0nEater avatar Jun 16 '23 00:06 Bac0nEater

As I already mentioned in my previous comment, this might break other users.

a current user of this plugin needs to include these new tags otherwise the min angular vel, max angular vel, min angular acceleration and max angular acceleration will not be defined.

there are two ways to include this:

  • target this to main
  • Add a legacy tag, to be able to choose the current behaviour or use this new one. The default behaviour should be the current behaviour

I'm sorry. I'm not sure what is the legacy tag. Is it the tags and releases in Github?

Bac0nEater avatar Aug 22 '23 00:08 Bac0nEater

As I already mentioned in my previous comment, this might break other users. a current user of this plugin needs to include these new tags otherwise the min angular vel, max angular vel, min angular acceleration and max angular acceleration will not be defined. there are two ways to include this:

  • target this to main
  • Add a legacy tag, to be able to choose the current behaviour or use this new one. The default behaviour should be the current behaviour

I'm sorry. I'm not sure what is the legacy tag. Is it the tags and releases in Github?

what I was trying to say, it;s that you can add a new tag inside the plugin, for example <angular_limits>:

        <angular_limits>true</angular_limits>
        <min_angular_velocity>-10</min_angular_velocity>
        <max_angular_velocity>10</max_angular_velocity>
        <min_angular_acceleration>-20</min_angular_acceleration>
        <max_angular_acceleration>20</max_angular_acceleration>
      </plugin>

if this is true then use the specific angular limits otherwise use the other limits to avoid break other users.

ahcorde avatar Aug 22 '23 08:08 ahcorde

Removing beta since there hasn't been any activity in a while. We can merge after Harmonic is released.

azeey avatar Aug 29 '23 23:08 azeey

Thanks for the contribution. Looking over this I'm not sure if this makes sense to me for an Ackermann based vehicle.

The problem resides in the fact that angular velocity is only a function of the heading/steering angle and the forward drive velocity of the wheels. This would rely upon someone doing the math to know which one is "limiting the system" and does not seem to add any functionality I could think of offhand for a real Ackermann based vehicle, instead this could be potentially much more risky to users.

Also not sure the value in setting min/max angular rates would do for Ackermann as well since it's not a diff drive.

What problem were you hoping to solve or added functionality that is missing were you hoping for this to have?

bperseghetti avatar Dec 27 '23 15:12 bperseghetti

Can we rescope this PR to this for UpdateVelocity: image

This already exists for UpdateAngle. aka. steering only.

In simulation, our robot is turning quite slowly, so we copied over the gain to help.

retinfai avatar Mar 27 '24 03:03 retinfai

Can we rescope this PR to this for UpdateVelocity: image

This already exists for UpdateAngle. aka. steering only.

In simulation, our robot is turning quite slowly, so we copied over the gain to help.

Sure, that seems reasonable, I would just maybe edit title of PR to match and cleanup the feature summary, or you can just open a new PR if you want and close this one referencing that new one. Your choice, but fastest route might be just the new PR route anyhow to avoid needing to handle the stale/blocking reviews on this anyhow.

bperseghetti avatar Mar 27 '24 03:03 bperseghetti

Sweet as, I can't close this from my side, as these guys have left the lab. But safe to say no more work on this thread will be done; so if you can close on your side, perfect.

I'll open a PR

retinfai avatar Mar 27 '24 10:03 retinfai

@retinfai, please make sure to rebase your branch on the latest merge for gz-sim8 (which includes the changes to the ackermann plugin) before creating that PR.

bperseghetti avatar Mar 27 '24 18:03 bperseghetti