CrossTheRoadElec / Phoenix-api

CTRE Phoenix language API (targets FIRST Robotics Competition, Linux, RaspPi, Windows)
Other
39 stars 28 forks source link

No getter for limit switch not being in normal state #27

Closed ryegleason closed 6 years ago

ryegleason commented 6 years ago

getSensorCollection().isFwdLimitSwitchClosed() seems to only get whether the limit switch is physically open or closed, meaning it works inconsistently with normally closed/normally open. It would be nice to have a method that returns whether the limit switch is in its normal state rather than just its physical position.

ozrien commented 6 years ago

That's the design intent. As the function name suggests, it returns true if the switch is closed (signal to ground), regardless of how you config the motor controller's limiting behavior. This is important as it allows general use of the pins (as generic digital inputs).

But it sounds like you want to know if the limit switch is limiting the motor output. In which case , poll the fault flags.... http://www.ctr-electronics.com/downloads/api/java/html/com/ctre/phoenix/motorcontrol/can/BaseMotorController.html#getFaults-com.ctre.phoenix.motorcontrol.Faults- ...and check the ForwardLimitSwitch instance variable... http://www.ctr-electronics.com/downloads/api/java/html/com/ctre/phoenix/motorcontrol/Faults.html

ryegleason commented 6 years ago

Would it be possible to also be able to get it from SensorCollection or BaseMotorController, or at least clearly document in getSensorCollection().isFwdLimitSwitchClosed() that the fault flags are another way to get the limit switch state? The fault detection API is nowhere near the first place I thought to look for limit switch getters.

Oblarg commented 6 years ago

Additionally, it's not clear if this does actually provide the functionality we want; will it return true if the limit switch is pressed, but the motor is not being told to move? In that case, the motor is arguably not being "limited." What about if the motor is moving in the other direction?

I also don't quite understand how having the limit switch getter respect the isNormallyClosed setting would make it less-useful as a generic digital input, either; if the user has cause to change the normallyClosed setting, they'll probably be negating the reading anyway. There's no loss of functionality.

ozrien commented 6 years ago

Limit switches A limit switch works by connecting the sense pin to ground. If the limit switch is connecting the pin to ground, the switch is "closed". If the switch is not connecting the sense pin to ground, it is "open" The "open" versus "close" state of the switch has nothing to due with NormallyOpen(NO) versus NormalyClosed(NC).

NO/NC is a setting to determine if the Talon should fault and neutral output when the respective switch is "open" versus "closed". So if you want the Talon to auto-off your motor output, it must know which state to look for. This is primarily a function of how you wire your limit switch. Most limit switches have COMMON, NC, and NO pins to make this easy.

Documentation here... image

And here... pin10 is ground... https://github.com/CrossTheRoadElec/Phoenix-Documentation/blob/master/README.md#setup-limit-switches

Additionally, it's not clear if this does actually provide the functionality we want; will it return true if the limit switch is pressed, but the motor is not being told to move? In that case, the motor is arguably not being "limited." What about if the motor is moving in the other direction?

isFwdLimitSwitchClosed => is the forward limit switch closed. Closed means pin is connected to ground - typically via a switch. This is the case regardless of how you config NormallyOpen versus NormallyClosed. The physical switch either connects the pin to ground or it doesn't (internally pulled up). This has nothing to do with the software limiting feature.

And we explain that it works even if limit switches are not enabled here... http://www.ctr-electronics.com/downloads/api/java/html/com/ctre/phoenix/motorcontrol/SensorCollection.html#isFwdLimitSwitchClosed--

I also don't quite understand how having the limit switch getter respect the isNormallyClosed setting would make it less-useful as a generic digital input, either; if the user has cause to change the normallyClosed setting, they'll probably be negating the reading anyway. There's no loss of functionality. A generic digital input is either voltage high or voltage low. "Closed" is unambiguously defined as low voltage. isFwdLimitSwitchClosed can be used to measure voltage high and voltage low.

  • If you are using the pin as a digital input, you generally don't care about the NC/NO, because there is no limiting. Call isFwdLimitSwitchClosed() to check the pin state. "closed" => logic low, this is unambiguous.
  • If you are using limiting, you can still call isFwdLimitSwitchClosed() to check the physical switch output.
  • If you just want to know if the motor is limiting, check the faults.