Miscellaneous improvements

[sajattack]

• better error typing
• more docs
• make buffer size const generic

[jbeaurivage]

I have two minor comments, but otherwise this looks pretty good!

[jbeaurivage]

I would recommend implementing From<embedded_hal::digital::Error> for Error (likewise for embedded_hal::spi::Error) to avoid manually mapping errors everywhere.

[sajattack]

I'm having some trouble figuring out how to do this with the generics on the Error. I think they help avoid dyn

[jbeaurivage]

Yeah, looks like you can't have both From implementations because they would overlap if SE == GE. In that case I'd recommend only implementing From<GE> for Error<SE, GE>, and using map_err for the one occurrence where Error::Spi appears :)

[jbeaurivage]

Although it might be beneficial to change the error type to this:

/// Errors
#[derive(Clone, Copy, Debug)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum Error<SE, GE>
where
    SE: embedded_hal::spi::Error,
    GE: embedded_hal::digital::Error,
{
    Spi(SE),
    Gpio(GE),
}

[ianrrees]

Hmm, yeah this is awkward because the SPI and GPIO error types could be the same...

I'd suggest not making Error generic across the types, instead have the Spi variant contain an spi::ErrorKind and the Gpio variant contain a digital::ErrorKind.

Then, a couple functions could tell the compiler how to interpret the results of calls that return the generic SPI/GPIO Results:

enum Error {
    Spi(spi::ErrorKind),
    Gpio(digital::ErrorKind),
}

/// Translates digital Result types to ours
fn digital<T, D: digital::Error>(digital_res: Result<T, D>) -> Result<T, Error> {
    digital_res.map_err(|e| Error::Gpio(e.kind()))
}

/// Translates SPI Result types to ours
fn spi<T, S: spi::Error>(spi_res: Result<T, S>) -> Result<T, Error> {
    spi_res.map_err(|e| Error::Spi(e.kind()))
}

// This way, you can do something like
fn demo_method(&mut self) -> Result<(), Error> {
    digital(rst.set_high())?;
}

[jbeaurivage]

I don't necessarily think one approach is better than the other, but the users could fine-tune the buffer size more if the *_buffered methods took the const generic parameter instead of the top-level struct. Also, users that don't want to use the buffered methods wouldn't have to specify a fake buffer size just to satisfy the API.

[sajattack]

Yeah I was considering that, but there were some weird dependency issues between methods calling eachother. I'll look at it more carefully and report back later.

[sajattack]

there's no way to add the generics to the function signature of the embedded_graphics functions afaict

[jbeaurivage]

True, then it seems like this suggestion isn't viable. Apologies!

[jbeaurivage]

Oh also I'd suggest optionally supporting defmt to display your error type