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

Add main I2C sensor Retry-loop #593

Merged
merged 13 commits into from
Jul 16, 2024
Merged

Add main I2C sensor Retry-loop #593

merged 13 commits into from
Jul 16, 2024

Conversation

tyeth
Copy link
Contributor

@tyeth tyeth commented Jun 12, 2024

This is a result of having retries in the drivers/wrappers, and a desire for them to be faster (more interrupt like). We are therefore removing unnecessary delays while waiting to retry one sensor multiple times, and instead retrying the entire suite of sensors up to three times before failing that periodic reading event.

I was trying to add another driver with a long retry time, and this work was the result instead. See #564

This is unfinished, as currently it will evaluate all sensors again, instead only failing ones need be retried.

There will be a follow up PR to remove any unnecessary retries during I2C sensor data reads once this is merged.

Change main I2C polling loop to retry failed sensors 3 times before marking that polling period expired
@tyeth tyeth marked this pull request as ready for review July 1, 2024 12:31
@tyeth tyeth requested a review from brentru July 1, 2024 13:56
@tyeth
Copy link
Contributor Author

tyeth commented Jul 1, 2024

@brentru ready for review

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

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

I think this is good! It increases how robust the update() loop is.

I have two notes:

Performance: I know you noted it in the PR description, but I want to get to it in this PR. Let's mark failed reads and only retry the sensors that fail.

Code complexity: I think the update() loop is growing in its complexity and provided an idea below about encapsulation

src/components/i2c/WipperSnapper_I2C.cpp Show resolved Hide resolved
src/components/i2c/WipperSnapper_I2C.cpp Show resolved Hide resolved
@tyeth tyeth requested a review from brentru July 12, 2024 17:23
@tyeth tyeth merged commit b2901a6 into main Jul 16, 2024
31 checks passed
@tyeth tyeth deleted the retry-loop branch July 16, 2024 21:45
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.

None yet

2 participants