Arm: clarify target units and display Mechanism2d in AdvantageScope #96
Conversation
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
JohnGilb
approved these changes
Feb 17, 2024
| public double getExtensionForArmAngle(double angle) { | ||
| // TODO: this is just a placeholder, the relationship will be nonlinear | ||
| var degreesPerMmExtension = 0.01; | ||
| return angle / degreesPerMmExtension; |
There was a problem hiding this comment.
⭐ check for degreesPerMmExtension being 0
There was a problem hiding this comment.
I think that's overkill for this placeholder logic, but I'll add it anyway
| // what angle does the arm make with the pivot when it's at our concept of zero? | ||
| var armPivotAngleAtArmAngleZero = 45; | ||
| var color = isCalibrated() ? new Color8Bit(0, 255, 0) : new Color8Bit(255, 0, 0); | ||
| var armActual2d = new Mechanism2d(10, 10, new Color8Bit(255, 255, 255)); |
There was a problem hiding this comment.
⭐ ⭐ the construction of this mechanism is kind of expensive; you should be able to initialize it in the constructor and update any needed fields in periodic().
This reverts commit 0ddd1e9.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why are we doing this?
We said the Arm would PID on the extension distance, but we were confusing this for the target angle a lot.
This code makes that separation more clear with a placeholder conversion between angle and distance provided.
Asana task URL: https://app.asana.com/0/38541457243752/1206617786151554
Whats changing?
Questions/notes for reviewers
through this I noticed a bug where once the arm is calibrated, it won't go down again
How this was tested
arm.in.AS.mp4