koibots8230 / RumbleBot2024

MIT License
0 stars 0 forks source link

Missing inputs for motor construction #4

Open eskiles opened 1 month ago

eskiles commented 1 month ago

@tobyMickelson Please review Anglerfish to find other settings that are used to properly configure a motor controller. Most of these show up in the Constants class.

You will quickly find that passing all of these individual arguments in the constructor is not ideal. Work with @JakeStew81 to define a configuration object for constructing motors. The constants class should use this type for motor config information.

https://github.com/koibots8230/RumbleBot2024/blob/031d932d3a9478744a8b93d109db4d089f4973a0/RumbleBot2024/src/main/java/frc/robot/devices/motor/MotorCANSparkMax.java#L11C12-L11C28

tobyMickelson commented 1 month ago

I updated the interface to allow for many more features from what I saw used in constants

eskiles commented 1 month ago

Those are the settings I was looking for, but that's not the implementation I was going for.

After we configure the motor, we never ask the motor for the config information. This includes PIDF values, inverted, ID, current limits, timeouts, scaling factors. So, we don't actually want getter/setter methods for these values. This could change if you all decide to support live tuning of motor configs from the dashboard.

Move these configuration parameters into their own class, MotorConfig. The motor interface should focus on the actions like getting and setting velocities/positions. A MotorConfig object gets passed into the MotorFactory.create method. This does the work to properly instantiate and configure the motor.

Since we don't know what all the SIM motor might need from the config data, I recommend just storing it as part of the SIM motor object.