IoPin trait not practical to implement with multiple input/output types

[davidlattimore]

The IoPin trait - added in #29, seems like it would be impractical to implement in a system with multiple input / output pin types. The STM32 HALs that I'm familiar with (g0, l4, h7) use different types for OpenDrain and PushPull pin types. Because IoPin is parameterized on both input and output pin types, it appears that you would need to implement every combination NxM (where N is the number of input types and M is the number of output types).

Is there a reason why it's one trait rather than two? I guess having a single trait can be good if you want to construct a trait object - although I don't think you can do that in this case anyway, since the methods take self by value.

The only implementation I managed to find was in linux-embedded-hal which seems to only have a single type for pins.

[Dirbaio]

There's one in stm32f4xx-hal. I agree the trait is quite unwieldy, yes: https://github.com/stm32-rs/stm32f4xx-hal/blob/dc1552b2fc2e14901203a13ab42e76a5121a08eb/src/gpio.rs#L574-L683

It also feels strange to me TInput, TOutput are trait generic params instead of associated types.

Also, taking self interacts badly with having driver structs own the pin, because it changes type on the fly so you have to store an enum in the struct to store all the possible states...

I wonder if it'd be better to have a trait for a pin that you can switch at runtime without changing type.

/// A pin that can switch between input and output modes at runtime
pub trait IoPin: InputPin + OutputPin {
    /// Configures the pin to operate in input mode
    fn as_input(&mut self);
    /// Configures the pin to operate in output mode
    fn as_output(&mut self)
}

[ryankurte]

I wonder if it'd be better to have a trait for a pin that you can switch at runtime without changing type.

this seems preferable to me, maybe with set_(input|output) and is_(input|output)?

i tried to do some trickier type-based stuff but, i think there's elegance in this simplicity

[kelnos]

I've run into some difficulties with IoPin as well, but from a different angle: a crate that uses an IoPin and wants to hold onto it for the life of a struct.

Initially I implemented the struct (simplified) as

pub struct Device<E, P>
where
    E: core::fmt::Debug,
    P: InputPin<Error = E> + OutputPin<Error = E>,
{
    pin: P,
}

But then while using linux-embedded-hal, I found that the Linux gpiochip interface doesn't allow you to have a pin handle that is both an input and output at the same time, and has to be explicitly switched back and forth (unlike the ESP32 I'm also using for this project). So I found IoPin, but I'm at a loss for how to do this in a reasonable way.

I've switched to (again, simplified):

pub struct Device<E, IP, OP>
where
    E: core::fmt::Debug,
    IP: InputPin<Error = E> + IoPin<IP, OP, Error = E>,
    OP: OutputPin<Error = E> + IoPin<IP, OP, Error = E>,
{
    pin: Option<OP>,  // need to use Option so I can easily pull the pin out of the struct to change its mode
}

And then using it looks something like this:

impl<E, IP, OP> Device<E, IP, OP>
where
    ... // stuff
{
    pub fn read<D: Delay<Error = E>>(&mut self, delay: &D) -> Result<Reading, MyError<E>> {
        let mut pin = self.pin.take().unwrap();
        pin.set_low()?;
        delay.delay_us(500)?;
        pin.set_high()?;
        delay.delay_us(500)?;

        let pin = pin.into_input_pin()?;
        let reading = {
            // a bunch of `is_high()` and `is_low()` and delays that builds some sort of sensor reading
       };

        let pin = pin.into_output_pin()?;
        self.pin = Some(pin);

        Ok(reading)
    }
}

Ok, so on the face of it that's not terrible, but what happens if there is an error anywhere? Welp, the pin gets dropped and is now gone for good, and if someone calls read() again it'll panic. So now I have to get rid of all the nice, easy ? error handling and manually check for error states and re-set the pin in self if there's an error before bailing.

I think the proposed alternative in #340 (comment) is nice from the perspective of simplicity, but the big downside is that the IoPin impl would still expose the set_high() and set_low() functions while the pin is in input mode. Another option:

trait IoPin<E, TInput, TOutput>
where
    E: core::fmt::Debug,
    TInput: InputPin<Error = E>,
    TOutput: OutputPin<Error = E>,
{
    type Error: core::fmt::Debug;
    fn as_input_pin(&self) -> Result<TInput, E>;
    fn as_output_pin(&self) -> Result<TOutput, E>;
}

The idea here is that the two as_ functions would return a completely different struct that implements either the input or output pin functionality. The issue here is that there is nothing keeping you from calling (for example) as_output_pin() while there's an instance of the input pin still alive (which could return an error, but it's a shame to force something to be a runtime error if we can make it a compile time error). I guess perhaps this could work too:

    fn as_input_pin<'a>(&'a mut self) -> Result<&'a TInput, E>;
    fn as_output_pin<'a>(&'a mut self) -> Result<&'a mut TOutput, E>;

The idea there would be to keep the mutable borrow on the IoPin open for the lifetime of the input/output pin impl. Downside here is that makes things a bit harder on implementors, as they'd have to stash the input/output struct instance inside self. Not completely terrible, though.

[Finomnis]

I also have doubts about IoPin. Questions that are unclear to me:

• What does it mean to switch between Input and Output? Is it expected that switching to Input will disable the output, or just enable read capability? (nrf chips, for example, can read/write simultaneously, and for energy consumption the input part can be switched off)
• Is it expected that pin configs that are already InputPin + OutputPin implement IoPin<Self, Self>? (This would mean that the output should not necessarily be disabled during read.)
• Should OpenDrain pin configs switch to their passive state during reads (if they implement IoPin<Self, Self>)?
• If objects are InputPin + OutputPin and also implement IoPin<Self, Self>, one could theoretically call .try_into_input_pin(), followed by .set_high(). How should this behave?

I know it now got removed for 1.0.0, but if it ever gets reintroduced, a thorough explanation what the expected pin behaviour is would absolutely be necessary. But I feel like this trait just introduces way too many corner cases and ambiguities to be re-introduced.

[rumatoest]

I've tried to implement a HAL based drive once and stuck into an issue that some MCU (AVR for example) are not supporting open/drain pins and similar stuff. But still you could communicate with some sensors via single pin by simply switching between input/output modes like it done for Arduino (see DHT11 DHT22).

In order to do it using HAL I need to have some interface that would not consume itself while toggling between input/output.

The thing here is that implementing InputPin + OutputPin trait for pins is not a mandatory. Which means that driver developers could not rely that InputPin + OutputPin will be in every HAL implementation for different MCU. Thus such drivers would not work everywhere while it is possible to force such behavior in HAL by introducing something like IoPin trait that must be implemented.