Timeout on I2C Reads (MPU6886)

JasonPittenger commented 9 months ago

Describe the bug

I have my m5Core2 reading from the MPUT6886 in the main loop at a rate of 25Hz (40mS).

#define ONE_SECOND                      1000        //mS
#define IMU_SAMPLE_RATE                 25
#define IMU_SAMPLE_TIME                 ONE_SECOND / IMU_SAMPLE_RATE

void setup() 
{
    Wire1.setTimeOut(50);       //Set I2C timeout to 50mS (instead of default 1000 mS)
    M5.IMU.Init();
}

void loop() 
{
    static uint32_t u32_GyroTime, u32_OldGyroTime, su32_IMUtime;

    if(millis() - su32_IMUtime >= IMU_SAMPLE_TIME)
    {//Gather IMU data
        su32_IMUtime += IMU_SAMPLE_TIME;
        u32_GyroTime = micros();
        M5.IMU.getGyroData(&s_IMUdata.f_gyroXraw, &s_IMUdata.f_gyroYraw, &s_IMUdata.f_gyroZraw);
        M5.IMU.getAccelData(&s_IMUdata.f_accXraw, &s_IMUdata.f_accYraw, &s_IMUdata.f_accZraw);
        s_IMUdata.d_dt = (u32_GyroTime - u32_OldGyroTime) / 1e6;
        ProcessIMUdata();
        u32_OldGyroTime = u32_GyroTime;
    }
}

The problem is sometimes there is an error on the read. This results in a 1000mS blocking delay, since the default timeout on I2C is 1000mS. I tried setting the wire timeout to 50 but this doesn't have any affect. Is there any way to set up a shorter timeout period?

To reproduce

I am running Arduino code. My stack is the M5 Core2 with a M5GO Battery Bottom2, a Mini GPS/BDS Unit (AT6558), a COMMU Module Extend RS485/TTL CAN/I2C Port and a Kmeter Unit with Thermocouple Temperature Sensor (MAX31855).

Expected behavior

I expect updated gyro and accel data every 40 mS. I have a data log recording every 100mS.

However, I get gaps of 1 second in the data log. I traced the delay down to the reads from the IMU.

Screenshots

Environment

OS: Windows 10
IDE &IDE Version:
Repository Version:

Additional context

No response

Issue checklist

[X] I searched for previous reports in the issue tracker
[X] My report contains all necessary details

icyqwq commented 3 months ago

From wire.cpp


TwoWire::TwoWire(uint8_t bus_num)
    :num(bus_num & 1)
    ,sda(-1)
    ,scl(-1)
    ,bufferSize(I2C_BUFFER_LENGTH) // default Wire Buffer Size
    ,rxBuffer(NULL)
    ,rxIndex(0)
    ,rxLength(0)
    ,txBuffer(NULL)
    ,txLength(0)
    ,txAddress(0)
    ,_timeOutMillis(50)
    ,nonStop(false)
#if !CONFIG_DISABLE_HAL_LOCKS
    ,nonStopTask(NULL)
    ,lock(NULL)
#endif
    ,is_slave(false)
    ,user_onRequest(NULL)
    ,user_onReceive(NULL)
{}

We can see the timeout is already 50ms, so we dont need to set it manualy. I can't reproduce this issue. To start, could you remove all peripherals except the IMU and see if the issue persists?

JasonPittenger commented 3 months ago

Unfortunately, I no longer have access to this device. It was mounted to my motorcycle and I have since sold it with my motorcycle. A good way to reproduce it would be to try to read from the IMU like I have above, and check how many samples you get per second. You should get about 25 samples per second. Then disconnect the IMU without changing the code and see how many samples you get per second. You should get 20 samples per second, since they will all timeout.

While I never removed all peripherals except the IMU, I did comment out the IMU code and the problem went away. To make sure it wasn't an issue with the IMU data processing, I also tried leaving in the IMU reads but commented out any of the processing. The 1 second gap returned.

I did find someone else has a similar problem. They "resolved" it by moving the I2C reads to the 2nd core and just dealing with the long timeout. https://forum.arduino.cc/t/arduino-ide-esp32-i2c-timeout-problem/1040744/14 I'm already using both cores, and I am reading I2C devices on both I2C busses.

Is it possible this was fixed already in an update that wasn't available back when I originally posted the problem?

icyqwq commented 2 months ago

What's your arduino version. From this issue, the bug was fixed in this commit

JasonPittenger commented 2 months ago

I have Arduino IDE version 1.8.19.

That looks like the same issue. Since that issue was found and fixed, AND since I can no longer test and verify a fix, I think we can call this issue closed.

m5stack / M5Core2