Team2168 / 2014_Main_Robot

Code for the 2014 FRC season.
Other
2 stars 1 forks source link

General Comment: Commands setting solenoids #68

Closed NotInControl closed 10 years ago

NotInControl commented 10 years ago

@jcorcoran

Programmers: when you have a command setting a solenoid in the execute block, you should not return true in the isFinished() block.

While the specifics escape me now, we noticed last year that sometimes the command would complete before the solenoid had time to latch, and the piston would not move.

In order to give the piston time to actuate isFinished should check the state of the solenoid and finish when the current state matches the commanded state in the execute block.

Is finished should contain something like return actuator.get() == DoubleSolenoid.Value.kReverse;

The easier way to do this is to add a method in the subsystem like isExtended/isRetracted, and return a boolean, do the check within the subsystem and just call the appropriate subsystem method in the isFinished block.

jcorcoran commented 10 years ago

For the record, DoubleSolenoid.Value.kReverse; doesn't return the position of the solenoid, it returns the last commanded state of the solenoid.

The destinction being - where you told the actuator (or even the valve) to go isn't necessarily where it currently is.

It is not safe to make the assumption that the actuator has actually moved to the specified destination position when the respective solenoid's get() method returns the state you commanded it to.

Sensors should be used in any condition where knowledge of the actuator's position is critical.

NotInControl commented 10 years ago

there were solenoid commands that simply returned true in isFinished (I believe they have been corrected). While this isn't an issue this year because you have a thread which runs and dies after sometime, I am almost certain that is by coincidence, and placing true in the finish block was done by mistake.

If using the regular solenoid class there will be times where your inital command (with return true in isFinished) command will fail to actuate the solenoid because it ended to quickly. Your statement is valid if you want to keep the command running long enough to have the actuator reach its destination.... While it may not have been clear in my original comment, the original comment was more concerned with keeping the command running long enough to see a state change on the solenoid. And to have the students understand why this needs to be done.

In either case, return true is incorrect.