Update Wire peripheral handler #669
Open
Conversation
This file contains 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
The existing onService method for I2C peripheral (slave) implementation swapped the sense of onRequest (master read) and onReceive (master write) handlers. It also didn't handle writes correctly, requiring two write operations to get bytes out of the buffer and in the onReceive handler. This implementation follows the SERCOM I2C flow defined for the default clock-stretching case (CTRLB.SCLSM=0) as described in the SAMD21 datasheet. Make sure the write NACK only happens on a stop
|
Memory usage change @ 4ca9f97
Click for full report table
|
This was referenced Apr 19, 2022
|
Thanks @stonehippo. Since I have seen this misconception happen several times, I would like to clarify that unrelated failures of the CI runs should never be the reason for a PR to one of Arduino's repositories not being reviewed or merged. The review of pull requests is done (or neglected 😢) by humans. The automated CI system is only there to facilitate the review. The reviewer is fully able to interpret the CI results and disregard the pre-existing failures that are completely unrelated to the changes proposed in the pull request. So their failure does not in any way block progress on the project development. The automated systems assist us, but they do not control us. Of course those failing CI runs may cause confusion for the contributors, so it is certainly important to maintain the CI system. |
|
Thanks, @per1234. I should also say that I had to cancel the other PR due to an error on my side of things (I made that PR from the master branch of my fork, which was a mistake, and when I corrected that, it closed the old PR automatically). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The existing onService method for I2C peripheral (slave) implementation
swapped the sense of onRequest (master read) and onReceive (master
write) handlers. It also didn't handle writes correctly, requiring two
write operations to get bytes out of the buffer and in the onReceive
handler.
This implementation follows the SERCOM I2C flow defined for the
default clock-stretching case (CTRLB.SCLSM=0) as described in the
SAMD21 datasheet.
(this was originally submitted as #605, but got blocked due to another issue with tests; see #616. Re-submitting, with the update rebased against latest master).