Multi-device node

[hufftheweevil]

First off, thank you to @caseyjhol and @MaddyTP for all of your recent work on this project. It's good to see things remaining active when there is so much stale code around.

After much consideration, I've decided to break off and make some considerable changes to my fork. This is mostly due to my need (want?) to use one node for multiple devices in some cases. I have a very good idea of how I want to accomplish that by building on top of this great repo. I intend on making as few breaking changes as possible, should there be any interested to pull my changes in to this one. However, I anticipate it being more than just a few lines of code, and I may refactor the code to attempt to simplify it. I'll totally understand if you don't want it. But I just wanted to put it all out there now, and perhaps see if anyone else is interested in this feature.

[caseyjhol]

I'm definitely open to that. It would obviously be nice to have as few breaking changes as possible, but I'd imagine there wouldn't need to be too many. I've been wanting to consolidate the smart-plug and smart-bulb code into one node, as it is mostly the same. However, I don't have a TP-Link bulb which makes testing difficult. Ultimately I think it would be ideal to keep everything under one roof, rather than have yet another fork. So I'm open to any an all ideas regarding which direction to take this project. I'm thinking this would be a good target for a v1.0.0 (with maybe an alpha or beta release first).

[hufftheweevil]

An update on my progress: I've got a bulk of the changes done. Now I'm going to incorporate the differences with the bulb stuff into my modified plug node.

I've kept pretty much everything the same from an outside point of view, other than adding the ability to have multiple devices for one node (but still allowing just one device to be configured from the properties, if the user wants to do it that way instead/additionally).

The only breaking change would be for the events. I am planning instead of getSomeEvents, the input message would be startSomeEvents, and additionally that would mean stopSomeEvents would work. Of course, all the possible methods of inputting those commands would still work (i.e. array, string, string with pipes, events property).

I suppose the other major breaking change would be the fact that there will only be one node. So no matter what, it'll cause all users to modify their flows. But I agree, it will be ideal to keep it all together. Plugs and switches and bulbs only have slight differences, and it will be a huge benefit to have a uniform node for them all.

Disclaimer: I have the following types of devices that I can test with... HS100, HS105, HS200, HS220, HS300. I don't have any lights (my heart lies with LIFX when it comes to lights), so I'll need some help with the testing before we deploy anything.

[hufftheweevil]

I think I've got it to a point where it meets all the goals I wanted to reach.

https://github.com/hufftheweevil/node-red-contrib-tplink

There is still some connection issues once in a while, but nothing catastrophic. Multi-plug devices definitely make it worse if you want to use to each plug individually, as the underlying API sends a packet to the device multiple times (if you want to poll/getInfo/etc for each plug). Clearly the devices are not meant to be communicated with in such rapid succession. It just means that sometimes we get timeouts (which I've tried my best to catch them all, but somehow some are still getting through to the node-red log). But all-in-all, as long as polling is enabled, you will eventually get all states and keep them up-to-date, for the most part.

Things to note:

• Changed to UDP over TCP. It seems to be more reliable, surprisingly. It also allows for shared sockets to be used. Perhaps an option could be added to let the user choose, if someone has an issue with this. Default send-message timeout is set to 30s.
• Changed default intervals to 60s for discovery and 15s for polling. Added ability to disable either by setting to 0.
• The major upgrade is the ability to have multiple devices per node.
  • As before, use the device IP property to choose a specific device. This device will be used for any input message that does not include a topic. Discovery/picker still works as before.
  • To work with another device, just include the IP as the topic on input.
  • Devices will be connected as needed.
  • All output messages include a topic with the device IP.
• To use a specific plug on a multi-plug device, add a /1 to the end of the IP address, where the digit is the plug number, zero-indexed. This works in property and the input.
• Events are now controlled via startEventName and stopEventName. All events stopped via stopAllEvents. Event outputs use EventName in the payload.event property.
• All other inputs kept the same
• Plug and light combined (Lights untested, as I don't have any)
• Organized documentation
• Added HS103 to support list (just got this the other day, nothing special, but it works)
• Node status now reflects the number of devices connected (and offline). If only one device is connected, then state info is displayed.

Comments, suggestions, questions, ideas welcome.

[caseyjhol]

Awesome! I say go ahead and submit a PR (if you want), and we can continue discussion there regarding certain changes (if there are any). Then, I can publish as v1.0.0-alpha so people can test it out (including bulbs) before finalizing.

[caseyjhol]

In testing with HS200 and HS220 switches, I'm frequently getting UDP timeout errors: Error connecting to device 192.168.1.80: Error: UDP Timeout. When changing to TCP, it seems to function normally.

[hufftheweevil]

You're right, TCP does seem to be working better than UDP. I'm wondering now if all my original connection issues were not due to the transport type, and instead were caused by other things that I have now fixed in the code. (I had found a huge memory leak in first original re-write where it was creating multiple device instances and then losing any reference to them, and hence they were polling when they shouldn't. All of those issues have been fixed, and now it seems TCP is the better way to go.)

[caseyjhol]

Most of my timeout errors disappeared after increasing my event interval to 2 minutes on nodes where I wasn't using events, but simply allowing it to be disabled like you've done is obviously more elegant. I think we're close on this...just need to find some people with bulbs to test it too. Are you familiar with how Node-RED handles pre-releases?

Also - would you be able to make a quick and dirty changelog update, at least to highlight some of the breaking changes?

[hufftheweevil]

Sorry, I'm not familiar with pre-releases, or really any part of the Node-RED release process.

I can make a changelog - probably based mostly on my post from above.

[hufftheweevil]

Totally forgot to mention before that @plasticrake also did a rework of tplink-smarthome-api, and it is now at 3.0.0-beta. So that is included in this rework. PR https://github.com/caseyjhol/node-red-contrib-tplink/pull/27 includes the changelog and a couple of other minor things.

[hufftheweevil]

I just noticed that that link in the about section for the repo goes to the original npm package that you had forked this from.

[caseyjhol]

Got it updated.

@nhausman1 Would you be able to try out v1.0.0-alpha.0 to make sure it works fine with bulbs?

[caseyjhol]

I need to play around with it some more to confirm, but have you had any issues using the node in a subflow? The node was being ignored when in a subflow, but when I moved the entire subflow into my main flow for testing it worked perfectly.

[hufftheweevil]

It appears to work for me in a subflow. I tested it with the commands, events, and control with no issues. Can you post an example of how you're using it?

[caseyjhol]

Looks like it's working now. I was getting some TCP errors in the log initially that I think were related. Haven't had any issues today, though.

[hufftheweevil]

I think we can close this now. If anyone has any problems with the multi-device feature, please open a new issue.