You are reading a single comment by @AkosLukacs and its replies. Click here to read the full conversation.
  • Hi!

    Recently ran into an issue with clearWatch in library code. (This cost me couple of hours, so possibly over exaggerating this...)

    clearWatch() and clearWatch(undefined) clears all pin-watches. You can accidentally call it with undefined, if you don't check whether you have an actual watch Id, for example line 31-32 in DHT22.js.

    Because of the "reliability" of the DHT11 & 22 sensors and the 10 default retries in the libraries, these are dangerous for almost 6 seconds (550 ms timeout * 10). If you don't read the docs, you can run into a race condition where you start a conversion before all possible timeouts of the previous read had finished.
    If you use setWatch elsewhere, that clearWatch(undefined) just destroys you program without crash, and leads to some head scratching... Or leads to a memory leak, if you call .read() too soon. It did leak slowly at 10 retries and 3 second read interval. At the end the setWatch 'count' was up to 50. But that's a different matter.

    Also clearTimeout and clearInterval is the same.

    First, a suggestion:
    What would be the preferred way to handle clear***()? Seen several ways in devices / modules code: I think it shouldn't be without checks and should set the 'watch' property to falsey, if there is one.

    For example there is this nice, but nontrivial looking example from the TouchRD code (_ is "this", and _.w is the watch Id):

    _.w = _.w && clearWatch(_.w);
    

    Or the more obvious

    if (this.watch) {
         clearWatch(this.watch);
         this.watch = undefined;
    }
    

    If there is a consensus, I'm willing to change the libraries, around dozen uses in JS files.

    Does anyone think it would worth making clearWatch(undefined) invalid, and changing the clear-all call explicit (for example pass in -1)? I know, it's a breaking change. Possibly write a warn to console?
    Or better just improve the libraries & docs?

About

Avatar for AkosLukacs @AkosLukacs started