Motor Template Classes

[nfoert]

Create motor template classes for power, velocity, linear position, and angular position. These classes can now later be used to greatly simplify our subsystems, and serve as a good foundation for implementing motion magic and other things later.

• Create base class files
• Implement PowerMotor
• Implement VelocityMotor
• Implement LinearPositionMotor
• Implement AngularPositionMotor
• Make classes inherit from PowerMotor to prevent duplicated code

[nfoert]

@Purgso I've started to lay the base parts of this and implemented PowerMotor. Could you check over what I did and see if you're happy with how I'm organizing it before I get into the complexity of implementing the other classes?

I had a few additional thoughts:

• Would it make sense to make a BaseMotor class to consolidate the logic for creating the motor and motor configurations as that will likely be repeated for each class?
• And I created a SC_TemplateMotorCurrentConfig dataclass, but that's identical to SC_SwerveCurrentConfig. Should we merge these into the same object?

[Purgso]

Yes, how you've organized it is exactly how I pictured.

If you find yourself duplicating code a lot then I definitely think a base class is a good idea. Actually couldn't PowerMotor work as the base class? We use set_power for open loop testing anyway. You may want to make a PositionMotorBase as well so you're not duplicating PID + Feed Forward + Motion Profile. Then the linear and angular versions only need to add unit conversions.

I don't see a reason to combine the motor configs. I prefer to keep drivetrain code entirely separate from the rest of the robot because they're developed completely differently. Drivetrain is reused with small changes each year while the rest of the robot is completely rewritten based on robot design with only the base building blocks reused. I'd rather keep them fully separate than risk a change to one breaking the other further down the road. Swerve configs contain exactly what we need for the swerve module and motor config should contain exactly what we need for one motor.

About your todo for inverting the motors, Here's how I did it for the swerve motors: InvertedValue is an enum where the two directions have values 0 and 1. This means you can construct an inverted value directly from a boolean:
self._motor_config.motor_output.inverted = InvertedValue(motor_config.inverted)

I'm going through CTRE's example swerve drivetrain looking for ways we can improve our code. I'll let you know if I find anything.

[nfoert]

Alright thank you. I agree that making a base class to a degree is a good idea, I think I may just wait to see how much of it is duplicated once I'm done with all four and separate them into a base class from there.

As for the motor configs I will separate that back out, I realize now that using the same one might be a bad idea for future proofing. And I'll make that change to the motor inversion as well.

[nfoert]

@Purgso, I think I've gotten VelocityMotor partially implemented. I did have a few questions:

• Did I implement at_target_speed correctly? I couldn't remember how the equivalent function's math worked on Agent Smith, and it was using REV motors so the C++ code can't just be directly ported over
• How should we handle PID and trapezoids? I created a PIDController in the class, but we won't have a periodic function to handle the states. Should these classes inherit from Subsystem and override periodic or did you have a better idea for this?

[Purgso]

Agent Smith uses an SC_LauncherSpeed object for setting speed. If you look at constants, you can see we have pairs of them (one for top roller, one for bottom) for the various scoring situations. SC_LauncherSpeed has two values: power level and speed. If both are zero, no power is sent to the motor. If power is not zero, the motor is run at the given power level and at_target_speed returns true if the speed is greater than the speed part of the SC_LauncherSpeed. The only other scenario is that speed is set and power isn't and in that case, it uses PID to hit the target speed. set_power can just construct an SC_LauncherSpeed with speed 0 and power of whatever set_power was called with and pass that object to set_power. We chose to do it this way so we can easily command speed when scoring or power when feeding and I think we should keep this behavior.

at_target_speed logic looks good for speed control. You'll need different logic for checking if we're above the target rpm for open loop control. Last year's equation was basically return (motor_velocity - target_velocity) * (1 if target_velocity >= 0 else -1) > 0. This works whether we have a positive or negative power and target speed.
Edit: I asked ChatGPT to optimize the equation and it came up with this also valid equation:
return 0 <= target_velocity <= motor_velocity or 0 >= target_velocity >= motor_velocity

Motors should inherit from Subsystem to get access to periodic. I don't see anything wrong with having the motors work this way.

[nfoert]

Okay thank you. I did my best to implement this, but I think I'm a little confused by how we need to set the PID and exactly when we need open loop vs closed loop control. It doesn't help that REV to CTRE is a good bit different and CTRE's examples are very minimal. If possible could we walk through it on Thursday just to make sure I'm understanding everything?

I'll keep working on the position control bases as well, I think I just need a refresher on how the logic for the different control modes worked on Agent Smith.

[nfoert]

@Purgso I think I've gotten the base parts of AngularPositionMotor implemented. The main things I'm not sure of is if the periodic logic is correct.

• Right now I have it constantly setting the motor to the set position when State is POSITION and we're not in test mode. Will we need this to also check if it's at a home position to know when to stop powering the motor even though we're not using a homing sequence?
• Also what was the intention of using an external encoder? Is this just to future proof if we ever have an external encoder on this mechanism and we'd use that instead of the motor's get_position()?

[Purgso]

I'm still reviewing but here's what I've found so far:

• We already have a Motion Control section in SC_Datatypes. We don't need an entire new section with all new classes for the template motors since they're meant to replace the existing motor stuff. I already separated out swerve so any changes you make won't affect the drivetrain.
• You're getting linter errors when setting the position and velocity of a trapezoid state because it's read-only. You need to create a new one instead.
• I don't want the template motor classes to have a test mode. Test mode should be handled in the parent subsystem (pivot or elevator) by calling the set_power function.
• I also don't want the template motor classes to have homing code because I think it's too hardware dependent so the parent subsystem should handle it. Optional homing with an external limit switch would add a lot of code to the motor that I don't think we need right now.
• Yes, the external encoder is for if we ever actually get a motor directly on the pivot like I've been asking. You can configure the motor to use the external encoder instead of the internal one by setting the id to the id of the encoder in motor_config.feedback for TalonFX and motor_config.external_feedback for TalonFXS. You can also set the gear ratio.
• If the subsystem wants to unpower the motor when in the home position, it can just call set_power(0)
• The stall percentage calculation is close but not quite right. We don't want to check if the motor is stalled when the duty cycle is super low because it's almost always going to return True. I think this limit can just be a constant within the class, it doesn't need to be passed through the constructor. The value you're using for that right now is the same one you're using as a threshold for if the motor is stalled. Also the stalled % needs to be corrected for voltage. The get_motor_stall_current function returns the stall current at 12v so we need to adjust it based on the supply voltage.

[Purgso]

Also the print_diagnostics function should just print diagnostics. Let the parent subsystem decide when it should be run.

[nfoert]

Okay thank you. I made most of those changes. I renamed SC_LauncherSpeed in favor of SC_VelocityControl and SC_PositionControl for the sake of having a more generic name.

Do you have an example of the correct stall percentage calculation? I'm not quite sure how that math worked, and what I did was just a copy of what we did last year.

[nfoert]

@Purgso I've adjusted the motor classes around to make them inherit from PowerMotor and also that LinearPositionMotor inherits from AngularPositionMotor.

If you have time, could you check over to make sure I did the unit conversion math right in the LinearPositionMotor?