gz-sim
gz-sim copied to clipboard
Ackermann Steering Plugin: separate linear and angular
🎉 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:
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
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?
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.
Removing beta
since there hasn't been any activity in a while. We can merge after Harmonic is released.
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?
Can we rescope this PR to this for UpdateVelocity:
This already exists for UpdateAngle. aka. steering only.
In simulation, our robot is turning quite slowly, so we copied over the gain to help.
Can we rescope this PR to this for UpdateVelocity:
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.
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, 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.