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

fix: Race condition fix for issue #257 #259

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

milesburton
Copy link
Owner

@milesburton milesburton commented Jan 4, 2025

feat(license): migrate to MIT license

  • Replace GNU Lesser General Public License with MIT License in both header and implementation files
  • Update copyright year to 2024
  • Add full MIT license text and conditions

feat(error-handling): add retry mechanism for temperature readings

  • Add retryCount parameter to getTemp() with default value of 0
  • Add retryCount parameter to getTempC() with default value of 0
  • Implement retry logic in temperature reading functions

fix(device-search): improve address retrieval reliability

  • Add index validation in getAddress() method
  • Enhance error handling for device search
  • Improve efficiency of device enumeration

docs(api): update function documentation

  • Add documentation for new retry parameters
  • Update method signatures in header file
  • Clarify temperature reading behavior

test(temperature): enhance temperature reading reliability

  • Add retry mechanism to handle intermittent sensor failures
  • Implement progressive retry logic
  • Add timeout protection for device communication

refactor(device-handling): improve device management

  • Optimize device count verification
  • Enhance device address validation
  • Improve error handling in device communication

@RobTillaart
Copy link
Contributor

@milesburton

minor note:
Add retryCount parameter to getTempF() with default value of 0 Fahrenheit too, to be consistent.

@milesburton
Copy link
Owner Author

@milesburton

minor note: Add retryCount parameter to getTempF() with default value of 0 Fahrenheit too, to be consistent.

Thanks Rob, let me make that change

@milesburton
Copy link
Owner Author

@RobTillaart @tdwilson Can you guys sanity check this?

@RobTillaart
Copy link
Contributor

@milesburton

Probably later this week, I have some catching up to with some other projects.

@milesburton
Copy link
Owner Author

Hey @RobTillaart, thanks for your getting back to me. I’ve been digging through my project cabinets but, unfortunately, I can’t find the test rig I built for exactly this purpose. Tim also got back to me, although it may be a few weeks before he can do any testing on his end.

In the meantime, I’m considering building a small test rig that would automatically sanity-test changes like these. Given all the Arduino libraries you’ve worked on, do you happen to have a similar setup already? I’d appreciate any insight or suggestions you might have.

@RobTillaart
Copy link
Contributor

@milesburton

The .github/workflows has three actions defined

  • Arduino-lint - checks the library.properties file and checks the examples, this is also used by the library manager of Arduino.
  • Json-check - not used but could check the library.json file used by platformIO
  • Arduino-CI - does two things (1) runs unit test and (2) compiles all examples

The detailed output of these actions can be seen here (and as green ticks e.g. in a PR)

image

Note:
The unit tests are disabled as there is a problem with finding CRC from onewire. See test folder
The compilation of the examples works, the last successful one was 3 days ago.
See - https://github.com/milesburton/Arduino-Temperature-Control-Library/actions/runs/12612151488/job/35148582797

The boards used for compilation are defined in the file .arduino-ci.yml (note the . at the begin)
I have some additional code in my libraries this file to also compile for the rpipico

Compiling the examples at least learns that the code of the library is syntactically correct and compiles.
Given the broad range of the examples most code paths are touched.
Type mismatches and several type of bugs are caught.

Compilation only is however not a unit test.


Finally in my A1301 repo / develop branch (my playground) I have a first try of the platformIO test runner.
Again this is also compile only.

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