pstolarz / OneWireNg

Arduino 1-wire service library. OneWire compatible. Dallas thermometers support.
BSD 2-Clause "Simplified" License
89 stars 20 forks source link

OneWireNg_Wrapper provided by OneWire.h in OneWireNg? #7

Closed mairas closed 3 years ago

mairas commented 3 years ago

I began to wonder: instead of having OneWireNg_Wrapper in a DallasTemperature fork, should it rather be provided by OneWire.h in the OneWireNg repo? If I understand correctly, in that case the changes to DallasTemperature would be limited just to the dependency declarations in library.json and library.properties?

It might even be possible to eventually convince the PlatformIO crowd to implement a "provides" or "replaces" field in library.json, similar to the Debian packaging Provides and Replaces keywords. In that case, no forking of any downstream repos would be required.

pstolarz commented 3 years ago

That's good idea and I'd been considering it while porting DallasTemperature. But decided to use the wrapper mainly for these 2 reasons:

On the other hand maybe that's not bad idea to write some sort of interface layer between these two libs. I'm aware of popularity of OneWire and its broad usage, so providing such OneWire to OneWireNg interface would help other people experiencing issues with OneWire to switch into this lib w/o any additional effort. In spare time I'll consider writing such class and when ready will ask you for test on your platform(s).

pstolarz commented 3 years ago

@mairas I've added OneWire compatibility interface into the library as described above. New implementation already on master branch (no new lib version yet). DallasTemperature fork is also updated but on branch onewire.

I've checked the changes on Atmega328P (Arduino Uno) and works fine. Could you check the change on your ESP setups?

mairas commented 3 years ago

I'm happy to report the new compatibility interface seems to work fine.

I changed DallasTemperature to point to https://github.com/pstolarz/Arduino-Temperature-Control-Library.git#onewire and OneWireNg to point to https://github.com/pstolarz/OneWireNg.git. I then cleaned the PlatformIO .pio build tree and let it rebuild the project, checking that DallasTemperature and OneWireNg were actually the new versions. Build seemed to go through fine. I then added the redundant #include <onewire.h> entry back to the code and it still worked fine.

Thanks for the effort!

pstolarz commented 3 years ago

Good to hear. I raised lib version to 0.7.2. Once indexed by Arduino / PlatformIO lib-repos I will update DallasTemperature fork on branch OneWireNg to point to this new version and will ask you to make the test one more time.

Thanks for the good idea of improvement and the test! Kiitos!

pstolarz commented 3 years ago

Done. DallasTemperature fork on OneWireNg branch is now a clone of the upstream with library descriptors as the only difference.