search

cyberblue234 / 2024CB234

Other
3 stars 0 forks source link

RPM Controller, possibly works? #13

Closed KittenGosCrazy closed 8 months ago

KittenGosCrazy commented 8 months ago

DO NOT merge this pull request until we can actually test it out. Don't want to get "Waiting on Programming" from the entire build team.

ejbunnell commented 8 months ago

Uhhh I may have already implemented it

ejbunnell commented 8 months ago

I mean from what I have seen I see no reason why it shouldn't work, but if it doesn't right out of the gate we can just use the amp scoring function, which uses speed instead

KittenGosCrazy commented 8 months ago

I would also (if possible) like to get the amp scorer on RPM mode too, better consistency. However, if just a direct power option works then we should stick with it.

I will check the controls branch for the RPM control. If we do have the RPM control in the branch, we can close this PR and move on to testing/tuning those

KittenGosCrazy commented 8 months ago

Checked out your RPM control in the current branch. I like the code structure you have in there currently a lot better. So will be closing this PR with this comment.

We may want to consider adding something in the Drivetrain and Elevator classes that returns if they are at their target. This would allow us to launch the note only if all conditions are met.

This branch should be our #1 test priority for today and Saturdays meeting, so we can hopefully move on to Auton code.