esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
728 stars 202 forks source link

When using I2C + shared bus + embassy, if there is no slave device at the accessed address, subsequent I2C operations will send the same signal repeatedly. #1790

Closed IvanLi-CN closed 1 month ago

IvanLi-CN commented 3 months ago

I am just a hobbyist, so my description might not be very clear. Please bear with me.

My program runs on an ESP32-C3, and I use i2c0. When I alternately access devices at two different addresses in two embassy tasks, if one of the slave devices is offline, the signals sent by the other task to the I2C bus always have the same address as the former, instead of the one specified in the code.

Here is a snippet of my code:

#[embassy_executor::task]
async fn run_i2c_a(
    dev: &'static mut I2cDevice<'static, CriticalSectionRawMutex, I2C<'static, I2C0, Async>>,
) {
    let addr = 0x0a;

    log::info!("run_i2c addr: 0x{:02x}", addr);
    loop {
        Timer::after(Duration::from_millis(1000)).await;
        match dev.write(addr, &[0x11]).await {
            Ok(_) => {
                log::info!("set addr: 0x{:02x}", addr);
            }
            Err(err) => {
                log::error!("failed to set addr: 0x{:02x}. {:?}", addr, err);
            }
        }

    }
}

#[embassy_executor::task]
async fn run_i2c_b(
    dev: &'static mut I2cDevice<'static, CriticalSectionRawMutex, I2C<'static, I2C0, Async>>,
) {
    let addr = 0x0b;

    log::info!("run_i2c addr: 0x{:02x}", addr);
    loop {
        Timer::after(Duration::from_millis(1000)).await;
        match dev.write(addr, &[0x11]).await {
            Ok(_) => {
                log::info!("set addr: 0x{:02x}", addr);
            }
            Err(err) => {
                log::error!("failed to set addr: 0x{:02x}. {:?}", addr, err);
            }
        }

    }
}

What I expect is that if one or both of 0x4a and 0x4b are offline, the tasks can still alternately attempt to access them (I am hoping to access the slave device when it comes back online after being disconnected for some reason). Currently, both 0x4a and 0x4b are nonexistent devices. The signal I sniffed with a logic analyzer is as shown below, where it continuously tries to access 0x4a. From the log outputs I added to the code, I can confirm that the code runs alternately, but the signal sent is stuck on the same value.

image

espflash-monitor log:

INFO - starting
INFO - run_i2c addr: 0x4b
INFO - run_i2c addr: 0x4a
ERROR - failed to set addr: 0x4b. I2c(AckCheckFailed)
ERROR - failed to set addr: 0x4a. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4b. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4a. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4b. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4a. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4b. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4a. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4b. I2c(ExecIncomplete)
ERROR - failed to set addr: 0x4a. I2c(ExecIncomplete)

...

i2c log:

1958420-1958490 I²C: Address/Data: Address write: 4B
1958409-1958409 I²C: Address/Data: Start
1958490-1958500 I²C: Address/Data: Write
1958500-1958510 I²C: Address/Data: NACK
1958517-1958517 I²C: Address/Data: Stop
1964852-1964852 I²C: Address/Data: Start
1964863-1964933 I²C: Address/Data: Address write: 4A
1964933-1964943 I²C: Address/Data: Write
1964943-1964953 I²C: Address/Data: NACK
1964960-1964960 I²C: Address/Data: Stop
2964900-2964900 I²C: Address/Data: Start
2964911-2964981 I²C: Address/Data: Address write: 4A
2964981-2964991 I²C: Address/Data: Write
2964991-2965001 I²C: Address/Data: NACK
2965008-2965008 I²C: Address/Data: Stop
2965301-2965301 I²C: Address/Data: Start
2965311-2965381 I²C: Address/Data: Address write: 4A
2965381-2965391 I²C: Address/Data: Write
2965391-2965401 I²C: Address/Data: NACK
2965408-2965408 I²C: Address/Data: Stop
3965284-3965284 I²C: Address/Data: Start
3965294-3965364 I²C: Address/Data: Address write: 4A
3965364-3965374 I²C: Address/Data: Write
3965374-3965384 I²C: Address/Data: NACK
3965391-3965391 I²C: Address/Data: Stop
3965692-3965692 I²C: Address/Data: Start
3965702-3965773 I²C: Address/Data: Address write: 4A
3965773-3965783 I²C: Address/Data: Write
3965782-3965792 I²C: Address/Data: NACK
3965800-3965800 I²C: Address/Data: Stop
4965651-4965651 I²C: Address/Data: Start
4965661-4965731 I²C: Address/Data: Address write: 4A
4965731-4965741 I²C: Address/Data: Write
4965741-4965751 I²C: Address/Data: NACK
4965759-4965759 I²C: Address/Data: Stop
4966072-4966072 I²C: Address/Data: Start

...

I am not sure if I am using it incorrectly or if there is some unexpected situation.

Here is the minimal reproducible repository related to the issue. You can pull this repository to test, and no slave device needs to be connected.

bjoernQ commented 2 months ago

I can reproduce this with just esp-hal / without shared-bus and with both, blocking and async

