rust-embedded/embedded-hal

[RFC] `digital::TriStatePin`

japaric opened this issue · 14 comments

What the title says. The use case for this is some sort of generic charlieplexing interface / driver / library.

Alternatives

Vanilla

/// A tri-state (low, high, floating) pin
pub trait TriStatePin {
    /// Drives the pin low
    fn set_low(&mut self);

    /// Drives the pin high
    fn set_high(&mut self);

    /// Puts the pin in floating mode
    fn float(&mut self);

    /// Checks if the pin is currently in floating mode
    fn is_floating(&self) -> bool;

    // plus maybe other `is_*` methods
}

Enum based

/// The states of a tri-state pin
enum State { Low, High, Floating }

/// A tri-state (low, high, floating) pin
pub trait TriStatePin {
    /// Changes the state of the pin
    fn set(&mut self, state: State);

    /// Gets the state of the pin
    fn state(&self) -> State;
}

Once we pick an alternative that we are happy with we can land it behind the "unproven" feature gate. Once someone has demonstrated that it works by actually implementing the trait and building some generic library on top of it we can un-feature gate it.

cc @therealprof

I have a library that could use this (dw1000 driver).

I think I like the second alternative better (enum based), though we would need a "get state" (output/input), and "get value" (low or high, could work for input or output).

Nevermind, I see that this isn't meant to cover all states, just "Output with floating". I like the enum based approach even more.

bachp commented

+1 for the enum based variant, it seems easier to comprehend

Enum approach is more pleasing. One concern is the "state" method could be confused for an InputPin read instead of the TriStatePin's set output.

Whoops, commented on so topics about this very thing that I forgot to raise my voice here. 😉

+1 for enum based, too.

I think a way to read the pin's state is critical -- with it, for instance, you could build a portable bit-banged I2C implementation on top of a TriStatePin.

What if the API looked something like:

/// The states of a tri-state pin
enum State { Low, High, Floating }

/// A tri-state (low, high, floating) pin
pub trait TriStatePin {
    /// Changes the state of the pin
    fn set(&mut self, state: State);

    /// Gets the output state of the pin
    ///
    /// Queries the state set in `set()`
    fn state(&self) -> State;

    /// Whether the pin is driven high
    ///
    /// Reads the pin's actual electrical state
    fn is_high(&self) -> bool;

    /// Whether the pin is driven low
    ///
    /// Reads the pin's actual electrical state
    fn is_low(&self) -> bool;
}

The other option would be for TriStatePin implementers to also implement InputPin -- does that make more sense from a modularity perspective? It doesn't match what's done with an OutputPin, where the one trait encompasses all pin operations.

@austinglaser TriStatePin does not let you use the pin as an input. State::Low means the output level is low, State::High means that the output level is high, and State::Floating leaves the output floating but you can't read the voltage level when it is in that state.

What you want for a bit banged I2C implementation is the IoPin trait proposed in #29; that trait lets you toggle between the InputPin and OutputPin modes.


There seems to be consensus here on the enum based API so feel free to send a PR.

@japaric I see what you're going for in principle here. I do think that there needs to be some discussion about reading the state of an output pin, however. Depending on the pin configuration in hardware (consider, for instance, an open-drain output), it can be critical to know the electrical state of a pin that is being "driven" high or low.

@austinglaser Not every MCU will allow to read back the state of an output pin so you might have to switch modes anyway (or store the state and hope it'll be the current one).

@therealprof Indeed... but some do. Maybe the right place isn't in TriStatePin, but there should be some way at the trait level to express that a particular pin can be meaningfully read while in output mode

That's a compelling idea. My first thought, though, is: "how do you read back something like a TriState pin?" Do you have a separate impl TriStateReadbackPin: TriStatePin trait? This would cause an explosion of Readback traits, for each possible type of output

What about a trait just called ReadbackPin, so you can express a trait bound such as OutputPin + ReadbackPin or TrStatePin + ReadbackPin?

Although that's starting to look a lot like (exactly like?) an InputPin -- maybe it's more of an implementation level thing, which encourages (where possible) the implementation of InputPin for OutputPins. Then, the trait bound looks like OutputPin + InputPin or TriStatePin + InputPin

What I've been talking about is actually reading the electrical state of an output pin. For cases where that makes sense (as I've said before, one such example is a pin configured in open-drain output mode), the implementation of InputPin could be the identical trait -- not a different one with the same name. I agree wholeheartedly that that would be a bad idea.