-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
i2c: support writes with a list of buffers #4746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
i2c: support writes with a list of buffers #4746
Conversation
start_addr() read_part() write_part()
For example: i2c.writeto(addr, (buf1, buf2)). This allows to efficiently (wrt memory) write data composed of separate buffers, such as a command followed by a large amount of data.
👍 For all the reasons you've outlined above and on #4020 and #3482. Speaking of For a port (e.g. NRF without making HAL changes) that only supports a single |
If you go down to the register level it can be implemented without memory allocation, like stm32 does it. The point with some hardware/MCUs (eg SAMD I think, and stm32 to some extent) is that they don't offer arbitrary flexibility, eg they don't allow to send only a start condition. The C API here is supposed to be the fine line between what hardware can support and maximum flexibility for creating I2C transactions.
I don't think it's worth it. The functionality can already be achieved using
The way these C functions work is that you must pass in some "continuation information" to tell what is coming next, so the underling I2C implementation can do its job. The full amount of data to send is not needed, just if it's 0, 1 or >1 bytes. In the simplest case the underling port can just buffer the calls, eg make a linked list / array of pointers to buffers, then allocate a big array at the end to concat all the data if necessary. It's a bit of extra work to compute the total length before the transaction starts because all input buffers must be extracted (from Python objects); the current implementation just extracts them one by one. |
👍
I was thinking you'd need to pre-allocate it but yeah of course that works too. (Sorry for the noise) |
Superseded by #4763. |
This implements the ideas for extending
i2c.writeto()
as previously discussed in #4020 (which is still pending). It allows to pass in a list of buffers to write all at once, eg:There are a few commits here:
i2c.writeto_mem()
i2c.writeto()
to accept a tuple/list of buffersIt's not a trivial change. Going down this route puts a burden on ports to implement the new I2C C-level API. Some ports will have trouble doing this because the I2C abstraction they expose is not powerful enough to support such partial transactions, even though I2C as a bus is completely fine doing such things.
esp32 port will be ok, it provides a way to compose multiple writes. nrf port won't be ok, it'll either not support this, need memory allocation to support it, or require low-level changes to its HAL (the hardware can support it, of course). zephyr port will be ok because it provides composition (although will require a small buffer to be set aside for holding the messages).
The alternative is to not go down this path at all. That would then put the burden on the user to find ways around the problem this is trying to solve, namely avoid dynamic memory allocation to combine buffers. That filters all the way up to the top-level user API because they can no longer simply pass buffers down into I2C-based drivers containing data/payload, in case such buffers must be combined with addition data before being sent out (in a single I2C transaction). Eg to send a string to a device might look to the user like
display.show('hello world')
but within the driver this string needs to be copied to a temporary buffer (of unknown initial size) and extra bytes added.So the choice is to push complexity all the way down to the I2C peripheral driver, or all the way up to the end user.