rust-embedded / embedded-hal

A Hardware Abstraction Layer (HAL) for embedded systems
Apache License 2.0
2.01k stars 200 forks source link

Major faux I2C interface incompatibility between 0.1.2 and 0.2.0 #81

Closed therealprof closed 6 years ago

therealprof commented 6 years ago

Even though the blocking I2C interface has not changed a bit between 0.1.2 and 0.2.0 Rust will not compile any application using I2C where one dependency is using version 0.1.2 and the other 0.2.0. I've only seen this for I2C so far, other interfaces (I've examples for serial, GPIO and SPI) do not exhibit this behaviour.

This issue has been reported in https://github.com/jamwaffles/ssd1306/issues/66 but I've seen it myself in all of my crates now, too, e.g.

error[E0277]: the trait bound `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>: embedded_hal::blocking::i2c::WriteRead` is not satisfied
  --> examples/i2c_hal_ina260serial.rs:68:26
   |
68 |         let mut ina260 = INA260::new(i2c, 0x40).unwrap();
   |                          ^^^^^^^^^^^ the trait `embedded_hal::blocking::i2c::WriteRead` is not implemented for `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>`
   |
   = note: required by `<ina260::INA260<I2C>>::new`

error[E0277]: the trait bound `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>: embedded_hal::blocking::i2c::Write` is not satisfied
  --> examples/i2c_hal_ina260serial.rs:68:26
   |
68 |         let mut ina260 = INA260::new(i2c, 0x40).unwrap();
   |                          ^^^^^^^^^^^ the trait `embedded_hal::blocking::i2c::Write` is not implemented for `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>`
   |
   = note: required by `<ina260::INA260<I2C>>::new`

error[E0599]: no method named `voltage` found for type `ina260::INA260<hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>>` in the current scope
  --> examples/i2c_hal_ina260serial.rs:75:34
   |
75 |             let voltage = ina260.voltage().unwrap();
   |                                  ^^^^^^^
   |
   = note: the method `voltage` exists but the following trait bounds were not satisfied:
           `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)> : embedded_hal::blocking::i2c::WriteRead`
           `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)> : embedded_hal::blocking::i2c::Write`

error[E0599]: no method named `current` found for type `ina260::INA260<hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)>>` in the current scope
  --> examples/i2c_hal_ina260serial.rs:81:34
   |
81 |             let current = ina260.current().unwrap();
   |                                  ^^^^^^^
   |
   = note: the method `current` exists but the following trait bounds were not satisfied:
           `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)> : embedded_hal::blocking::i2c::WriteRead`
           `hal::i2c::I2c<hal::<unnamed>::I2C1, (hal::gpio::gpiof::PF1<hal::gpio::Alternate<hal::gpio::AF1>>, hal::gpio::gpiof::PF0<hal::gpio::Alternate<hal::gpio::AF1>>)> : embedded_hal::blocking::i2c::Write`

error: aborting due to 4 previous errors

After matching the versions to either 0.1.2 or 0.2.0 (or dropping I2C use) the same code will compile just fine. I don't see any reason why it shouldn't continue working, especially since only I2C seems to be affected and it would be great if we could somehow ease the transition.

@japaric Any ideas?

japaric commented 6 years ago

That's expected. v0.1.x traits from v0.1.x are different from v0.2.x traits even if they have the exact same signature (the traits live in different crates -- v0.1.x and v0.2.x are different crates). This was mentioned in the announcement and in https://github.com/japaric/embedded-hal/pull/72#issuecomment-388491531.

especially since only I2C seems to be affected

The problem affects all traits but it's only seen when a driver crate (generic over trait) and a HAL implementation (trait implementation) interact. You only see problems with I2C because probably you are using I2C drivers and not e.g. SPI or serial drivers.

therealprof commented 6 years ago

You only see problems with I2C because probably you are using I2C drivers and not e.g. SPI or serial drivers.

Nope, I'm only seeing it with I2C, not SPI, not Serial. Just verified again, cf. ssd1306 crate:

# cargo build --release --example blinky
    Finished release [optimized + debuginfo] target(s) in 0.09s
# cargo build --release --example blinky_i2c
   Compiling ssd1306 v0.1.0 (file:///Users/egger/OSS/ssd1306)
error[E0277]: the trait bound `blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>: hal::prelude::Write` is not satisfied
   --> examples/blinky_i2c.rs:97:1
    |
97  | / fn draw_square(disp: &mut OledDisplay, xoffset: u32, yoffset: u32) {
98  | |     // Top side
99  | |     disp.set_pixel(xoffset + 0, yoffset + 0, 1);
100 | |     disp.set_pixel(xoffset + 1, yoffset + 0, 1);
...   |
120 | |     disp.set_pixel(xoffset + 0, yoffset + 3, 1);
121 | | }
    | |_^ the trait `hal::prelude::Write` is not implemented for `blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>`
    |
    = note: required because of the requirements on the impl of `ssd1306::interface::DisplayInterface` for `ssd1306::interface::I2cInterface<blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>>`
    = note: required by `ssd1306::mode::GraphicsMode`

error[E0277]: the trait bound `blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>: hal::prelude::Write` is not satisfied
  --> examples/blinky_i2c.rs:45:1
   |
45 | / app! {
46 | |     device: blue_pill::stm32f103xx,
47 | |
48 | |     resources: {
...  |
58 | |     },
59 | | }
   | |_^ the trait `hal::prelude::Write` is not implemented for `blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>`
   |
   = note: required because of the requirements on the impl of `ssd1306::interface::DisplayInterface` for `ssd1306::interface::I2cInterface<blue_pill::i2c::I2c<blue_pill::<unnamed>::I2C1, (blue_pill::gpio::gpiob::PB8<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>, blue_pill::gpio::gpiob::PB9<blue_pill::gpio::Alternate<blue_pill::gpio::OpenDrain>>)>>`
   = note: required by `ssd1306::mode::GraphicsMode`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0277`.
error: Could not compile `ssd1306`.

To learn more, run the command again with --verbose.
austinbes commented 6 years ago

IDK if this is the issue here, but I remember a major crate (serde) dealing with similar issues when they released 1.0 with breaking changes.

If I remember correctly, there was a write-up about the solution they used. It was something like making a last release on the previous series which stripped out elements that were unchanged, and instead depended on those same elements from the next release version. There was a nice write-up about this -- I'm trying to find it, but haven't had much luck yet.

I apologize if this is something you are aware of already that doesn't apply -- I'm fairly new to the wide world of rust library compatibility and versioning.

austinbes commented 6 years ago

Here's the writeup I was thinking of, at least: https://github.com/dtolnay/semver-trick

japaric commented 6 years ago

@austinbes Thanks for the link. That's a clever trick. I think it can be applied in this case.

I'll release v0.1.3 using the trick and we can see how it goes.

japaric commented 6 years ago

embedded-hal v0.1.3 has been released. Experience reports using it in both sides HAL impl / drivers would be appreciated.

therealprof commented 6 years ago

Hm, it doesn't help in my specific ssd1306 case but I'll try a few more cases later.

therealprof commented 6 years ago

Hah! I take it back, seems to work a treat on ssd1306, too. There was another conflicting change in the used branch of the stm32f103xx-hal fork so that needed a pin.

ilya-epifanov commented 6 years ago

@therealprof the changes needed are described here: https://github.com/japaric/stm32f103xx-hal/pull/50#issuecomment-387765643

therealprof commented 6 years ago

@ilya-epifanov Yeah, I'm not so happy about using a crate from a forked github repository for examples anyway but it is what it is.

jamwaffles commented 6 years ago

For the record neither am I, but it means I/we can develop on actual hardware which is a big plus.

On Tue, 15 May 2018, 12:48 Daniel Egger, notifications@github.com wrote:

@ilya-epifanov https://github.com/ilya-epifanov Yeah, I'm not so happy about using a crate from a forked github repository for examples anyway but it is what it is.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/japaric/embedded-hal/issues/81#issuecomment-389138396, or mute the thread https://github.com/notifications/unsubscribe-auth/AAw4F-EbhTmhVdIZLn-pGufWZDqtwcgbks5tysCFgaJpZM4T89NT .

dbrgn commented 6 years ago

This can probably be closed?

therealprof commented 6 years ago

@dbrgn Yes, thanks for the reminder.