-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds Swerve2/3/4/6ControllerCommand to Commands2 #38
Closed
Closed
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
8bc6e39
Adds Swerve2/3/4/6ControllerCommand to Commands2
d883093
Fixes for mypy errors
645163a
Fixes broken test run for incomplete pid
1042681
Experimenting with mypy assignment error
8464e6f
Ignoring mypy type error with explicit annotation
3c74662
Re-formatted with black
3a8432e
Merge branch 'robotpy:main' into swervecontrollercommand
lospugs 92455c0
Added Typing.overload to constructor definitions, forced kwargs for o…
10c6705
Removed superfluous comment
f2828ae
Updated implementation to use Overtake and Beartype.
95b54b9
Fixed test fixtures to supply tuple to odometry update
3a441f4
Fixed tests for odometry update calls to send Tuples
0efaf1d
Fix argument
virtuald 68f1f5d
Bug fix: Added addRequirements call to contstructors
bb2778c
Pinned versions of experimental libraries
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,292 @@ | ||
# Copyright (c) FIRST and other WPILib contributors. | ||
# Open Source Software; you can modify and/or share it under the terms of | ||
# the WPILib BSD license file in the root directory of this project. | ||
from __future__ import annotations | ||
from typing import Callable, Optional, Union, Tuple | ||
from typing_extensions import overload | ||
from overtake import overtake | ||
from wpimath.controller import ( | ||
HolonomicDriveController, | ||
PIDController, | ||
ProfiledPIDControllerRadians, | ||
) | ||
from wpimath.geometry import Pose2d, Rotation2d | ||
from wpimath.kinematics import ( | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
SwerveModuleState, | ||
) | ||
from wpimath.trajectory import Trajectory | ||
from wpilib import Timer | ||
|
||
from .command import Command | ||
from .subsystem import Subsystem | ||
|
||
|
||
class SwerveControllerCommand(Command): | ||
""" | ||
A command that uses two PID controllers (PIDController) and a ProfiledPIDController | ||
(ProfiledPIDController) to follow a trajectory (Trajectory) with a swerve drive. | ||
|
||
This command outputs the raw desired Swerve Module States (SwerveModuleState) in an | ||
array. The desired wheel and module rotation velocities should be taken from those and used in | ||
velocity PIDs. | ||
|
||
The robot angle controller does not follow the angle given by the trajectory but rather goes | ||
to the angle given in the final state of the trajectory. | ||
""" | ||
|
||
@overload | ||
def __init__( | ||
self, | ||
trajectory: Trajectory, | ||
pose: Callable[[], Pose2d], | ||
kinematics: Union[ | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
], | ||
xController: PIDController, | ||
yController: PIDController, | ||
thetaController: ProfiledPIDControllerRadians, | ||
desiredRotation: Callable[[], Rotation2d], | ||
outputModuleStates: Callable[[SwerveModuleState], None], | ||
requirements: Tuple[Subsystem], | ||
) -> None: | ||
""" | ||
Constructs a new SwerveControllerCommand that when executed will follow the | ||
provided trajectory. This command will not return output voltages but | ||
rather raw module states from the position controllers which need to be put | ||
into a velocity PID. | ||
|
||
Note: The controllers will *not* set the outputVolts to zero upon | ||
completion of the path- this is left to the user, since it is not | ||
appropriate for paths with nonstationary endstates. | ||
:param trajectory: The trajectory to follow. | ||
:param pose: A function that supplies the robot pose - use one of the odometry classes to | ||
provide this. | ||
:param kinematics: The kinematics for the robot drivetrain. Can be kinematics for 2/3/4/6 | ||
SwerveKinematics. | ||
:param outputModuleStates: The raw output module states from the position controllers. | ||
:param requirements: The subsystems to require. | ||
:param xController: The Trajectory Tracker PID controller | ||
for the robot's x position. | ||
:param yController: The Trajectory Tracker PID controller | ||
for the robot's y position. | ||
:param thetaController: The Trajectory Tracker PID controller | ||
for angle for the robot. | ||
:param desiredRotation: The angle that the drivetrain should be | ||
facing. This is sampled at each time step. | ||
""" | ||
super().__init__() | ||
self._trajectory = trajectory | ||
self._pose = pose | ||
self._kinematics = kinematics | ||
self._outputModuleStates = outputModuleStates | ||
self._controller = HolonomicDriveController( | ||
xController, yController, thetaController | ||
) | ||
self._desiredRotation = desiredRotation | ||
self._timer = Timer() | ||
self.addRequirements(requirements) # type: ignore | ||
|
||
@overload | ||
def __init__( | ||
self, | ||
trajectory: Trajectory, | ||
pose: Callable[[], Pose2d], | ||
kinematics: Union[ | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
], | ||
xController: PIDController, | ||
yController: PIDController, | ||
thetaController: ProfiledPIDControllerRadians, | ||
outputModuleStates: Callable[[SwerveModuleState], None], | ||
requirements: Tuple[Subsystem], | ||
) -> None: | ||
""" | ||
Constructs a new SwerveControllerCommand that when executed will follow the | ||
provided trajectory. This command will not return output voltages but | ||
rather raw module states from the position controllers which need to be put | ||
into a velocity PID. | ||
|
||
Note: The controllers will *not* set the outputVolts to zero upon | ||
completion of the path- this is left to the user, since it is not | ||
appropriate for paths with nonstationary endstates. | ||
|
||
Note 2: The final rotation of the robot will be set to the rotation of | ||
the final pose in the trajectory. The robot will not follow the rotations | ||
from the poses at each timestep. If alternate rotation behavior is desired, | ||
the other constructor with a supplier for rotation should be used. | ||
|
||
:param trajectory: The trajectory to follow. | ||
:param pose: A function that supplies the robot pose - use one of the odometry classes to | ||
provide this. | ||
:param kinematics: The kinematics for the robot drivetrain. Can be kinematics for 2/3/4/6 | ||
SwerveKinematics. | ||
:param xController: The Trajectory Tracker PID controller | ||
for the robot's x position. | ||
:param yController: The Trajectory Tracker PID controller | ||
for the robot's y position. | ||
:param thetaController: The Trajectory Tracker PID controller | ||
for angle for the robot. | ||
:param outputModuleStates: The raw output module states from the position controllers. | ||
:param requirements: The subsystems to require. | ||
""" | ||
super().__init__() | ||
self._trajectory = trajectory | ||
self._pose = pose | ||
self._kinematics = kinematics | ||
self._outputModuleStates = outputModuleStates | ||
self._controller = HolonomicDriveController( | ||
xController, yController, thetaController | ||
) | ||
self._desiredRotation = self._trajectory.states()[-1].pose.rotation | ||
self._timer = Timer() | ||
self.addRequirements(requirements) # type:ignore | ||
|
||
@overload | ||
def __init__( | ||
self, | ||
trajectory: Trajectory, | ||
pose: Callable[[], Pose2d], | ||
kinematics: Union[ | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
], | ||
controller: HolonomicDriveController, | ||
desiredRotation: Callable[[], Rotation2d], | ||
outputModuleStates: Callable[[SwerveModuleState], None], | ||
requirements: Tuple[Subsystem], | ||
) -> None: | ||
""" | ||
Constructs a new SwerveControllerCommand that when executed will follow the | ||
provided trajectory. This command will not return output voltages but | ||
rather raw module states from the position controllers which need to be put | ||
into a velocity PID. | ||
|
||
Note: The controllers will *not* set the outputVolts to zero upon | ||
completion of the path- this is left to the user, since it is not | ||
appropriate for paths with nonstationary endstates. | ||
|
||
:param trajectory: The trajectory to follow. | ||
:param pose: A function that supplies the robot pose - use one of the odometry classes to | ||
provide this. | ||
:param kinematics: The kinematics for the robot drivetrain. Can be kinematics for 2/3/4/6 | ||
SwerveKinematics. | ||
:param controller: The HolonomicDriveController for the drivetrain. | ||
:param desiredRotation: The angle that the drivetrain should be | ||
facing. This is sampled at each time step. | ||
:param outputModuleStates: The raw output module states from the position controllers. | ||
:param requirements: The subsystems to require. | ||
|
||
""" | ||
super().__init__() | ||
self._trajectory = trajectory | ||
self._pose = pose | ||
self._kinematics = kinematics | ||
self._outputModuleStates = outputModuleStates | ||
self._controller = controller | ||
self._desiredRotation = desiredRotation | ||
self._timer = Timer() | ||
self.addRequirements(requirements) # type:ignore | ||
|
||
@overload | ||
def __init__( | ||
self, | ||
trajectory: Trajectory, | ||
pose: Callable[[], Pose2d], | ||
kinematics: Union[ | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
], | ||
controller: HolonomicDriveController, | ||
outputModuleStates: Callable[[SwerveModuleState], None], | ||
requirements: Tuple[Subsystem], | ||
) -> None: | ||
""" | ||
Constructs a new SwerveControllerCommand that when executed will follow the | ||
provided trajectory. This command will not return output voltages but | ||
rather raw module states from the position controllers which need to be put | ||
into a velocity PID. | ||
|
||
Note: The controllers will *not* set the outputVolts to zero upon | ||
completion of the path- this is left to the user, since it is not | ||
appropriate for paths with nonstationary endstates. | ||
|
||
Note 2: The final rotation of the robot will be set to the rotation of | ||
the final pose in the trajectory. The robot will not follow the rotations | ||
from the poses at each timestep. If alternate rotation behavior is desired, | ||
the other constructor with a supplier for rotation should be used. | ||
|
||
:param trajectory: The trajectory to follow. | ||
:param pose: A function that supplies the robot pose - use one of the odometry classes to | ||
provide this. | ||
:param kinematics: The kinematics for the robot drivetrain. Can be kinematics for 2/3/4/6 | ||
SwerveKinematics. | ||
:param controller: The HolonomicDriveController for the drivetrain. | ||
:param outputModuleStates: The raw output module states from the position controllers. | ||
:param requirements: The subsystems to require. | ||
|
||
""" | ||
super().__init__() | ||
self._trajectory = trajectory | ||
self._pose = pose | ||
self._kinematics = kinematics | ||
self._outputModuleStates = outputModuleStates | ||
self._controller = controller | ||
self._desiredRotation = self._trajectory.states()[-1].pose.rotation | ||
self._timer = Timer() | ||
self.addRequirements(requirements) # type: ignore | ||
|
||
@overtake(runtime_type_checker="beartype") | ||
def __init__( | ||
self, | ||
trajectory: Trajectory, | ||
pose: Callable[[], Pose2d], | ||
kinematics: Union[ | ||
SwerveDrive2Kinematics, | ||
SwerveDrive3Kinematics, | ||
SwerveDrive4Kinematics, | ||
SwerveDrive6Kinematics, | ||
], | ||
outputModuleStates: Callable[[SwerveModuleState], None], | ||
requirements: Tuple[Subsystem], | ||
*, | ||
controller: Optional[HolonomicDriveController] = None, | ||
xController: Optional[PIDController] = None, | ||
yController: Optional[PIDController] = None, | ||
thetaController: Optional[ProfiledPIDControllerRadians] = None, | ||
desiredRotation: Optional[Callable[[], Rotation2d]] = None, | ||
): | ||
... | ||
|
||
def initialize(self): | ||
self._timer.restart() | ||
|
||
def execute(self): | ||
curTime = self._timer.get() | ||
desiredState = self._trajectory.sample(curTime) | ||
|
||
targetChassisSpeeds = self._controller.calculate( | ||
self._pose(), desiredState, self._desiredRotation() | ||
) | ||
targetModuleStates = self._kinematics.toSwerveModuleStates(targetChassisSpeeds) | ||
|
||
self._outputModuleStates(targetModuleStates) | ||
|
||
def end(self, interrupted): | ||
self._timer.stop() | ||
|
||
def isFinished(self): | ||
return self._timer.hasElapsed(self._trajectory.totalTime()) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would benefit from the
@overload
approach too.An easy way to see what they should look like is download the 2023 commands whl file, unzip it, and peek at
commands2/_impl/__init__.pyi
which has all of the definitions that the prior version defined.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually had no idea that @overload existed. If you haven't figured it out yet, not a Python programmer :)
Hopefully tomorrow I can work up some updates for the other PRs as well that will hopefully make this much cleaner than me just creating overloads by arbitrarily forcing optional arguments to be required in certain permutations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, overload brings its own share of troubles -- it's just for the type checker, you still have to figure out which set of parameters are incoming.
See
robotpy-commands-v2/commands2/waituntilcommand.py
Line 38 in 0608bb8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed a change to this one implementing overload.
I don't know if this is too much of a breaking change, but I forced kwargs for all the non-positional arguments and then added the overload for the type checker. I added more tests to validate, and it seems to "work".
I don't know enough about Python that by explicitly making the subsystem a Tuple and forcing kwargs, what that does on the other end of things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out the type checker was not happy about it in Mypy, though when stepping through with a debugger, both in my force Tuple, and in the status quo (before the Tuple change) at runtime addRequirements receives a tuple[Subsystem] as it's type, not a Subsystem (which is what the type checker in Mypy says is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's important to remember that type checking is not enforced at runtime, so running code can sometimes work perfectly fine even if mypy is very angry about it. The primary utility of type hints is twofold: often mypy/pyright is right and type errors indicate that you should fix something (and you can find out before running the code), and it also serves as a useful API documentation to indicate what the function is expecting to receive.
I think we should investigate https://github.com/gabrieldemarmiesse/overtake + beartype to see if we can make this multiple dispatch stuff easier to write.