Added routine.trajectory() method #912
Conversation
|
Is adding a second way to do the same thing necessary? Can we remove the other one? |
Possibly; I didn't want to introduce a breaking change. |
|
The API is new for 2025, so we can break whatever we want before kickoff. |
| /** A generator that returns an auto routine that does nothing */ | ||
| static final AutoRoutineGenerator NONE = factory -> AutoFactory.VOID_ROUTINE; |
There was a problem hiding this comment.
Why change the way this is done?
There was a problem hiding this comment.
voidRoutine has to be an instance method because AutoRoutine now depends on the AutoFactory.
| final TrajectoryCache trajectoryCache = new TrajectoryCache(); | ||
| final Supplier<Pose2d> poseSupplier; | ||
| final BiConsumer<Pose2d, ? extends TrajectorySample<?>> controller; | ||
| final BooleanSupplier mirrorTrajectory; | ||
| final Subsystem driveSubsystem; | ||
| final AutoBindings bindings = new AutoBindings(); | ||
| final Optional<TrajectoryLogger<? extends TrajectorySample<?>>> trajectoryLogger; |
There was a problem hiding this comment.
this feels off to me; I don't get the benefit from breaking encapsulation like this
There was a problem hiding this comment.
I could pass them in as dependencies instead of passing in the AutoFactory itself. Overall though, it still doesn't break encapsulation from the library side(and was the only way I could do this)
|
The members being package protected is silly, just have the trajectory methods in auto factory be package protected. |
oh-yes-0-fps
force-pushed
the
trajectory-method-in-routine
branch
from
601f774 to
e6d487a
Compare
oh-yes-0-fps
force-pushed
the
trajectory-method-in-routine
branch
from
5367358 to
4224991
Compare
oh-yes-0-fps
force-pushed
the
trajectory-method-in-routine
branch
from
725e96f to
b997828
Compare
| @@ -213,7 +213,7 @@ public AutoRoutine fivePieceAutoCompositionSeg(AutoFactory factory) { | |||
| // value | |||
| // AMP, SUB, SRC: The 3 starting positions | |||
| // Try to load all the trajectories we need | |||
| final AutoTrajectory ampToC1 = factory.trajectory("ampToC1", factory.voidLoop()); | |||
There was a problem hiding this comment.
nvm this causes an exception
Currently, the way to construct AutoTrajectories is using the factory.trajectory method:
This PR provides a better-named alternative:
Note: c++ and docs updates are still tbd.