Stage 2 CTRE Solution Code - #175
Conversation
roboteer5291
added
CTRE
|
🌐 Preview URL: https://pr-175.frcsoftware.pages.dev |
Daniel1464
left a comment
Daniel1464
left a comment
There was a problem hiding this comment.
I know this is still a draft, but left some preliminary comments since i have time
| "enableCppIntellisense": false, | ||
| "currentLanguage": "java", | ||
| "projectYear": "2027_alpha5", | ||
| "teamNumber": 9999 |
There was a problem hiding this comment.
Small nitpick, but you can set this to null instead.
| static final double kI = 0; | ||
| static final double kD = 7; | ||
|
|
||
| static final double POSITION_TOLERANCE = 3.0/360.0; |
There was a problem hiding this comment.
Would this be better served using the units API? Degrees.of(3) would be a lot more clear here
There was a problem hiding this comment.
Changed to use Units.degreesToRotations instead. Since we aren't using the units API anywhere else really, I don't think it'd make sense to have it for this.
| motor.getConfigurator().apply(leaderConfiguration); | ||
|
|
||
| // Set up periodic method to run every code loop | ||
| Scheduler.getDefault().addPeriodic(this::periodic); |
There was a problem hiding this comment.
In the previous exercises, mechanism.periodic() is called manually in robotPeriodic(). Personally I think it's slightly better because it gives you control as to when the periodic() method is run
|
|
||
| private final FlywheelSim sim; | ||
|
|
||
| private final double GEAR_RATIO = (18.0 / 12.0) * (54.0 / 18.0) * (22.0 / 18.0) * (22.0 / 18.0); |
There was a problem hiding this comment.
It would be better to make the gear ratio a parameter of ClawSim so that users can play around with different gear ratios
There was a problem hiding this comment.
What value does this get in terms of teaching frc programming?
|
🌐 Preview URL: https://pr-175-frcsoftware.frcsoftware.workers.dev |
| * @return whether the arm is at that positoin | ||
| */ | ||
| public boolean isAtPosition(double position) { | ||
| return Math.abs(getPosition() - position) < Constants.POSITION_TOLERANCE; |
There was a problem hiding this comment.
Why not get the closed loop error status signal?
There was a problem hiding this comment.
This checks whether the arm is at a given position (ie is vertical) rather than whether it's at its setpoint. In a lot of cases they're the same thing, but I have run into cases where "at setpoint" isn't good enough. May not be necessary in this code (I'm actually thinking with state machines/v3 it won't be in general), but I guess that's just how I wrote it initially. I'll leave this open so I can come back and check it once I have more code done.
| * @param voltage the voltage to apply to the motor | ||
| * @return a command | ||
| */ | ||
| public Command setVoltage(double voltage) { |
There was a problem hiding this comment.
Why would you need open loop control on an arm?
There was a problem hiding this comment.
I don't think you would in this case, but for a homing sequence or the like it may be useful. It's just something I tend to include in all mechanisms, may not be useful here
| /** | ||
| * @return the velocity of the elevator, in meters per second | ||
| */ | ||
| public double getVelocity() { | ||
| return velocitySignal.getValueAsDouble() * Constants.PULLEY_CIRCUMFERENCE; | ||
| } |
There was a problem hiding this comment.
Is this a useful measurement to have?
| public Superstructure(Elevator elevator, Arm arm) { | ||
| this.elevator = elevator; | ||
| this.arm = arm; | ||
| } |
There was a problem hiding this comment.
How 'perfect' should this code be? As shown already, Elevator and Arm aren't really separate Mechanisms, they're just wrappers around a subset of motors in the one real Mechanism, the superstructure. The elevator and arm would never be commanded on their own, only via the superstructure to ensure proper coordination.
There are a couple of things necessary to do this 'correctly' from a software engineering pov:
- don't make elevator and arm implement
Mechanism. They have no need, since they never interact with the scheduler (at least, they shouldn't). - Make them package private, and only make the superstructure a public class, so that external classes cannot interact with them. They should always go through the superstructure, since that's the
Mechanism, andElevatorandArmare just abstractions because they mimic how we talk about the robots physical components. Software shouldn't care about whether they're elevator and arm, or a single superstructure, and hiding that abstraction prevents you from writing code that relies on this specific implementation of superstructure that is arbitrary
I understand that in this one example, having them be the way they are can work. However, the entire point of this course is to enable students/teams to write code that works all the time (or close to it). I like going out on the field and having alliance partners whose robots work really well, so I want this course to teach the best possible solution at each step that will ensure long term success.
3 participants
Creates stage 2 CTRE Solution Code. Still very much a WIP
Todo: