espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
13.51k stars 7.26k forks source link

cannot read an I2C command of unkown length (IDFGH-13004) #13954

Open combalafra01 opened 4 months ago

combalafra01 commented 4 months ago

Answers checklist.

IDF version.

5.2.2

Espressif SoC revision.

ESP32

Operating System used.

Windows

How did you build your project?

VS Code IDE

If you are using Windows, please specify command line type.

None

Development Kit.

ESP32-DevKitC

Power Supply used.

USB

What is the expected behavior?

it should be possible to read a I2C request of unknown length on slave side it should be possible to read a I2C response of unknown length on master side

What is the actual behavior?

it is not possible ... or I don't understand how to do it

Steps to reproduce.

use 2 ESP32, one as master, one as save I want the master to send length/value data to the slave so the exact size of the data sent by the master to the slave is unknown on the slave side. The data have a known maximum length. master uses i2c_master_transmit to send the data. First 2 bytes are used for length, example: 0x03 0x00 0xAA 0xBB, 0XCC to sent AABBCC from master to the slave. In this example we could assume that the maximum length is 256. If on slave side I use i2c_slave_receive with a length of 5 I get all my data but I cannot do that since I do not know the length of the request. If on the slave side I use a length of 2 in i2c_slave_receive I get the length part of the request (0x03 in my example) but if I call i2c_slave_receive with this length of 3 I do not receive any data because it expects a new I2C request to do something. If on the slave side I use the maximum length of 256 in i2c_slave_receive I have memory issues and sometime crashes in s_i2c_handle_complete because it really try to read 256 bytes.

Debug Logs.

No response

More Information.

this was possible with the legacy driver. The only solution I would have with the new driver would be to always send the maximum number of bytes but in term of performance this is really bad because I will send lots of useless 0x00 to pad the data sent.

combalafra01 commented 4 months ago

I have changed a little bit the function s_i2c_handle_complete in the same way s_i2c_handle_rx_fifo_wm is written and it seems to work now: I can call _i2c_receive_slave with a length that is the maximum length of my message and it will read correctly if the message is shorter.

`static IRAM_ATTR void s_i2c_handle_complete(i2c_slave_dev_handle_t i2c_slave, i2c_slave_receive_t t, BaseType_t do_yield) { i2c_hal_context_t *hal = &i2c_slave->base->hal; uint32_t rx_fifo_cnt; i2c_ll_get_rxfifo_cnt(hal->dev, &rx_fifo_cnt); if (rx_fifo_cnt != 0) { uint32_t fifo_cnt_rd = MIN(t->rcv_fifo_cnt, rx_fifo_cnt); i2c_ll_read_rxfifo(hal->dev, i2c_slave->data_buf, fifo_cnt_rd); memcpy(t->buffer + i2c_slave->already_receive_len, i2c_slave->data_buf, fifo_cnt_rd); i2c_slave->already_receive_len += fifo_cnt_rd; t->rcv_fifo_cnt = 0; } if (i2c_slave->callbacks.on_recv_done) {

i2c_slave_rx_done_event_data_t edata = {
    .buffer = t->buffer,
};
i2c_slave->callbacks.on_recv_done(i2c_slave, &edata, i2c_slave->user_ctx);
xSemaphoreGiveFromISR(i2c_slave->slv_rx_mux, do_yield);
i2c_ll_disable_intr_mask(hal->dev, I2C_LL_SLAVE_RX_EVENT_INTR);

} }`

combalafra01 commented 4 months ago

it would be a good idea to add the actual length of the buffer in i2c_slave_rx_done_event_data_t

i2c_slave_rx_done_event_data_t edata = {
    .buffer = t->buffer,
    .buffer_len = i2c_slave->already_receive_len,
};