search

PaulStoffregen / OneWire

Library for Dallas/Maxim 1-Wire Chips
http://www.pjrc.com/teensy/td_libs_OneWire.html
579 stars 382 forks source link

depower() misses DIRECT_WRITE_LOW(baseReg, bitmask); #37

Closed RobTillaart closed 3 years ago

RobTillaart commented 6 years ago

Going through the code of OneWire I noticed that the depower() function does call DIRECT_MODE_INPUT(baseReg, bitmask); but does not call DIRECT_WRITE_LOW(baseReg, bitmask); which is needed to depower if I understand correctly.

If agreed upon I can make a pull request to fix. That PR would also refactor the write() and write_bytes() to include if ( !power) depower(); to remove duplicate code.

orgua commented 6 years ago

you don't do a lot of embedded dev, right? writing the pin low is not needed. switching the pin to input- or "high impedance"-mode is enough. the whole implementation of the power-argument is faulty. the bus is powered for a short time even when power==false. i fixed this and several other bugs in my fork: https://github.com/orgua/OneWire

RobTillaart commented 6 years ago

define a lot :) If the pin low is not needed, the implementation (in this master) of write() and write_bytes() do not need pin low. At least the code is inconsistent.
I'll have a look at your fork how you solved it.

orgua commented 6 years ago

well, for "define a lot": just read the dev history in the main readme.md on the frontpage.

pin_low has other purposes, especially on AVRs. this current code is a big mixup of ancient half-implemented features. but paul calls it "very mature" and won't change a bit

RobTillaart commented 3 years ago

I checked my open issues and came across this old one. Not relevant anymore for me so I close it.