Skip to content

Pull OneWire 2.3.8 from Paul Stoffregen's upstream #2981

Merged
slaff merged 8 commits intoSmingHub:developfrom
pljakobs:fix/OneWire
Dec 3, 2025
Merged

Pull OneWire 2.3.8 from Paul Stoffregen's upstream #2981
slaff merged 8 commits intoSmingHub:developfrom
pljakobs:fix/OneWire

Conversation

@pljakobs
Copy link
Contributor

I have noticed OneWire issues on the ESP32C3 when using WiFi - from the data on the wire, it seemed like the timing was slightly off due to calling nointerrups() for the critical sections.
After some investigation and dropping the idea to implement the protocol using the RMT peripheral (mostly because that is strictly send or receive and having both on the same pin requires constantly switching modes) I looked at the upstream and there, the same issue is addressed by using

#define noInterrupts() {portMUX_TYPE mux = portMUX_INITIALIZER_UNLOCKED;portENTER_CRITICAL(&mux)
#define interrupts() portEXIT_CRITICAL(&mux);}

instead of actually disabling / enabling interrupts.

This works reliably for me

However: this has had pretty little testig on my end (just this one platform, just a DS18B12 temperature sensor) so I would love to see it tested in more environments.

@what-the-diff
Copy link

what-the-diff bot commented Nov 23, 2025

PR Summary

  • Introduction of OneWire Submodule - A new Component added
    A new component was added to our system named "OneWire". It's like adding a new cog to our machine from another manufacturer's existing parts (referred to as a 'submodule') which lives at https://github.com/PaulStoffregen/OneWire.git.

  • New Storage for OneWire
    A new storage space (a 'directory') Sming/Libraries/OneWire was added specifically for this new OneWire component.

  • Improved Understanding in Code in OneWire.cpp
    Modified certain short phrases in the code (macros), specifically for 'interrupts', making our code cleaner and easier to understand.

  • Enhancements for Better Platform Compatibility
    Updated a header file connected with OneWire to add recognition elements ('macros') for various platforms our software can run on, namely the ESP32, ESP8266, and RP2040.

  • Removal of Extra Files
    Certain files that were part of the OneWire library, including the main header file ('OneWire.h'), description file ('README.md'), and some example files providing usage scenarios were removed to eliminate clutter in our system.

  • Removal of Syntax Highlighting Feature
    Removed a file (keywords.txt) that provided syntax highlighting feature in the coding interface for OneWire. The readability of our code in the integrated development environment (IDE) now relies on default settings.

  • Reference to Subproject's Versions
    Added a reference to a specific version of the OneWire system in the form of a 'commit', essentially marking the point to which we have updated this part of our software.

@pljakobs
Copy link
Contributor Author

not sure how I should interpret those test failures, given that the code compiles fine on Windows? (I'm also not seeing the full test run output)
also, I notice that there is no esp32 in the test cases

@mikee47
Copy link
Contributor

mikee47 commented Nov 25, 2025

Thanks for this PR!

not sure how I should interpret those test failures, given that the code compiles fine on Windows? (I'm also not seeing the full test run output) ...

Yep, Windows builds are very slow so are not as comprehensive as linux builds.

... also, I notice that there is no esp32 in the test cases

Do you mean in code? esp32 runs in a separate git action, though they all fail due to missing digitalPinIsValid(), guess that was added in the new version.

The other failures seem to be because begin() now has a mandatory pin parameter...

- Make ARDUINO_xxx definitions available for `OneWire_direct_regtype.h`
- Provide `digitalPinIsValid()` macro for ESP32 use
@mikee47
Copy link
Contributor

mikee47 commented Nov 25, 2025

@pljakobs I've taken the liberty of (hopefully) fixing the build errors.

Fixes support for all variants, including esp32c2 which fails CI tests.
@slaff slaff added this to the 6.2.0 milestone Nov 25, 2025
@pljakobs
Copy link
Contributor Author

thanks @mikee47!

@mikee47
Copy link
Contributor

mikee47 commented Nov 26, 2025

I note that OneWire::begin() had a call to noPullup(pin) which is no longer present in the new version. Is this significant?

@pljakobs
Copy link
Contributor Author

generally, OneWire is an open collector bus, so a pullup is needed, but that's generally an external 4.7k - not sure why you'd explicitly disable the internal pullup unless it was significantly lower than 10k or so.

@slaff
Copy link
Contributor

slaff commented Nov 26, 2025

@pljakobs Please test this PR on your micro controllers to see if the issue is fixed with this PR and there are no un-expected side effects.

@mikee47
Copy link
Contributor

mikee47 commented Nov 26, 2025

generally, OneWire is an open collector bus, so a pullup is needed, but that's generally an external 4.7k - not sure why you'd explicitly disable the internal pullup unless it was significantly lower than 10k or so.

The internal pullup on esp* and rp2040 are all very weak by comparison (47K+ IIRC) so I would agree. Although now I think about this, pinMode(pin, INPUT) will itself disable pullups anyway so it is actually a redundant call and won't affect code operation.

So I think this is safe.

@mikee47
Copy link
Contributor

mikee47 commented Nov 26, 2025

@pljakobs Have you checked this out on an esp8266?

@pljakobs
Copy link
Contributor Author

@pljakobs Have you checked this out on an esp8266?

I have not yet have time to do so but will asap. I can also test Esp32.

@slaff
Copy link
Contributor

slaff commented Dec 1, 2025

I have not yet have time to do so but will asap. I can also test Esp32.

@pljakobs any progress here?

@pljakobs
Copy link
Contributor Author

pljakobs commented Dec 3, 2025

finally got around to verify it with an ESP8266 (sorry it took a while, most of them are designed into things, so I had to dig out a dev board)
I have re-written the OneWire Arduino test script to a Sming app (https://github.com/pljakobs/samples-OneWire) and correctly reads a single DS18B20. I have not yet tested multiple sensors or 18S20 sensors or so, but to me, this looks okay.
As there is no OneWire sample per se, maybe you would want to submodule that?

PS: I have a few more OneWire devices on order to verify wider compatibility, but DS18x20 should be the most widely used

@slaff
Copy link
Contributor

slaff commented Dec 3, 2025

As there is no OneWire sample per se, maybe you would want to submodule that?

You can try this sample: https://github.com/SmingHub/Sming/tree/develop/samples/Temperature_DS1820

@slaff slaff merged commit b40075d into SmingHub:develop Dec 3, 2025
48 of 50 checks passed
@pljakobs
Copy link
Contributor Author

pljakobs commented Dec 3, 2025

works too


Reading temperature DEMO
T1 = 21.00 Celsius, (69.80 Fahrenheit)


70052661 DBG: DS1820 reading task start, try to read up to 4 sensors
70223804 DBG: CRC is valid
70223869 DBG: Chip = DS18B20
70223938 DBG: No more address found
70224019 DBG: 1 DS1820 sensors found
71138194 DBG: T1
71143224 DBG: Data = 1 50 1 4b 46 1f ff 1f 10 c1 CRC=c1
71143385 DBG: Temperature = 21. Celsius, 69.800003052 Fahrenheit
71243634 DBG: DS18S20 reading task end

@slaff
Copy link
Contributor

slaff commented Dec 3, 2025

works too

Thank you for testing this too.

@slaff slaff mentioned this pull request Dec 16, 2025
5 tasks
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.

3 participants