OutputPort proposal

[sajattack]

trafficstars

Shamefully stolen from @japaric's issue #30.

I needs for my parallel LCDs.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ryankurte (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.

[sajattack]

Here's a sample implementation: (imagine it on this line of atsamd-hal https://github.com/atsamd-rs/atsamd/blob/2e71c08a952f9e5b1c1137a1277b8f61c14e9b04/hal/src/gpio.rs#L423)

impl OutputPort<u32> for Port {
    fn write(&mut self, word: u32) {
        unsafe {
            (*PORT::ptr()).outset.write(|bits| {
                bits.bits(word)
            });
        }
    }
}

[ryankurte]

hey thanks for the PR! looks pretty good to me.

the only question i have is how we define / configure buses of widths that are not an existing (standard integer width) type, and can this be made clear in the interface and/or documentation?

(cc. #30)

[therealprof]

We talked about this a bit on IRC earlier today. I think there're few other problems with the comments and I'd certainly would like to see a slightly more complete impl doing something other than simply exposing the full set register...

[sajattack]

Maybe we could have a mask parameter, or use the uX crate?

[luojia65]

https://github.com/rust-embedded/embedded-hal/issues/30#issuecomment-557439768 https://github.com/rust-embedded/embedded-hal/issues/30#issuecomment-557921314 Should the width be an associated type parameter in trait other than in generic type bound? That could be a more reasonable design as typically the chip only allow one width type for one port (as far as I know).

[burrbull]

I've made testing implementation of OutputPort: https://github.com/burrbull/embedded-hal/tree/outport rebased on 0.2.x + const generic port size (N) https://github.com/burrbull/hd44780-driver/commit/cc8861bd5f8ea352472b9f42b88175f395b844a9 - using in hd44780-driver https://github.com/stm32-rs/stm32f4xx-hal/pull/426 - OutputPort<N, u8> implementation example for F4 cc @eldruin @therealprof

[eldruin]

Thanks for your test implementation @burrbull. For reference, here is the code:

/// A digital output "port"
///
/// `N` is number of pins in "port"
///
/// `Width` is the size of the data; it could be `u8` for an 8-bit parallel
/// port, `u16` for a 16-bit one, etc.
///
/// **NOTE** The "port" doesn't necessarily has to match a hardware GPIO port;
/// it could for instance be a 4-bit ports made up of non contiguous pins, say
/// `PA0`, `PA3`, `PA10` and `PA13`.
#[cfg(feature = "unproven")]
pub trait OutputPort<const N: u8, Width> {
    /// Error type
    type Error;
    /// Outputs `word` on the port pins
    ///
    /// # Contract
    ///
    /// The state of all the port pins will change atomically ("at the same time"). This usually
    /// means that state of all the pins will be changed in a single register operation.
    fn write(&mut self, word: Width) -> Result<(), Self::Error>;
}

A few questions/remarks:

1. The unconstrained Width parameter will be a problem for interoperability.
2. What do do when N and the number of bits of Width differ?
3. What to do with widths that do not match primitive types like u8/u16... ?
4. Little-endian vs big-endian words.
5. How to send several words back-to-back?
6. While this allows for HAL implementations to define OutputPorts, it is not possible for the user to define these. e.g. it is not possible for the user to get a 4-bit OutputPort made of PA0, PA3, PA10 and PA13 like stated in the docs if the HAL impl does not provide it as such. However, HALs can only provide so many pin combinations and OutputPort sizes without falling into macro madness.

[burrbull]

What if limit trait to u8 only? Just delete Width.

[burrbull]

6. While this allows for HAL implementations to define OutputPorts, it is not possible for the user to define these. e.g. it is not possible for the user to get a 4-bit OutputPort made of PA0, PA3, PA10 and PA13 like stated in the docs if the HAL impl does not provide it as such. However, HALs can only provide so many pin combinations and OutputPort sizes without falling into macro madness.

If you have looked at implementation example, you could see that it support all combinations of pins located on 1 real port.

[burrbull]

@eldruin Look at this please:

#[cfg(feature = "unproven")]
pub trait OutputPort<const N: usize> {
    /// Error type
    type Error;
    /// Outputs `word` on the port pins
    ///
    /// # Contract
    ///
    /// The state of all the port pins will change atomically ("at the same time"). This usually
    /// means that state of all the pins will be changed in a single register operation.
    fn write(&mut self, word: u8) -> Result<(), Self::Error>;
}

#[cfg(feature = "unproven")]
impl<PIN, const N: usize> OutputPort<N> for [PIN; N]
where
    PIN: OutputPin,
{
    type Error = PIN::Error;
    fn write(&mut self, word: u8) -> Result<(), Self::Error> {
        for i in 0..N {
            if word & (1 << i) != 0 {
                self[i].set_high()?;
            } else {
                self[i].set_low()?;
            }
        }
        Ok(())
    }
}

and for tuples:

macro_rules! port_tuple {
    ($N:literal, ($($i:literal),+), ($($P:ident),+), ($($b:ident),+)) => {
        impl<$($P),+, E> OutputPort<$N> for ($($P),+)
        where
            $($P: OutputPin<Error=E>),+,
        {
            type Error = E;
            fn write(&mut self, word: u8) -> Result<(), Self::Error> {
                let ($($b),+) = self;
                $(
                    if word & (1 << $i) != 0 {
                        $b.set_high()?;
                    } else {
                        $b.set_low()?;
                    }
                )+
                Ok(())
            }
        }
    }
}

port_tuple!(2, (0, 1), (P0, P1), (b0, b1));
port_tuple!(3, (0, 1, 2), (P0, P1, P2), (b0, b1, b2));
port_tuple!(4, (0, 1, 2, 3), (P0, P1, P2, P3), (b0, b1, b2, b3));
port_tuple!(
    5,
    (0, 1, 2, 3, 4),
    (P0, P1, P2, P3, P4),
    (b0, b1, b2, b3, b4)
);
port_tuple!(
    6,
    (0, 1, 2, 3, 4, 5),
    (P0, P1, P2, P3, P4, P5),
    (b0, b1, b2, b3, b4, b5)
);
port_tuple!(
    7,
    (0, 1, 2, 3, 4, 5, 6),
    (P0, P1, P2, P3, P4, P5, P6),
    (b0, b1, b2, b3, b4, b5, b6)
);
port_tuple!(
    8,
    (0, 1, 2, 3, 4, 5, 6, 7),
    (P0, P1, P2, P3, P4, P5, P6, P7),
    (b0, b1, b2, b3, b4, b5, b6, b7)
);

[thejpster]

How does your write function upload the contract:

The state of all the port pins will change atomically ("at the same time"). This usually means that state of all the pins will be changed in a single register operation.

[burrbull]

How does your write function upload the contract:

The state of all the port pins will change atomically ("at the same time"). This usually means that state of all the pins will be changed in a single register operation.

It is just dumb implementation above for HALs which don't have native support of OutputPort.

More relevant implementation example is in https://github.com/stm32-rs/stm32f4xx-hal/blob/d94044188defb82583bd561fe8fb3363932454cb/src/gpio/outport.rs

[eldruin]

Thanks for the simplified macro here @burrbull. I understood now how it solves the combination problem nicely. This example implementation works for N <= 8 and uses the word least-significant bits up to N in that order. I guess little-endian vs big-endian word problem is also solved by this macro, since the user can just put the pins in in the the reverse or whatever order in the input tuple/array.

These other questions remain:

1. What about ports with more than 8 bits?
2. What to do when N and the number of bits of Width differ?

• I am fine with just documenting that the lowest N number of bits will be taken from the input word. Maybe somebody else has a problem with this?
• However, what happens if N is bigger than the number of word bits? Can we ensure this does not happen?

3. How to send several words back-to-back?

• Probably out of scope here except if we want to take a word array, or rather unnecessary, but I think it is still worth discussing.
• Does anybody have an use case for this?

It should be noted that we have generic Word types elsewhere as well. e.g. in the SpiBus/SpiDevice traits. There we simply require the type to be Copy and provide u8 as default type. We can choose a similar approach here where we have a Word parameter, that is often simply u8, but HALs can also support stuff like u16. Doing that would solve 1.

[burrbull]

1. What about ports with more than 8 bits?

This is main question. But what is real use-case? Even if we solve endianess questions (which is not trivial), how to implement it considering "at the same time"? Pins must be on 1 real port for this. I can imagine only specialized chips (like shift-regiters) in this role. Also ports in microcontrollers have limitations on power. Maybe I missing something important?

[eldruin]

An example of a device that exposes 16 IO pins which can be driven at the same time is the classic I2C PCF8575. (The datasheet talks about two ports because the data for them is sent in two bytes but this note is irrelevant here.) It drives the 16 pins at once after acknowledging the second byte of data. I do not know how common this is, though. This device is also an example of a device that can receive back-to-back transmissions. i.e. it receives any (even) number of bytes and after each second byte, drives the 16 output pins. This seems to be pretty common.