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

Turret Presets #153

Merged
merged 22 commits into from
Feb 24, 2024
Merged

Turret Presets #153

merged 22 commits into from
Feb 24, 2024

Conversation

ACat701
Copy link
Contributor

@ACat701 ACat701 commented Feb 20, 2024

Closes #159

- Create and refactor desired shooter speed logic into the subsystem
- Rename "climberPref" (it was annoying me)
- Create preset velocities
- Change default locking position back to NONE
@ACat701
Copy link
Contributor Author

ACat701 commented Feb 21, 2024

@FRCTeam3255/code-reviewers Currently thinking about our trap implementation. Although I think its nice that we have logic to "shoot into the trap," I find it very unlikely that we'll actually be doing that. It also makes the code simpler if we don't.

I'd like to propose making all of the logic simpler by turning the trap score into a preset (like subwoofer & the amp override). For reference, it's currently similar to "Lock Amp"

Another similar topic of discussion: the AMP!!!
Our current desired implementation of the Amp in my brain (following our controller layout, not the code) is as follows:

  • "Lock Amp" = Shooter and pitch are constant values. The only auto-aim is the turret
  • "Amp Preset" = Shooter, pitch, and turret are constant values

At this point, what is the Pro of having the lock amp button? I think it won't get used, just adding complexity to the driving experience (which we really want to minimize with our current situation). I propose that we remove the "Lock Amp" button entirely (at least for now) and remap the controller accordingly.

also I am taking all of my misc. clean-up changes and moving them to their own pr because i realised i made so many that i just want in main at this point so look out for that pr in like an hour k thx (#162)

@ACat701
Copy link
Contributor Author

ACat701 commented Feb 21, 2024

Testing if LockPitch gives us good angles in SIM:

WING

Observed Value: 18 degrees
Statistic: 19.1 degrees
image

CENTERLINE

Observed Value: 16 degrees
Statistic: 13.96
image

Really bad desmos graph of those here: https://www.desmos.com/calculator/56mrsfygsj

@TaylerUva
Copy link
Member

@ACat701 I agree. I think we just have lock speaker (pose yaw and pitch and static rpm). And then AMP preset (static pitch, yaw, rpm), Trap preset (static pitch, yaw, rpm) and sub preset (static pitch, yaw, rpm). I don't think the robot is physically capable of doing AMP, Trap or Sub of those with anything but static presets. Only speaker can be dynamic.

@ACat701
Copy link
Contributor Author

ACat701 commented Feb 21, 2024

Changes have been implemented 👍

@ACat701 ACat701 added the Untested This pull request has not been tested at all label Feb 22, 2024
@ACat701
Copy link
Contributor Author

ACat701 commented Feb 23, 2024

I changed the height of the speaker to make that test work. It probably did need to be shifted slightly, but not as much as I changed it. In hindsight, the real issue was this: (the todo comment cracks me up lmao)

 // TODO: Update with real values (MUST DO BEFORE TESTING!)
    /**
     * The position, in meters, of the center of rotation for the pitch motor
     * relative to the center of the robot (Robot Coordinates).
     * 
     * @see <a href=
     *      "https://docs.wpilib.org/en/stable/docs/software/basic-programming/coordinate-system.html">Robot
     *      Coordinate System</a>
     */
    public static final Transform3d ROBOT_TO_PITCH = new Transform3d(
        new Translation3d(0, 0, 0),
        new Rotation3d(0, 0, 0));

@TaylerUva Could you get me the values for this transform 3d (from the cad)? We'll ignore the whole "its on a turret" issue for now. Just get the position based on when the turret is pointing straight ahead

TaylerUva
TaylerUva previously approved these changes Feb 24, 2024
@ACat701 ACat701 marked this pull request as ready for review February 24, 2024 22:38
@ACat701 ACat701 requested a review from a team as a code owner February 24, 2024 22:38
@ACat701 ACat701 enabled auto-merge (squash) February 24, 2024 22:38
@ACat701 ACat701 dismissed TaylerUva’s stale review February 24, 2024 22:39

Because i dont care!!!

TaylerUva
TaylerUva previously approved these changes Feb 24, 2024
@ACat701 ACat701 merged commit ba00de2 into main Feb 24, 2024
1 check passed
@ACat701 ACat701 deleted the subwoofer-&-amp-preset branch February 24, 2024 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Untested This pull request has not been tested at all
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set desired pitch angle to zero when disabled
2 participants