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

In i2c_read() check endTransmission() and requestFrom() to ensure data returned is valid #11

Merged
merged 1 commit into from
Nov 8, 2022

Conversation

bakerkj
Copy link
Contributor

@bakerkj bakerkj commented Oct 28, 2022

I believe this change addresses issues like #10 and maaad/RadSens1v2/issues/3 where i2c communications issues were not detected and therefore erroneous values were returned to the caller.

  1. As documented here Arduino Wire endTransmission() documentation
    endTransmission() returns 0 on success and other values on error.

    In the case of an error i2c_read() now returns false so callers do
    not try to interpret invalid data.

  2. As documented here Arduino Wire requestFrom() documentation
    requestFrom() returns the number of bytes returned from the peripheral.

    In the case of fewer bytes returned than the number requested in
    i2c_read() return false to callers so they do not try to interpret
    invalid data.

…nctions/communication/wire/endtransmission/

   endTransmission() returns 0 on success and other values on error.

   In the case of an error i2c_read() now returns false so callers do
   not try to interpret invalid data.

2) As documented here https://www.arduino.cc/reference/en/language/functions/communication/wire/requestfrom/
   requestFrom() returns the number of bytes returned from the
   peripheral.

  In the case of fewer bytes returned than the number requested in
  i2c_read() return false to callers so they do not try to interpret
  invalid data.
@krbaker
Copy link

krbaker commented Nov 1, 2022

+1
This is helping me solve some data inconsistency issues, thanks

@octopolyr
Copy link
Collaborator

Hi! Thanks for feedback! We've recieved your pull request, I'll take a time to talk with our CTO to approve it
@bakerkj @krbaker

Copy link
Owner

@climateguard climateguard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes accepted, thank you

@climateguard climateguard merged commit 59b2cf8 into climateguard:master Nov 8, 2022
@bakerkj
Copy link
Contributor Author

bakerkj commented Nov 9, 2022

@climateguard / @octopolyr is it worth tagging a new release # with this fix?

@octopolyr
Copy link
Collaborator

@bakerkj
We're working on a couple of features, the update will happen within a week. Please accept our apologies for the inconvenience

@bakerkj
Copy link
Contributor Author

bakerkj commented Feb 2, 2023

Hi @octopolyr , I was curious how the tag creation was going? Thanks!

@bakerkj
Copy link
Contributor Author

bakerkj commented May 18, 2023

@octopolyr / @climateguard I was just checking in. Would it be possible to issue a new release?

Thanks!

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.

4 participants