Fix Spi for EH1 alpha

[tommy-gilligan]

error[E0308]: mismatched types
   --> src/main.rs:72:9
    |
68  |     let mut spi = spi.init(
    |                       ---- arguments to this method are incorrect
...
72  |         &embedded_hal::spi::MODE_0,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `embedded_hal::spi::Mode`, found `Mode`
    |
    = note: `Mode` and `embedded_hal::spi::Mode` have similar names, but are actually distinct types

I was getting an error. Not sure if these are the right changes but fixes the issue for me. Potentially the feature conditionals could be tidied up.

[tommy-gilligan]

What is failing on CI here is not failing for me locally. Not sure what difference is causing it.

[ithinuel]

It is failing because the examples always/only use embedded-hal 0.2.7.
The eh1_0_alpha is not mean to replace eh 0.2.7 implementations/types with eh1_0 but add them to existing types.

So the issue here is a bit unfortunate.
A way to solve this would be to define an rp2040-hal specific Mode type, that implements From<embedded_hal::spi::Mode> and, when eh1_0_alpha is enabled, also From<eh1::spi::Mode>.
Then, the public method of rp2040-hal's Spi that expect a Mode should then be modified to be in the form pub fn foo<T: Into<rp2040-hal::spi::Mode>>(...) and let them do a mode.into() internally.

I'd probably made the rp2040-hal's Mode a struct Mode(embedded_hal::spi::Mode) (a newtype using the 0.2.7 mode) and make From<eh1::spi::Mode convert from 1.0 alpha mode to 0.2.7 mode.

It's a bit convoluted but that's the most "seamless"/non-breaking I can think of for users.

[ithinuel]

This is a good catch! however, this solution isn't going to work very well.
I left another comment to explain a bit more what's happening and one way of working around that issue.

[tommy-gilligan]

It is failing because the examples always/only use embedded-hal 0.2.7.
The eh1_0_alpha is not mean to replace eh 0.2.7 implementations/types with eh1_0 but add them to existing types.

Thank you so much for figuring that out! Makes sense to me now.

It's a bit convoluted but that's the most "seamless"/non-breaking I can think of for users.

I'll give it a go

[tommy-gilligan]

Introduces a clone

[ithinuel]

It would be a breaking change but 0.9.0 is planned to have loads anyway, so this can be made into:

    fn set_format<M: Into<Mode>>(&mut self, data_bits: u8, mode: M) {
        let mode: Mode = mode.into();

EDIT: actually we don't need the generic to go that deep in the API.
See the top level comment.

[tommy-gilligan]

Re-exporting embedded-hal's MODE_0, MODE_1 etc. might improve ease of use.

[ithinuel]

This looks like a great step forward!
The bounds for copy and clone can be avoided by using From<&eh1::spi::Mode>.
I don't know if that's a common/recommended practice.

Alternatively, the function may as well take the mode by value.

[tommy-gilligan]

Alternatively, the function may as well take the mode by value.

I prefer this but it'll be more of a breaking change.

It would be a breaking change but 0.9.0 is planned to have loads anyway, so this can be made into:

I spoke too soon haha

[ithinuel]

Actually, all the private methods (set_format, init_spi) can take a plain rp2040_hal::spi::Mode.

init and init_slave can take a generic M: Into<Mode> by value and do the required mode.into() as it calls the private method.

The Mode type is always going to be a fairly small type (about 2 bytes) so according to the AAPCS (IIRC) such type can be passed in a single register, so it is the same price as passing a reference.