Skip to content
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

Doc Expansion #890

Merged
merged 45 commits into from
Nov 22, 2024
Merged

Doc Expansion #890

merged 45 commits into from
Nov 22, 2024

Conversation

oh-yes-0-fps
Copy link
Collaborator

requires #884 to be merged first

oh-yes-0-fps and others added 7 commits October 31, 2024 12:56
- changed `atTimeAndPlace` to `atTimeAndPose`
- removed unused `timeOffset` member from `AutoTrajectory`
- inlined some methods only used once
- added easier flipping to trajectory sample fetching
- added method to get the underlying trajectory of an `AutoTrajectory`
choreolib/src/main/java/choreo/auto/AutoChooser.java Outdated Show resolved Hide resolved
docs/choreolib/auto-routines.md Outdated Show resolved Hide resolved
docs/choreolib/auto-routines.md Outdated Show resolved Hide resolved
docs/choreolib/auto-routines.md Outdated Show resolved Hide resolved
docs/choreolib/auto-routines.md Outdated Show resolved Hide resolved
@shueja
Copy link
Collaborator

shueja commented Nov 16, 2024

I like the restructuring of the .md files, but I'll want to do a final spelling/wording pass before merging this.

choreolib/src/main/java/choreo/auto/AutoChooser.java Outdated Show resolved Hide resolved
docs/choreolib/auto-routines.md Show resolved Hide resolved
docs/choreolib/basic-usage.md Outdated Show resolved Hide resolved
docs/choreolib/installation.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@spacey-sooty spacey-sooty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to use RobotContainer for all the code examples that use Robot? Since it's still in the WPILib templates many teams will still be using it and I feel this could be a source of confusion

docs/choreolib/auto-controller.md Outdated Show resolved Hide resolved
docs/choreolib/auto-controller.md Show resolved Hide resolved
docs/choreolib/auto-factory.md Outdated Show resolved Hide resolved
docs/choreolib/basic-usage.md Show resolved Hide resolved
The "Auto Controller" is a function that consumes a `Pose2d` as well as a sample (`SwerveSample` or `DifferentialSample` depending on drive base).
The controller should capture your drive subsystem to directly control it.

## Basic PID Controller
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular implementation is overkill. I think we should recommend a function in the drivebase subsystem, which captures the subsystem and PID controllers. This is how the CTRE examples work.

@shueja
Copy link
Collaborator

shueja commented Nov 22, 2024

Approved for merge. My one remaining comment can be tackled in a later PR because the currently recommended code does work.

@oh-yes-0-fps oh-yes-0-fps merged commit b849eec into SleipnirGroup:main Nov 22, 2024
32 checks passed
@oh-yes-0-fps oh-yes-0-fps deleted the doc-expandsio branch November 22, 2024 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants