team401 / high-key-2024

Other
2 stars 0 forks source link

Add shooter/aimer test mode #41

Closed aidnem closed 1 month ago

aidnem commented 1 month ago

Purpose Add a test mode to manually program shooter angle and RPM and then take shots. Should hopefully allow for an easy time fine-tuning shooter values without having to redeploy. Project Scope

aidnem commented 1 month ago

@jkleiber instead of having a fixed offset hardcoded, we could consider trusting that the arm is down when the robot is powered on and calibrate the encoder to the bottom limit value on init, so that it would automatically calibrate itself instead of us having to find the offset manually whenever we need it.

jkleiber commented 1 month ago

I'm not sure what problem we would be solving there?

aidnem commented 1 month ago

Just hopefully decreasing the risk of having something really bad happen if the encoder offset ever gets messed up, and also preventing issues if it drifts over time or something weird like that. We also don't really need it for now as long as Aimer works.

jkleiber commented 1 month ago

This type of procedure would be more appropriate for a sensor that is capable of drift - in the case of an absolute encoder, sensor drift means the encoder has dislodged in some way, and would be a failure mode that we cannot handle in our current architecture.

If we were manufacturing 1000 of these robots, it would make sense to create an auto-calibration sequence. This is because it would be reasonable to assume the CANCoder is installed in the exact same configuration every time, and we would want to have the same behavior across robots.

For our use case, I think the actual main benefit would be that we would essentially guarantee that we can swap out the CANCoder if it breaks with no calibration work (assuming the CANCoder is installed exactly the same). So I think we shouldn't totally dismiss the idea. From a risk perspective, I think it's a fairly low likelihood that we break the CANCoder and a low criticality in terms of how much time we would lose to recalibrating (it took us like 5-10 minutes to calibrate this the first time).

aidnem commented 1 month ago

Okay, sounds good.

aidnem commented 1 month ago

@jkleiber @minhnguyenbhs @linglejack06 @avidraccoon I discovered why none of our smartdashboard values were being logged from test mode. We forgot to actually call RobotContainer.testInit. 2024 Robot Code Robot.java Robot.testInit (line 134):

    @Override
    public void testInit() {
        CommandScheduler.getInstance().cancelAll();

        robotContainer.enabledInit();
        robotContainer.testInit();
    }

We need to add the last 2 lines to our robotContainer in high-key for our RobotContainer.testInit to ever actually be called.

aidnem commented 1 month ago

Sorry for the mass ping but I just wanted to let everyone know so that if we write test modes for every subsystem we won't have to debug this issue on multiple different branches.

jkleiber commented 1 month ago

Ugh, I figured this was the case. Good find @aidnem !

aidnem commented 1 month ago

@jkleiber either we have to stop using the CANCoder and use the motor's internal encoder only or we have to stop it (and the min-angle setpoint) from drifting. I'm not entirely sure where the problem lies but we can figure it out in shop tomorrow.

jkleiber commented 1 month ago

@aidnem do you think it is possible that the FusedCanCoder is what is causing our problems? We could try going to pure CanCoder mode.

If you/someone create a branch for going to CanCoder we could give that a try. From here on out I want to do 1:1 ratio of branches to features (I do not want us to maintain large integration branches anymore)

aidnem commented 1 month ago

@jkleiber it could be, I hadn't considered that. Especially if our motor gear reduction is wrong or something like that, that would make it get progressively worse/wrong different amounts over time. I've created issue #52 to try it out.

minhnguyenbhs commented 1 month ago

this may already be done?

jkleiber commented 1 month ago

@minhnguyenbhs is right, this is done already