Allow use of non-integer and custom encoders #7
Conversation
|
acmerobotics/road-runner-quickstart#407 adds support for the changes in this PR to the quickstart (but also works without these changes). |
| @@ -9,11 +9,11 @@ import kotlin.math.min | |||
| import kotlin.math.round | |||
|
|
|||
| class PositionVelocityPair( | |||
There was a problem hiding this comment.
I intend for this class to represent encoder measurements in tick units (and really measurements made through a Rev hub given the raw fields). I would rather create new types and plumbing for sensors that don't fit this model.
There was a problem hiding this comment.
Makes sense. It may still be worth having some way of expanding it for other ways of reading, such as Octoquad or the pinpoint's raw encoder passthrough system.
| ) | ||
|
|
||
| sealed interface Encoder { |
There was a problem hiding this comment.
Are the odometry sensors modeled as a different kind of encoder? What alternate implementations are you pondering and why shouldn't they be included in this module?
There was a problem hiding this comment.
Currently in my road-runner-ftc fork I am representing the OTOS position and the Pinpoint raw encoder data as encoder implementations only for tuning. https://github.com/jdhs-ftc/road-runner-ftc-otos/blob/master/RoadRunner/src/main/java/com/acmerobotics/roadrunner/ftc/OTOS.kt and https://github.com/jdhs-ftc/road-runner-ftc-otos/blob/pinpoint/RoadRunner/src/main/java/com/acmerobotics/roadrunner/ftc/Pinpoint.kt (note: for the pinpoint I am currently using the raw odometry encoder passthrough for tuning, not the calculated pose, and the comments reflect that)
I mainly did this because it seemed the easiest way to use them for tuning. I have not included them in this PR because honestly I'm not sure they're up to roadrunner standards for quality and testing, but making this no longer sealed would allow me to much more easily develop and/or distribute them.
For actual following I'm using modifications to updatePoseEstimate to set the field centric pose directly, as previously discussed; that does not use these encoder modifications at all.
| // check if the second to last number in the data is non-integer | ||
| if (xs[-2] !== Math.round(xs[-2])) { | ||
| return numDerivOffline(ts, xs).map((est, i) => { | ||
| return (vs[i + 1], est) |
There was a problem hiding this comment.
What do you intend for this to do? I'm assuming you don't want the comma operator here (or maybe I'm missing something).
And doesn't this conflate "double positions" with "missing velocities"?
There was a problem hiding this comment.
My only goal with this change is to not run inverseOverflow on the data if it isn't from an integer source. I am honestly uncertain that this is a great way of detecting and doing that, and very open to feedback on better ways.
|
|
@j5155 can you help me understand how you are integrating your fork of this repository back into your teams codebase? |
For devices like the SparkFun OTOS and GoBilda PinPoint, tuning is made much easier if it can support non-integer encoders and can allow other libraries to make custom encoders. I have tested this pretty extensively with the SparkFun OTOS and can confirm that it works.