TriStatePin proposal

[osannolik]

I have simply added the enum variant as per the proposal in #31, since it seemed to be the most popular.

I am in the middle of implementing a charlieplexing crate for use in a project of mine (custom board using an stm32l051) and would therefore need this trait.

To try it out I implemented the trait for the stm32l0xx-hal in a branch together with an example application, and got the charlieplexing working as a proof of concept on my custom board driving 30 leds.

The charlieplexing crate is far from done, but I think the trait is going to work great, so to get the wheels spinning and allow for more opinions, I thought I might as well open a pull request right away...

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thejpster (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[thejpster]

This seems straightforward enough, but I wonder if it needs an RFC before we go to unproven implementation.

@jamesmunns can you stick this on the agenda for Tuesday?

[thejpster]

Lol. I'm an idiot. There is an RFC! You even linked to it...

Still, I'd like to discuss this on Tuesday as I'm not quite sure of the process for merging.

[osannolik]

So, what is the status on this? Have there been any discussions? It would be great to have this on master.

[ryankurte]

we've just landed IoPin #269 which lets one toggle between input and output pin type-states, do we still need a TriStatePin or is does #269 cover the use cases for this okay?

(i feel like the type-state approach is possibly less ergonomic than that proposed here, but it should be pretty easy to implement a more convenient wrapper over it with an API like this)

cc. @eldruin

[ExplodingWaffle]

Hello- Maybe I'm missing something, but I think I've bumped into a usecase that requires this or something like it. Threw together a quick example:

enum Things {
    Thing1,
    Thing2,
    Thing3,
}

let there_is_no_tri = pins.gpio13.into_floating_input();

let thing_closure = |thing: Things| -> () {
    match thing {
        Things::Thing1 => { there_is_no_tri.into_floating_input(); () }
        Things::Thing2 => there_is_no_tri.into_push_pull_output().set_low().unwrap(),
        _ => unimplemented!(),
    };
};

thing_closure(Things::Thing1);
thing_closure(Things::Thing2);

This code produces an error:

error[E0382]: use of moved value: `thing_closure`
   --> src\main.rs:245:9
    |
244 |         thing_closure(Things::Thing1);
    |         ----------------------------- `thing_closure` moved due to this call
245 |         thing_closure(Things::Thing2);
    |         ^^^^^^^^^^^^^ value used here after move
    |
note: closure cannot be invoked more than once because it moves the variable `there_is_no_tri` out of its environment
   --> src\main.rs:236:21
    |
236 |                     there_is_no_tri.into_floating_input();
    |                     ^^^^^^^^^^^^^^^
note: this value implements `FnOnce`, which causes it to be moved when called
   --> src\main.rs:244:9
    |
244 |         thing_closure(Things::Thing1);
    |         ^^^^^^^^^^^^^

Since into_floating_input takes self rather than &self or &mut self, the closure moves the variable out of the environment and it isn't reentrant (god bless rustc for making that error so self-explanatory :D ). IoPin has the same problem of taking self.
I imagine there are many other uses (drivers?) that also have problems with this approach.

Just as an example of something that DOES currently work:

let mut boring_pin = pins.gpio13.into_push_pull_output();

let mut thing_closure = |thing: Things| -> () {
    match thing {
        Things::Thing1 => boring_pin.set_high().unwrap(),
        Things::Thing2 => boring_pin.set_low().unwrap(),
        _ => unimplemented!(),
    };
};

thing_closure(Things::Thing1);
thing_closure(Things::Thing2);

With this change, I'm pretty sure I could do this, and it would work fine.

let mut tristate_pin = pins.gpio13.into_tristate_output();

let mut thing_closure = |thing: Things| -> () {
    match thing {
        Things::Thing1 => boring_pin.set( PinState::Floating ).unwrap(),
        Things::Thing2 => boring_pin.set( PinState::Low).unwrap(),
        _ => unimplemented!(),
    };
};

thing_closure(Things::Thing1);
thing_closure(Things::Thing2);

One thing to note is that PinState already exists now because of IoPin, so you'd have to use some other enum (and perhaps have set_low(), set_high(), set_floating() as well).
(sorry, that was a lot of code but I hope it got the point across)