search

caseyjhol / node-red-contrib-tplink

A collection of Node-RED nodes for TP-Link Smart Home devices
https://www.npmjs.com/package/node-red-contrib-tplink-iot
MIT License
16 stars 15 forks source link

Move .deviceConnected=true into the promise #18

Closed hufftheweevil closed 4 years ago

hufftheweevil commented 4 years ago

This fix prevents an error from occurring if an action is called before the device connects.

MaddyTP commented 4 years ago

Not sure why, but this change does not work on multi-plug devices (HS107/HS300).

hufftheweevil commented 4 years ago

Odd - I haven't had time to test this out, but just looking at the code it doesn't seem like there should be an issue with the multi-plug path. Are you saying it never changes the .deviceConnected flag to true?

Also - I'm a little new to the whole PR business - I thought I could make a separate PR for a different commit to my fork, but now I realized it just added it to this one. The other commit just adds the device IP to the output messages as the .topic - helpful for when you connect multiple nodes up to one function.

MaddyTP commented 4 years ago

Yeah, I don’t see a problem including this in the promise either, but the flag never changes for HS107/HS300. These just won’t connect with those two lines moved into the promise.

caseyjhol commented 4 years ago

@hufftheweevil In the future, you'll want to create a separate branch for a new feature, and that will allow you to submit multiple PRs and continue to make changes without one branch affecting the other.

hufftheweevil commented 4 years ago

Closing to create separate PRs for each change and debug issue with original PR