NorthernMan54 / node-red-contrib-homebridge-automation

Homebridge and Node-RED Integration
Apache License 2.0
107 stars 18 forks source link

Anonymous setTimeout function #35

Open dxdc opened 4 years ago

dxdc commented 4 years ago

Throughout the code, the use of anonymous setTimeout functions are used to clear the node status.

setTimeout(function() { node.status({}); }, 10 * 1000);

The problem is that this will cause problems when events happen in shorter durations. Suppose that:

To get around it, how about something like this? Not sure if it would be a good idea to add to the 'close' events also so that any existing timeouts are cleared.

  function clearNodeStatus(sec) {
      clearTimeout(node.timeout);
      node.timeout = setTimeout(function() {
          node.status({});
      }, sec * 1000);
  }

Related, removed and done can be used in the 'close' function, which may help with some of the node statuses. I've seen sometimes unexpected values when redeploying nodes (vs. restarting). See documentation here: https://nodered.org/docs/creating-nodes/node-js

Could be useful as well?

this.on('close', function(removed, done) {
    if (removed) {
        // This node has been deleted
    } else {
        // This node is being restarted
    }
    done();
});
NorthernMan54 commented 4 years ago

Tks for pointing these out, I will add them to the backlog.

On Jan 17, 2020, at 1:27 AM, dxdc notifications@github.com wrote:

 Throughout the code, the use of anonymous setTimeout functions are used to clear the node status.

setTimeout(function() { node.status({}); }, 10 * 1000); The problem is that this will cause problems when events happen in shorter durations. Suppose that:

@0 sec, event 1 arrives (setTimeout triggered) @8 sec, event 2 arrives (setTimeout triggered) @10 sec, setTimeout (from event 1) is triggered, status is erased To get around it, how about something like this? Not sure if it would be a good idea to add to the 'close' events also so that any existing timeouts are cleared.

function clearNodeStatus(sec) { clearTimeout(node.timeout); node.timeout = setTimeout(function() { node.status({}); }, sec * 1000); } Related, removed and done can be used in the 'close' function, which may help with some of the node statuses. I've seen sometimes unexpected values when redeploying nodes (vs. restarting). See documentation here: https://nodered.org/docs/creating-nodes/node-js

Could be useful as well?

this.on('close', function(removed, done) { if (removed) { // This node has been deleted } else { // This node is being restarted } done(); }); — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.