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

Don't waste RAM in the low-level TWI Wire drivers. #196

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

dewhisna
Copy link
Contributor

@dewhisna dewhisna commented Jan 5, 2023

Apply MCUdude/MiniCore#245 to this MegaCore repo.

Since twi_readFrom() and twi_writeTo() are both blocking functions, there is no need to allocate a special twi_masterBuffer. Doing so wastes valuable RAM, uses extra time to copy the data to the secondary buffer, and limits the transfer size to TWI_BUFFER_SIZE. Instead, it only needs a pointer to the buffer for the IRQ to use for the transfer. And, if asynchronous non-blocking functions are ever added, which will require a different API and callbacks, etc., then the existing txBuffer and rxBuffer for slave mode can just be used there too, since master mode and slave mode can't both be active at the same time.

Since twi_readFrom() and twi_writeTo() are both blocking functions,
there is no need to allocate a special twi_masterBuffer.  Doing so
wastes valuable RAM, uses extra time to copy the data to the secondary
buffer, and limits the transfer size to TWI_BUFFER_SIZE.  Instead, it
only needs a pointer to the buffer for the IRQ to use for the transfer.
And, if asynchronous non-blocking functions are ever added, which will
require a different API and callbacks, etc., then the existing txBuffer
and rxBuffer for slave mode can just be used there too, since master
mode and slave mode can't both be active at the same time.
@MCUdude
Copy link
Owner

MCUdude commented Jan 5, 2023

Perfect, thanks!

@MCUdude MCUdude merged commit 98186d7 into MCUdude:master Jan 5, 2023
@dewhisna dewhisna deleted the LessTWIRAM branch January 5, 2023 20:24
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.

2 participants