Embedded-hal: OutputPin trait usability -- add set_value

[YonasJ]

Right now the v2 ouput pin looks like this:

/// Single digital push-pull output pin
pub trait OutputPin {
    /// Error type
    type Error;

    /// Drives the pin low
    ///
    /// *NOTE* the actual electrical state of the pin may not actually be low, e.g. due to external
    /// electrical sources
    fn set_low(&mut self) -> Result<(), Self::Error>;

    /// Drives the pin high
    ///
    /// *NOTE* the actual electrical state of the pin may not actually be high, e.g. due to external
    /// electrical sources
    fn set_high(&mut self) -> Result<(), Self::Error>;
}

This assumes 2 methods called set_low and set_high are how people want to interact with it. I think it might be nice to offer some flexibility of also having a set or set_value method that accepts a boolean.

The reason I think that this might be helpful is that when using the OutputPin, I nearly always already have a bool, so I always write the following code:

if val_to_set {x.set_high()?;} else {x.set_low()?;}

And this is probably affecting a lot of users.

Maybe this issues has already been beaten to death, in which case, I am sorry. I looked and could not find any other discussion on this.

For the InputPin, there is no need for get_value(), as is_high() is essentially get_value(), and is_low() essentially returns the invert of the value.

Any concerns with this idea? If no, I could put in a pull request.

[ryankurte]

The reason I think that this might be helpful is that when using the OutputPin, I nearly always already have a bool, so I always write the following code...

I too do this a lot. I _think_ the justification for this was that it doesn't require a specific type or for physical meaning to be ascribed to a boolean, but I can't find the receipts. I personally wouldn't be opposed to an additional method (set_val(&mut self, val: bool) or something) on the trait with default implementation using set_high/set_low internally. Thoughts @rust-embedded/hal ?

[burrbull]

It's better to have set_state with

enum PinState {
    Low,
    High,
}

in embedded-hal instead of bool. Maybe with From<bool> and ops::Neg implementation.