Skip to content
This repository has been archived by the owner on May 15, 2021. It is now read-only.

GPIO.split() claims to produce PINnn<Input<Floating>> but doesn't enable the input buffer #20

Open
mattheww opened this issue Apr 7, 2019 · 1 comment

Comments

@mattheww
Copy link
Contributor

mattheww commented Apr 7, 2019

GPIO.split() returns a Parts struct, whose members have types like PIN0<Input<Floating>>:
https://docs.rs/nrf51-hal/0.6.2/nrf51_hal/gpio/gpio/struct.Parts.html

This means its members claim to implement the embedded_hal::digital::InputPin trait, and so the compiler will let you call is_high() and is_low() on them.

But split() doesn't do anything to enable the input buffer for each pin (which is disabled by default), so these calls won't return meaningful information until you explicitly call one of the .into_xxx_input() methods.

In general the typestate system for these PIN types means the compiler prevents you making mistakes like this, so I think it would be better to either

  • have split() reconfigure all the pins as floating input; or
  • have a third mode for the PIN types which doesn't implement either input or output traits, and have Parts use those types.

The second change isn't fully backwards compatible, but if it makes a program stop compiling it's probably revealing a bug.

There's also a weakness that nothing stops you arbitrarily configuring the GPIO pins before calling split(), leading to another way for the typestates to get out of sync with the hardware. Either of the changes above would remove that weakness too.

@Nemo157
Copy link
Contributor

Nemo157 commented Apr 8, 2019

I took the second approach while experimenting with alternative typestate implementations in embrio-nrf51 (which was strongly inspired by nrf51-hal's implementation). It definitely seems like the correct solution to me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants