(resolved) I2C call panicking system

[mzimmers]

Also, just wanted to make sure that the function was expecting the address of a 16 bit variable instead of an int.

Which function? Here's the complete code for the function that reads the registers of the external device via I2C:

Code: Select all

esp_err_t Max77818::readReg(uint8_t devAddr, uint8_t regAddr, uint16_t *data)
{
    esp_err_t rc;
    uint32_t len;
    uint8_t addrRead;
    uint8_t addrWrite;
    string s;
    size_t bytesFree;
//    portMUX_TYPE mutex = portMUX_INITIALIZER_UNLOCKED;

    bytesFree = heap_caps_get_free_size(MEMORY_CAPS);
    if (bytesFree < 1024)
    {
        ESP_LOGW(TAG, "readReg(): there are currently %d bytes free in heap.", bytesFree);
    }

    if (devAddr == MAX77818_ADDR_FUELGAUGE)
    {
        len = 2; // fuel gauge registers are 2 bytes.
    }
    else
    {
        len = 1;
    }

    // note about data (address) field: the ESP32 docs say to shift addresses one bit to the left,
    // but it appears that the addresses supplied in the MAX77818 data sheet already have been shifted.
    // keep this in mind when setting addresses from the datasheet.

    addrRead = devAddr | I2C_MASTER_READ;
    addrWrite = devAddr | I2C_MASTER_WRITE;

//    taskENTER_CRITICAL(&mutex);

    m_i2c_cmd = i2c_cmd_link_create();
    if (m_i2c_cmd != nullptr)
    {
        //ESP_LOGI(TAG, "i2cReadReg(): m_i2c_cmd is 0x%x.", (unsigned) m_i2c_cmd);
        // build the i2c transfer.
        rc = (i2c_master_start(m_i2c_cmd));
        rc |= (i2c_master_write_byte(m_i2c_cmd, addrWrite, true));
        rc |= (i2c_master_write_byte(m_i2c_cmd, regAddr, true));
        rc |= (i2c_master_start(m_i2c_cmd));
        rc |= (i2c_master_write_byte(m_i2c_cmd, addrRead, true));
        rc |= (i2c_master_read(m_i2c_cmd, (uint8_t *) data, len, I2C_MASTER_LAST_NACK));
        rc |= (i2c_master_stop(m_i2c_cmd));
        if (rc == ESP_OK)
        {
        }
        else
        {
            s = Max77818_addr_str.at(regAddr);
            ESP_LOGE(TAG, "readReg(): error \"%s\" on %s, register 0x%x during I2C setup.",
                     esp_err_to_name(rc),
                     s.c_str(),
                     regAddr);
        }

        // execute the i2c transfer.
        rc = (i2c_master_cmd_begin(I2C_PORT_NBR, m_i2c_cmd, I2C_WAITTIME_TICKS));
        if (rc == ESP_OK)
        {
            //ESP_LOGI(TAG, "readReg(): device address 0x%02x, register 0x%02x data read as 0x%x.", devAddr, regAddr, *data);
        }
        else
        {
            s = Max77818_addr_str.at(regAddr);
            ESP_LOGE(TAG, "readReg(): error \"%s\" on %s, register 0x%x during I2C read.",
                     esp_err_to_name(rc),
                     s.c_str(),
                     regAddr);
        }

        // if this was a one-byte read, clear the high byte of the data word.
        if (devAddr != MAX77818_ADDR_FUELGAUGE)
        {
            *data &= 0xff;
        }
    }
    else
    {
        ESP_LOGE(TAG, "readReg(): i2c_cmd_link_create() call failed; terminating.");
        rc = ESP_FAIL;
        vTaskDelay(portMAX_DELAY); // might not keep this here.
    }
    i2c_cmd_link_delete(m_i2c_cmd);
//    taskEXIT_CRITICAL(&mutex);

    return rc;
}

[fly135]

So, if all you do is call that function over and over the program eventually crashes? BTW, I'm using IDF v3.1.3.

John

[mzimmers]

Essentially, yes. The program does a few other things (looks at queues for entries), but when the device is left alone, as it was in this case, not much else is really happening.

I could try regressing to 3.1.3, though I remember trying that once, and it was a pretty big hassle. And as I recall, I was having this issue (or at least something similar) when I was on 3.1.

[fly135]

Essentially, yes. The program does a few other things (looks at queues for entries), but when the device is left alone, as it was in this case, not much else is really happening.

That might be where your problems lie.

[mzimmers]

How so? I use queues for inter-task communication. But my structures are small, and every event loop has a wait at the end. Hard to see how I'd be overconsuming resources with these, especially given how much heap I seem to have available.

[fly135]

How so? I use queues for inter-task communication. But my structures are small, and every event loop has a wait at the end. Hard to see how I'd be overconsuming resources with these, especially given how much heap I seem to have available.

I'm not convinced that you are having a problem with consuming resources. You've posted crashes with a corrupt heap and haven't indicated that you know why that happened.

John A

[mzimmers]

That's true. I've seen a few flavors of strange behavior:

- the occasional i2c malloc error
- kernel panics (traceable into the i2c library)
- heap poisoning (traceable into the i2c library)

I did find a possible flaw in my program: the higher level functions using ReadReg were called from two tasks. It's entirely possible that these two tasks could have overlapped in their timing of the calls, and that one or more of my functions (or something within the i2c library) isn't reentrant, which I imagine could cause a variety of errors.

I've changed my code to eliminate this possibility, and will let it run for awhile. I'll report back with any meaningful updates.

[fly135]

That's true. I've seen a few flavors of strange behavior:

- the occasional i2c malloc error
- kernel panics (traceable into the i2c library)
- heap poisoning (traceable into the i2c library)

I did find a possible flaw in my program: the higher level functions using ReadReg were called from two tasks. It's entirely possible that these two tasks could have overlapped in their timing of the calls, and that one or more of my functions (or something within the i2c library) isn't reentrant, which I imagine could cause a variety of errors.

i2c_master_cmd_begin uses a mutex to serial access to the I2C. So I don't think that multiple threads should be a problem. But you can always surround I2C access with your own mutex.

If you experience a situation where the heap is corrupted, then all bets are off. Who knows what kind of carnage is going on behind closed doors. Heap corruption would be my first thing to focus on when having a problem. If you know that it happened then all the crashes for apparent other reasons could still be from overwriting unintended memory locations.

John A

[mzimmers]

Well, I certainly wouldn't claim that I don't any heap bugs in my code, but...since I made my code change, it's run flawlessly for almost 4 hours, which is 2 orders of magnitude better than its original behavior, and 1 order of magnitude better than when I slowed down my I2C access to .5 second intervals. And there's no use of heap in any of my call chain into the I2C library.

I hear what you're saying about the library access being controlled by a mutex, but I just wonder whether my failure to protect against concurrent calls was exposing a latent issue in the library. It admittedly wasn't the best practices of programming on my part, but I don't think any heap errors were/are occurring in this task's program space.

[mzimmers]

It's been running 28 hours now without a problem. I'm not claiming that I don't have a heap problem somewhere, but the lesson I'm taking from this is to not permit the possible overlapping of calls into the I2C library.

Thanks to all who looked at this with me.