fix(Wire): write(buf, len) now returns the actual number of bytes sent - #632
Open
saikumar-mandaji wants to merge 1 commit into
Open
Conversation
TwoWire::write(const uint8_t*, size_t) called the single-byte write(uint8_t) in a loop -- which already correctly returns 0 (via setWriteError()) once txBuffer fills, since txBuffer is a fixed BUFFER_LENGTH-byte buffer -- but discarded that per-byte return value and always returned the full requested quantity regardless. Callers paging a large write across multiple beginTransmission()/ endTransmission() calls (the standard workaround for the 32-byte buffer limit) had no way to detect the truncation and would silently believe the whole buffer had been queued. Now returns the count of bytes actually accepted into txBuffer before the first failed write(), stopping the loop there -- mirroring Print::write(const uint8_t*, size_t)'s already-established convention in this exact codebase (cores/arduino/Print.cpp), which has the identical "accumulate until write() returns 0, then stop" shape. Not compiled locally (no avr-gcc toolchain available in this environment, only arm-none-eabi-*) -- this is a minimal, mechanical change to an existing loop with no new includes or API surface, and its logic is identical in shape to Print::write()'s already-shipping implementation in the same repository, which limits the risk of an uncompiled change. CI will provide the first real build/verification. Fixes arduino#597
|
Memory usage change @ df5f248
Click for full report table
Click for full report CSV |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TwoWire::write(const uint8_t*, size_t)calls the single-bytewrite(uint8_t)in a loop — which already correctly returns 0 (viasetWriteError()) oncetxBufferfills, sincetxBufferis a fixedBUFFER_LENGTH-byte buffer — but discards that per-byte return value and always returns the full requestedquantityregardless.This means code paging a large write across multiple
beginTransmission()/endTransmission()calls (the standard workaround for the 32-byte buffer limit, exactly as described in the linked issue) has no way to detect truncation and silently believes the whole buffer was queued.Now returns the count of bytes actually accepted into
txBufferbefore the first failedwrite(), stopping the loop there — mirroringPrint::write(const uint8_t*, size_t)'s already-established convention in this exact codebase (cores/arduino/Print.cpp), which has the identical "accumulate untilwrite()returns 0, then stop" shape.Honest verification note: not compiled locally — I don't have an
avr-gcctoolchain in this environment, onlyarm-none-eabi-*. This is a minimal, mechanical change to an existing loop (no new includes, no API/signature change), and its logic is identical in shape toPrint::write()'s already-shipping implementation in the same repository, which limits the risk of an uncompiled change. Happy to address any CI/review feedback.Fixes #597