I can get it to work with the changes from #1847 and recreating the driver between the tries

jounathaen commented 2 months ago

I think I have the same issue. I have an I2C bus map in my code, and with the latest master, this is stuck for eternity.

/// Scans the I2C bus and prints a map of all found devices
pub async fn bus_map<I2C: I2c>(i2c: &mut I2C) {
    let mut results = [-1; 16 * 8];
    for (addr, res) in results.iter_mut().enumerate() {
        if let Ok(()) = i2c.read(addr as u8, &mut [0; 0]).await {
            *res = addr as i8;
        }
    }
    info!("I2C Map:");
    info!("      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f");
    for i in 0..8 {
        let mut res_string = String::<64>::new();
        write!(res_string, "{:#4x} ", i * 16).unwrap();
        for r in &results[i * 16..(i + 1) * 16] {
            if *r != -1 {
                write!(res_string, " {:2x}", r).unwrap();
            } else {
                write!(res_string, " --").unwrap();
            }
        }
        info!("{res_string}");
    }
}
trinhanhtuan commented 1 month ago

Hi All, It seems I have the same issue. The I2C scanning stops at 3 first iterations (see below error log). I found an user also has the same issue here but can't find any answer yet. If I change I2C::new to I2C::new_with_timeout then no hang but scan out nothing. Do you have any idea?

Here is the code: (I tested on ESP32 GPIO21+22, ESP32-C3 GPIO2+3 and an OLED with SH1106 driver, I tested with both esp-hal v0.18 & v0.19)

    let peripherals = Peripherals::take();
    let system = SystemControl::new(peripherals.SYSTEM);
    let clocks = ClockControl::max(system.clock_control).freeze();
    esp_println::logger::init_logger_from_env();
    let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);

    let mut i2c0 = I2C::new(
        peripherals.I2C0,
        io.pins.gpio21,         // SDA
        io.pins.gpio22,         // SCL
        100.kHz(),
        &clocks,
        None);

    for addr in 0..=127 {
        log::info!("Scan {}", addr as u8);
        let res = i2c0.read(addr as u8, &mut [0]);

        match res {
            Ok(_) => log::info!("Dev Found @ {}", addr as u8),
            Err(_) => log::info!("No Dev")
        }
    }

    loop {}

And here is the error:

rst:0x1 (POWERON_RESET),boot:0x33 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff0030,len:7104
load:0x40078000,len:15576
load:0x40080400,len:4
ho 8 tail 4 room 4
load:0x40080404,len:3876
entry 0x4008064c
0x4008064c - esp_hal::interrupt::xtensa::vectored::handle_interrupts
    at /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-hal-0.18.0/src/interrupt/xtensa.rs:441
I (31) boot: ESP-IDF v5.1-beta1-378-gea5e0ff298-dirt 2nd stage bootloader
I (31) boot: compile time Jun  7 2023 07:48:23
I (33) boot: Multicore bootloader
I (37) boot: chip revision: v3.0
I (41) boot.esp32: SPI Speed      : 40MHz
I (46) boot.esp32: SPI Mode       : DIO
I (50) boot.esp32: SPI Flash Size : 4MB
I (55) boot: Enabling RNG early entropy source...
I (60) boot: Partition Table:
I (64) boot: ## Label            Usage          Type ST Offset   Length
I (71) boot:  0 nvs              WiFi data        01 02 00009000 00006000
I (79) boot:  1 phy_init         RF data          01 01 0000f000 00001000
I (86) boot:  2 factory          factory app      00 00 00010000 003f0000
I (94) boot: End of partition table
I (98) esp_image: segment 0: paddr=00010020 vaddr=3f400020 size=02800h ( 10240) map
I (110) esp_image: segment 1: paddr=00012828 vaddr=3ffb0000 size=00008h (     8) load
I (115) esp_image: segment 2: paddr=00012838 vaddr=40080000 size=015a0h (  5536) load
I (126) esp_image: segment 3: paddr=00013de0 vaddr=00000000 size=0c238h ( 49720)
I (149) esp_image: segment 4: paddr=00020020 vaddr=400d0020 size=052cch ( 21196) map
I (158) boot: Loaded app from partition at offset 0x10000
I (158) boot: Disabling RNG early entropy source...
INFO - Scan 0
INFO - No Dev
INFO - Scan 1
INFO - No Dev
INFO - Scan 2
bjoernQ commented 1 month ago

For me this code works fine on ESP32-C3 without anything attached (using 0.19.0). On ESP32 I see the same behavior as you with 0.19.0 but it gets though all 128 addresses when using current main

trinhanhtuan commented 1 month ago

For me this code works fine on ESP32-C3 without anything attached (using 0.19.0). On ESP32 I see the same behavior as you with 0.19.0 but it gets though all 128 addresses when using current main

Yes, I met the same.

I also tried with earlier esp-hal versions (e.g. v0.15) but the same issue. It is weird. Thanks.

bjoernQ commented 1 month ago

One thing you could try is to re-create the driver in case of a failure:

let mut peripherals = Peripherals::take();
    let system = SystemControl::new(peripherals.SYSTEM);
    let clocks = ClockControl::max(system.clock_control).freeze();
    esp_println::logger::init_logger_from_env();
    let mut io = Io::new(peripherals.GPIO, peripherals.IO_MUX);

    let mut i2c0 = I2C::new(
        &mut peripherals.I2C0,
        &mut io.pins.gpio21, // SDA
        &mut io.pins.gpio22, // SCL
        100.kHz(),
        &clocks,
    );

    for addr in 0..=127 {
        log::info!("Scan {}", addr as u8);
        let res = i2c0.read(addr as u8, &mut [0]);

        match res {
            Ok(_) => log::info!("Dev Found @ {}", addr as u8),
            Err(_) => {
                log::info!("No Dev");

                core::mem::drop(i2c0);
                i2c0 = I2C::new(
                    &mut peripherals.I2C0,
                    &mut io.pins.gpio21, // SDA
                    &mut io.pins.gpio22, // SCL
                    100.kHz(),
                    &clocks,
                );
            }
        }
    }

    loop {}

If that helps, we probably need a more robust error recovery mechanism in the driver

trinhanhtuan commented 1 month ago

One thing you could try is to re-create the driver in case of a failure:

...

If that helps, we probably need a more robust error recovery mechanism in the driver

I did it but issue is still there. Because we use v0.19 then I put None for the last param of I2C::new() . I can't find anywhere the explanation for this param. I don't know if anyone successfully use esp_hal::I2C with real devices so I can learn from them. It surprises me a bit.

    let mut i2c0 = I2C::new(
        &mut peripherals.I2C0,
        &mut io.pins.gpio21,         // SDA
        &mut io.pins.gpio22,         // SCL
        100.kHz(),
        &clocks,
        None
    );

    for addr in 0..=127 {
        log::info!("Scan {}", addr as u8);
        let res = i2c0.read(addr as u8, &mut [0]);

        match res {
            Ok(_)   => log::info!("Dev Found @ {}", addr as u8),
            Err(_)  => {
                        log::info!("No Dev");
                        core::mem::drop(i2c0);
                        i2c0 = I2C::new(
                                &mut peripherals.I2C0,
                                &mut io.pins.gpio21,
                                &mut io.pins.gpio22,
                                100.kHz(),
                                &clocks,
                                None
                            );
                    }
        }
    }
IvanLi-CN commented 1 month ago

I have updated to version 0.20.1 and made concerted efforts to recreate the driver under no_std, async, and shared bus conditions.

However, these attempts were unsuccessful. Due to my limited proficiency in Rust, I have tried several approaches but have been unable to resolve the issues related to I2cDevice and the 'static lifetime.

My persistence in using I2cDevice is motivated by the future requirement to share the I2C bus across multiple embassy tasks.

Below is a simplified version of my test code, for which I am seeking assistance.

#![no_std]
#![no_main]
#![feature(impl_trait_in_assoc_type)]

use embassy_embedded_hal::shared_bus::asynch::i2c::I2cDevice;
use embassy_executor::Spawner;
use embassy_sync::{blocking_mutex::raw::CriticalSectionRawMutex, mutex::Mutex};
use embedded_hal_async::i2c::I2c;
use esp_backtrace as _;
use esp_hal::{
    clock::ClockControl,
    gpio::Io,
    i2c::I2C,
    peripherals::Peripherals,
    prelude::*,
    system::SystemControl,
    timer::systimer::{SystemTimer, Target},
};

#[esp_hal_embassy::main]
async fn main(_spawner: Spawner) {
    esp_println::logger::init_logger_from_env();

    log::info!("starting");

    let peripherals = Peripherals::take();
    let system = SystemControl::new(peripherals.SYSTEM);
    let clocks = ClockControl::max(system.clock_control).freeze();

    let systimer = SystemTimer::new(peripherals.SYSTIMER).split::<Target>();
    esp_hal_embassy::init(&clocks, systimer.alarm0);

    let io = Io::new(peripherals.GPIO, peripherals.IO_MUX);
    let mut i2c0 = peripherals.I2C0;
    let mut sda = io.pins.gpio4;
    let mut scl = io.pins.gpio5;

    let mut i2c = I2C::new_async(&mut i2c0, &mut sda, &mut scl, 100u32.kHz(), &clocks);
    let mut i2c_bus = Mutex::<CriticalSectionRawMutex, _>::new(i2c);

    let mut dev = I2cDevice::new(&i2c_bus);

    loop {
        match dev.write(0xaa, &[0x11]).await {
            Ok(()) => {
                log::info!("Ok");
            }
            Err(err) => {
                log::error!("Error: {:?}", err);

                core::mem::drop(i2c);
                i2c = I2C::new_async(&mut i2c0, &mut sda, &mut scl, 100u32.kHz(), &clocks);
                i2c_bus = Mutex::<CriticalSectionRawMutex, _>::new(i2c);

                dev = I2cDevice::new(&i2c_bus);
            }
        }
    }
}
image

Alternatively, is it possible to reset the current driver without recreating it entirely? I am exploring options to reinitialize or reconfigure the existing driver instance to address the issues I've encountered.