Replaced orElseGet in docs for starting pose #947
Conversation
docs/choreolib/auto-routines.md
Outdated
| .andThen( | ||
| autoAimAndShoot(), | ||
| race( | ||
| intake(), | ||
| ampToC1.cmd(), | ||
| aimFor(ampToC1.getFinalPose().orElseGet(Pose2d::new)))) | ||
| aimFor(ampToC1.getFinalPose().orElse(new Pose2d())))) |
There was a problem hiding this comment.
Idk how this even got here but it shouldn't be here
There was a problem hiding this comment.
No yeah i replaced it on purpose; i think i should revert it tho
| routine.kill(); | ||
| return new Pose2d(); | ||
| })) | ||
| resetOdometry(initialPose.get()) |
There was a problem hiding this comment.
If I'm not mistaken, this method is prone to error as it is only evaluating if the initial pose of the trajectory should be flipped when fivePieceAutoTriggerSeg is invoked. If the user is using AutoChooser, this would be when the auto is selected, at which point the robot may know the correct alliance. However assuming a worst case scenario where the user is utilizing the traditional SendableChooser or a custom solution, the segment would be initialized at startup. resetOdometry() should either be expected to accept a Pose2d supplier, or should be deferred. I understand this issue isn't the initial scope of the PR however I feel it is relevant.
There was a problem hiding this comment.
Hmm... I get what you mean. I'll prob change it to a Pose2d supplier(the OG docs also didn't cover this use case).
| routine.kill(); | ||
| return new Pose2d(); | ||
| })) | ||
| resetOdometry(initialPose.get()) |
| } | ||
|
|
||
| Command ret = sequence( | ||
| resetOdometry(startinNoteOwnedose), | ||
| resetOdometry(initialPose.get()), |
Replaced the orElseGet in the docs in favor of isEmpty() checks for the starting pose's Optional.
Both of them work the same way, but an explicit isEmpty() check + returning a void routine makes it easier for rookie coders reading the documentation.
(Also corrects an invalid Commands.none() return in a method that returns an AutoRoutine)