Skip to content

Conversation

@orgua
Copy link

@orgua orgua commented Aug 19, 2017

hi paul, i know you only anticipate PR for new hardware support because you say the lib has "very mature code", but this statement is simply not true. over the years it became a big patchwork of ancient or even useless code. there were several bugs to find. the most obvious one is in the bus-power feature. the function write_bit() always powers the bus without permission. I modernized and extended the interface of the lib without breaking old code. i did tests on several platforms. see list of changes below.
where do you see problems in merging this PR?

I would like to co-maintain the onewire-lib with you. Currently I have rebuild and maintained the https://github.com/orgua/OneWireHub project for emulating slave-devices and i would like to apply my experiences to the onewire-master-lib.

changes i made so far:

  • real life tested with several platforms (atmega, atsamD21, Atsam3x, esp8266, esp32, teensy)
  • extend and fix hardware support (specially esp-boards, esp32, ...)
  • added travis CI for a big bunch of controllers
  • delayMicroseconds() hardware abstraction to allow proper nRF51-use (redbear messed up the delaymicroseconds()-implementation)
    • will be removed as soon redbear fixes its code
  • overload write() to replace write_bytes() and read() to replace read_byte()
  • update code documentation, doxygen style
  • allow to use internal pull-up if micro controller has support for it -> most µC support INPUT_PULLUP in combination with pinMode(), others will get an error message or warning if feature is enabled but not supported / tested
  • clean up compiler-switches
  • better implementation to allow parasitic power to bus, otherwise master will act as open-drain-only! -> this was totally wrong before as the bus was powered for a short amount of time even if powering is disabled
  • extend examples with ds2438 and a bus-discovery feature
  • clean up examples, easier to understand, more similar to each other
  • put hardware dependant code into platform.h
  • better naming of variables and a lot of code cleanup
  • simplify pin-access and interrupt-handling
  • improve onewire class-interface (it was possible to copy the object, ...)
  • reduce size of data types if possible, more const correctness and much more explicit code
  • get rid of most ancient c and cpp code and reduce language mix
  • decide on one code style - Allman braces - was closest to main style in original
  • a lot of how-to documentation for this lib inside this readme

@orgua
Copy link
Author

orgua commented Sep 7, 2017

Hi Paul,

it is hard to guess your thoughts when you say nothing at all.

  • You will look into it in the next 1-6 month?
  • You don't want any help maintaining the library, despite the comments in the code?
  • You are categorically against documenting, modernizing and fixing the code?

///
/// \return true = a device responds with a presence pulse.
/// false = there is no device or the bus is shorted or otherwise held low for more than 250uS
bool reset();
Copy link

Choose a reason for hiding this comment

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

This has the same problem as the original version (#79). You can't tell the difference between no devices and a shorted bus.

Copy link
Author

Choose a reason for hiding this comment

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

ok, it could be changed easily but brings one more comparison. and it will break old code. most codebases will only test for non-zero. On the other hand - a major version change should be allowed to break code (with proper documentation).
the most elegant solution (so old code still works) would be to add a register for error-codes that could be checked.

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.

4 participants