Hap-Node-Client icon indicating copy to clipboard operation
Hap-Node-Client copied to clipboard

Add missing bind calls, use try/catch block for this.RegisterPin

Open dxdc opened this issue 4 years ago • 4 comments

@NorthernMan54 what do you think about this? At least may lead to some helpful debugging.

dxdc avatar May 05 '21 22:05 dxdc

What I found during testing was that this was undefined 100% of the time when homebridge-alexa invoked the functions. I did go thru various versions of the bind position, but once I identified that it was not defined it did not look like a feasible approach.

My thought at this time is see if anyone raises this as an issue, and recommend that they just switch to a fixed IP and Port. As most clients are using device ID as the key how likely is this to fail ?

NorthernMan54 avatar May 05 '21 22:05 NorthernMan54

My thought at this time is see if anyone raises this as an issue, and recommend that they just switch to a fixed IP and Port. As most clients are using device ID as the key how likely is this to fail ?

The issue you raised was that IP addresses can change, so that's the purpose of the code. I don't see the harm in at least putting it in a try/catch block since it will at least work in other places?

It looks to me something to do with this block of code, due to the traceback. I.e., somehow, callback() is then re-invoking this.registerPin.

setTimeout(function() {
      // debug('Timeout:');
      browser.stop();
      populateCache = false;
      callback();
    }, timeout * 1000);

Wondering also, maybe it shouldn't be an anonymous setTimeout? Not really sure, haven't dug into it enough.

This PR covers all the cases of _populateCache as far as I can tell, but it would be interesting to know what callbacks are tripping this.

dxdc avatar May 05 '21 23:05 dxdc

The callback being triggered is from the MDNS browser and is expected

Another approach I was thinking about was to change the function signature of registerpin and make it available as a function without the this scope. And have the exported registerpin call the function version, and the callback also call the function version. ie remove the THIS dependancy

NorthernMan54 avatar May 05 '21 23:05 NorthernMan54

Another approach I was thinking about was to change the function signature of registerpin and make it available as a function without the this scope. And have the exported registerpin call the function version, and the callback also call the function version. ie remove the THIS dependancy

Thanks for the explanation. Yes, this approach might be even better than the existing one. We just need some way to access the global pincache without tripping over this.

dxdc avatar May 06 '21 14:05 dxdc