Skip to content
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

Improved I2C read API to reduce memory allocation and copying. #32

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

npryce
Copy link
Contributor

@npryce npryce commented Feb 16, 2013

Removed reading(size), replaced with reading_into(buf), where buf is a mutable buffer (e.g. bytearray or ctype struct). This allows client code to control when the buffer is allocated and reuse buffers between I2C transactions, so reducing memory allocation overhead. The I2C data is read directly into the buffer, not copied from a ctypes stringbuffer into a pure Python byte buffer, so reducing memory copying overhead.

Removed reading(size), replaced with reading_into(buf). Can read into
mutable buffers (e.g. bytearrays and ctype structs).
@stuartervine
Copy link

I wondered what people thought about adding an alias of writing_from -> writing_bytes?

It would balance the reading_into method.

@npryce
Copy link
Contributor Author

npryce commented Feb 16, 2013

There is a writing(buf) method but the name does not match the new reading_into method.

Maybe reading_into should be named reading to match writing.

@joewalnes
Copy link

Functionality-wise LGTM.

I second Stuart's comment about the naming of the read vs write methods - let's try to make them symmetrical. Also, it may be convenient to leave the old style reading method around for convenience.

@npryce
Copy link
Contributor Author

npryce commented Feb 20, 2013

The SPI API should behave the same

@npryce
Copy link
Contributor Author

npryce commented Feb 24, 2013

Regarding reading_into vs writing: all other Python APIs have write(bytes) and readinto(bytearray), so by convention the API calls are not named symmetrically. So I think we should make writing_from an alias of writing and leave writing in the API.

@npryce
Copy link
Contributor Author

npryce commented Mar 8, 2013

I've thought of a way to implement this and keep the API backwards compatible. It's a bit more complex but an overall improvement I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants