use odom gearset/drivescales when available#464
Conversation
use odom gearset/drivescales when available
Codecov Report
@@ Coverage Diff @@
## develop #464 +/- ##
===========================================
+ Coverage 88.72% 88.81% +0.09%
===========================================
Files 140 140
Lines 5878 5880 +2
===========================================
+ Hits 5215 5222 +7
+ Misses 663 658 -5 |
| auto ccGearset = gearset; | ||
| auto ccDriveScales = driveScales; | ||
| if(differentOdomScales) { | ||
| ccDriveScales = odomScales; |
There was a problem hiding this comment.
Can't we just use odomScales in either case?
There was a problem hiding this comment.
Will test whether that works.
| auto ccDriveScales = driveScales; | ||
| if(differentOdomScales) { | ||
| ccDriveScales = odomScales; | ||
| ccGearset = AbstractMotor::gearset::green; |
There was a problem hiding this comment.
Yeah, that may not be necessary. will test
There was a problem hiding this comment.
@Octogonapus - During physical testing I ran into issues with an externally geared chassis which I've also resolved in this PR. Feedback?
Ran into issues testing on a physical robot that has an externally geared drivetrain. This produces correct behavior
| mode = distance; | ||
|
|
||
| const double newTarget = itarget.convert(meter) * scales.straight * gearsetRatioPair.ratio; | ||
| const double newTarget = itarget.convert(meter) * scales.straight; |
There was a problem hiding this comment.
Is this change (and the one below) backwards compatible for people that don't call withDimensions with a GearsetRatioPair?
There was a problem hiding this comment.
Yes, if one does .withDimensions(AbstractMotor::gearset::green, {{4.0_in,10_in}, imev5GreenTPR}) it will create with a gearsetRatioPair with a 1.0 gear ratio defaulted.
There was a problem hiding this comment.
I am not sure where the external ratio is accounted for, then. It seems like it is always discarded.
There was a problem hiding this comment.
@Octogonapus - you are correct. Latest commit should fix this. The chassisControllerPID now works physically with both external tracking wheels and configured to use the drive train as the odom source.
I've phyically tested this with both configurations
Fixed CCPID issue with integrated odom vs tracking wheel
Octogonapus
left a comment
Octogonapus
left a comment
There was a problem hiding this comment.
Thanks for working on this rats nest 👍
3 participants
Description of the Change
When using geared drivetrains and odom, the odom drivescales are not used, leading to inaccurate movements
Motivation
Correct behavior described above
Possible Drawbacks
N/A
Verification Process
Physical test
Applicable Issues
Fixes #462