wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.03k stars 607 forks source link

General Pose Estimator #3703

Closed TateStaples closed 11 months ago

TateStaples commented 2 years ago

Is your feature request related to a problem? Please describe. I am trying to build an automatic FRC robot and the poseEstimators have been very useful. I have been trying to make team libraries to wrap this functionality but I need a separate class for each drivetrain type (I am trying to be as robust as possible)

Describe the solution you'd like I would like to create a standard PoseEstimator class that takes chassisSpeeds instead of specific wheel speeds. My logic is that MecanumPoseEstimator and SwervePoseEstimator use essentially the same code but with different kinematics (I would also guess the dif chassis could be estimated the same way). If you put all the code into one base file and then inherit from that it would provide more functionality, be more readable, and more robust to unique types of drivetrains.

Describe alternatives you've considered The current system is workable, so if it isn't a priority thats fine

Additional Information I was planning on making this own my own but the KalmanFilterLatencyCompensator isn't accessible from outside the package.

calcmogul commented 2 years ago

I have a bunch of questions.

an automatic FRC robot

What does that mean?

I have been trying to make team libraries to wrap this functionality but I need a separate class for each drivetrain type (I am trying to be as robust as possible)

I don't know what you mean by robust here. Could you explain your use case more?

I would like to create a standard PoseEstimator class that takes chassisSpeeds instead of specific wheel speeds. My logic is that MecanumPoseEstimator and SwervePoseEstimator use essentially the same code but with different kinematics (I would also guess the dif chassis could be estimated the same way).

That's not correct. The differential drive pose estimator uses a different model from the mecanum and swerve pose estimators.

If you put all the code into one base file and then inherit from that it would provide more functionality, be more readable, and more robust to unique types of drivetrains.

The internals of each class are different enough that you couldn't really stick any of it in a base class. You'd be limited to just an interface. In that case, an interface doesn't provide any more functionality. Even if you were able to refactor it into a base class, that's not adding functionality; that's just moving the functionality around.

Also, how is it more readable if you do that? What other drivetrain types would you have, and why does adding an interface make those easier to implement?

TateStaples commented 2 years ago
  1. generally using visual slam algorithms and game object recognition to play an FRC-style game with no user input
  2. More flexible to different types of drivetrains
  3. I get DifDrive is different, but that is just an optimization. The math from Mec and Swerve would still work with any chassis type.
  4. Looking at Mec & Swerve, they seem to use all the same math, just have seperate kinematics to turn the corresponding wheel speeds into chassis speeds. If you put all that shared math into one class, you condense the amount of code and clarify what each thing is doing. You can then inherit from the base class to add kinematic options for different drivetrain types.
TateStaples commented 2 years ago

Also, how is it more readable if you do that? What other drivetrain types would you have, and why does adding an interface make those easier to implement?

There are a ton of random fringe case drivetrains. Adding the functionality to use Pose regardless of chassis type would be useful and more flexible. Examples: https://www.chiefdelphi.com/t/weirdest-drive-trains/159291/3

TateStaples commented 2 years ago

I made a scuffed version here: https://github.com/TateStaples/AutoFRC/blob/master/src/main/kotlin/kyberlib/auto/DrivePoseEstimator.kt

calcmogul commented 11 months ago

5355 added a PoseEstimator base class that lets you specify custom kinematics and odometry. It can be used separately, though it's recommended to make a derived class that wraps the interactions.