adumont / tplink-cloud-api

A node.js npm module to remotely control TP-Link smartplugs (HS100, HS110) and smartbulbs (LB100, LB110, LB120, LB130) using their cloud web service (no need to be on the same wifi/lan)
https://itnerd.space
GNU General Public License v3.0
130 stars 44 forks source link

gracefully handle tplink.login() failure #18

Closed blaskovicz closed 6 years ago

blaskovicz commented 6 years ago

If login fails to return a token, it throws the error: (node:3353) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'token' of undefined

Rather than the actual response error (for example, I didn't pass any params): { error_code: -20104, msg: 'Parameter doesn\'t exist' }

ColinMcNeil commented 6 years ago

Is it a bug to assume the developer is passing params?

blaskovicz commented 6 years ago

I would say throwing an error if the required function params aren't passed AND exposing the actual response error is preferred. I can put a pull request up once I've had more time to play with the library and see what else I like/have issues with.

ColinMcNeil commented 6 years ago

This repo hasn't been updated since April... https://github.com/adumont/tplink-cloud-api/pull/17 If you want to fork it, I would also like to PR this https://github.com/adumont/tplink-cloud-api/pull/14

blaskovicz commented 6 years ago

I also found this repo, https://github.com/plasticrake/tplink-smarthome-api/, which looks very active but doesn't support cloud interactions out of the box. Another option is to add cloud support there so we can merge these feature sets, since that repo is very active.

ColinMcNeil commented 6 years ago

That one is also much more than what I typically need it for, and also does not work with Webpack. (https://github.com/ColinMcNeil/TP-Link-WebApp) Otherwise I would've stopped subscribing to notifications here.

blaskovicz commented 6 years ago

Understood. I was using it as a reference since I noticed some other good features that are implemented there but missing here. For example, when I do a device list, I'd like to see those devices come back as instantiated device objects so I can directly action on them (eg: powerOn, etc).

I took a look at your code! I'm trying to build a similar thing (dashboard for turning things on/off) but on a garmin watch (eg: I did this for nest https://github.com/blaskovicz/garmin-nest-camera-control)

blaskovicz commented 6 years ago

@ColinMcNeil have you noticed issues with device list returning stale data from tplink's api? For example, I was able to easily turn my LB120 on/off but the status stays at 1 in the response.

blaskovicz commented 6 years ago

forked to https://github.com/blaskovicz/tplink-cloud-api and did 3 things:

ColinMcNeil commented 6 years ago

Yes, the return data is unreliable. You can send another request on a delay and it might give better updates?

blaskovicz commented 6 years ago

@ColinMcNeil I sent a request this morning after about 6 hours and it still says the light is on. This is going to be a big issue for my app if the results are never updated. :/

I ran into a similar issue with Nest API, but the cache latency wasn't forever bad...

Any ideas?

blaskovicz commented 6 years ago

We can move that discussion over here: https://github.com/blaskovicz/tplink-cloud-api/issues/3

blaskovicz commented 6 years ago

Addressed in #19 .

still not sure what's going on with the device list status caching, @adumont have you run into that?

blaskovicz commented 6 years ago

I realized that asking for the system info of a device (system: { get_sysinfo: null }), will return the actual state ({light_state: { on_off: 0 }}), whereas getDeviceList perpetually returns { status: 1}

blaskovicz commented 6 years ago

added more functions for .powerOn and .powerOff for bulbs

adumont commented 6 years ago

Addressed in #19 by @blaskovicz (Thanks)