koibots8230 / RumbleBot2024

MIT License
0 stars 0 forks source link

Elevator clean up #11

Closed eskiles closed 3 days ago

eskiles commented 1 month ago

Member variables that are created once and don't change should have the final keyword. This will ensure a compile time error if the object isn't created rather than a runtime null pointer exception.

Remove the member variable isReal. WPILIP already handles the logic for calling simulation. All simulation activity should occur during simulationPeriodic and only during simulationPeriodic. Notice that the isReal check in periodic has an if real, but doesn't do anything if it isn't. Calling the real hardware doesn't affect the simulation.

If the robot is real, then the simulation objects should be set to null. This will cause a null pointer exception if simulation objects are called during normally scheduled robot activities.

These changes will have some logic impacts.

Naming conventions: private SparkPIDController realFeedback; This name confused me when I was reading through the code. Simulation is the exception, so it needs to be clearly marked. Real objects are required and do not always have a sim counter part. It is cleaner to not call out real for these. Feedback isn't as descriptive as something like elevatorPIDControl.

Final question. What do you expect to learn from the simulation and how will you use that information for the real robot?

https://github.com/koibots8230/RumbleBot2024/blob/8773c39fc4cb7bd8ce25e9f93c5bdcaa31ae06c7/src/main/java/frc/robot/subsystems/Elevator.java#L30