Conversation
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## ign-math6 #194 +/- ##
==========================================
Coverage 99.20% 99.21%
==========================================
Files 63 65 +2
Lines 6019 6087 +68
==========================================
+ Hits 5971 6039 +68
Misses 48 48
Continue to review full report at Codecov.
|
Signed-off-by: Louise Poubel <louise@openrobotics.org>
|
@scpeters , I refactored the API, implementation and naming conventions. I think the class is easier to use and the math is more straight forward now. Let me know what you think. |
…robotics/ign-math into chapulina/6/speed_limiter Signed-off-by: Louise Poubel <louise@openrobotics.org>
scpeters
left a comment
There was a problem hiding this comment.
sorry for the delay in getting back to this. thanks for addressing my earlier comments; I have a few more now
src/SpeedLimiter.cc
Outdated
|
|
||
| _v = _v0 + dv; | ||
| } | ||
| const double accDesired = (_vel - _prevVel) / dtSec; |
There was a problem hiding this comment.
I think the code is clearer to read this way, but I believe it may have been written the other way to avoid a division operation and divide by 0 concerns
There was a problem hiding this comment.
Yeah I agree that this may have been the motivation. One lesson I took from the fact that the math had been wrong on the ros_control implementation all this time is the value of readability. I'm leaning towards leaving the math straightforward so it's easier to debug in the future. I added tests for the dt == 0 case, so I think we're covered.
Signed-off-by: Louise Poubel <louise@openrobotics.org>
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸
🎉 New feature
Summary
We've been using the
SpeedLimiterclass ported fromros_controlon Ignition Gazebo's diff drive controller for a while. Now another controller is being added and wants to use the class too (Ackermann on gazebosim/gz-sim#618), so it's good to move it to a common place where it can be reused by various controllers instead of copied. Ignition Math feels like a good place to put this.The class had originally been copied from
ros_controlwithout much testing. While porting it toign-math, I added tests and noticed that the jerk limit calculation was mistaken and is being fixed on ros-controls/ros_controllers#388. I believe the version on this PR should be correct.Some other changes:
chronofor timeTest it
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