Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor discovery and offline handling #35

Open
timonreinhard opened this issue Aug 21, 2016 · 9 comments
Open

Refactor discovery and offline handling #35

timonreinhard opened this issue Aug 21, 2016 · 9 comments
Assignees
Milestone

Comments

@timonreinhard
Copy link
Contributor

timonreinhard commented Aug 21, 2016

There current implementation makes it a bit too hard to work with devices appearing and disappearing under unstable network conditions:

  • The client emits events for network errors only after having a subscription for device events established.
  • I'd prefer having the discovery emitting events for the sake of consistency.
  • The current discover method has some smell in it.
  • The specifics of handling errors are completely left to the API consumer.
@timonreinhard timonreinhard added this to the 1.0 milestone Aug 21, 2016
@timonreinhard timonreinhard self-assigned this Aug 21, 2016
@timonreinhard
Copy link
Contributor Author

Will cover #30

@Sawtaytoes
Copy link

Sawtaytoes commented Jul 20, 2017

I've noticed that when doing the discovery, I can start my node instance multiple times and each time I'll get a different list of devices. This means that sometimes a device might temporarily go offline, and I'll no longer be able to control it.

I've also noticed if I wait too long in the code from doing new Wemo() to wemo.discover(cb), no devices will show up. It'd be nice to specify exactly when I want the listener activated and only at that time begin the discovery. It'd also be nice to setup as many listeners as I want if I wanna pass around the wemo object provided I have some separation where certain parts of the code don't need to know about what other parts are doing when devices are discovered.

I'm using this library to control devices in my home and these bugs are getting me everyday. Sometimes I'll lose the ability to turn on/off a device because the library has lost access to it. This might be a case where I need to cache the results of the initial discovery and always re-use those so the device information is available to do a wemo.client(deviceInfo). Dunno how I'd know what to remove from the cache, but that should fix it right? Or is there a way to pull available devices from the WeMo API like IFTTT does and cache that so even if they're not discovered initially, I can still use them since I know they're supposed to be there?

@netslayer
Copy link

Agree with @Sawtaytoes. I'm seeing discovery sometimes pick up devices and if they drop off wifi for some reason than I lose the ability to control them. I run discovery every minute in node and facing an issue right now where the device will not be re-discovered after an error until I restart the node services. I have a feeling there is a bug in that discovery/recovery code somewhere.

@timonreinhard
Copy link
Contributor Author

@netslayer It might give you the client instance for the meanwhile dropped-off device. See index.js#L149

@netslayer
Copy link

I'm going to dig through the code now to see what's happening because it just reproduced again. client.on('error' returns "HTTP 400:

400 Bad Request

" and then that device never appears again on re runs of the discovery. If I restart node it will be found.

@timonreinhard
Copy link
Contributor Author

Interesting. Did you run with debug output enabled and can post the logs?

@netslayer
Copy link

To fix the problem I removed the if check "if (!self._clients[device.UDN] || self._clients[device.UDN].error) {" for checking if the device is already registered. Now when the device returns a 400 and then comes back it still will get discovered on the next discovery run. It appears that as soon as a 400 error occurs the error field is undefined and this if check prevents the re-discovery of the device when it comes back on (say after a network issue).

Thu, 31 May 2018 21:34:15 GMT wemo-client Subscription request failed with HTTP 400
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Error occurred connecting to device at [object Object]
Thu, 31 May 2018 21:34:16 GMT wemo-client device EventEmitter {

and the device object itself has no error defined so it won't be found in the if check for discovery
" error: undefined,"

Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1
Thu, 31 May 2018 21:34:16 GMT wemo-client subscription still pending

Thu, 31 May 2018 21:34:20 GMT wemo-client subscription still pending
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:21 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:23 GMT wemo-client Subscription request failed with HTTP 400
Thu, 31 May 2018 21:34:23 GMT wemo-client Subscription request failed with HTTP 412
Thu, 31 May 2018 21:34:24 GMT wemo-client subscription still pending
Thu, 31 May 2018 21:34:24 GMT wemo-client subscription still pending
Thu, 31 May 2018 21:34:25 GMT wemo-client Subscription request failed with HTTP 400
Thu, 31 May 2018 21:34:35 GMT wemo-client Renewing subscription - Device: uuid:Insight-1_0-231651K12012AB, Service: urn:Belkin:service:basicevent:1

{ address: '10.0.0.51', family: 'IPv4', port: 3097, size: 407 }
Thu, 31 May 2018 21:35:00 GMT wemo-client GetInsightParams Response: { 's:Envelope':
{ '$':
{ 'xmlns:s': 'http://schemas.xmlsoap.org/soap/envelope/',
's:encodingStyle': 'http://schemas.xmlsoap.org/soap/encoding/' },
's:Body': { 'u:GetInsightParamsResponse': [Object] } } }

Thu, 31 May 2018 21:35:01 GMT wemo-client Found device: {"root":{"$":{"xmlns":"urn:Belkin:device-1-0"},"specVersion":{"major":"1","minor":"0"},"device":{"deviceType":"urn:Belkin:device:insight:1","friendlyName":"M1","manufacturer":"Belkin International Inc.","manufacturerURL":"http://www.belkin.com","modelDescription":"Belkin Insight 1.0","modelName":"Insight","modelNumber":"1.0","modelURL":"http://www.belkin.com/plugin/","serialNumber":"231651K1200F6E","UDN":"uuid:Insight-1_0-231651K1200F6E","UPC":"123456789","macAddress":"149182B44E14","firmwareVersion":"WeMo_WW_2.00.11057.PVT-OWRT-InsightV2","iconVersion":"0|49153","binaryState":"0","iconList":{"icon":{"mimetype":"jpg","width":"100","height":"100","depth":"100","url":"icon.jpg"}},"serviceList":{"service":[{"serviceType":"urn:Belkin:service:WiFiSetup:1","serviceId":"urn:Belkin:serviceId:WiFiSetup1","controlURL":"/upnp/control/WiFiSetup1","eventSubURL":"/upnp/event/WiFiSetup1","SCPDURL":"/setupservice.xml"},{"serviceType":"urn:Belkin:service:timesync:1","serviceId":"urn:Belkin:serviceId:timesync1","controlURL":"/upnp/control/timesync1","eventSubURL":"/upnp/event/timesync1","SCPDURL":"/timesyncservice.xml"},{"serviceType":"urn:Belkin:service:basicevent:1","serviceId":"urn:Belkin:serviceId:basicevent1","controlURL":"/upnp/control/basicevent1","eventSubURL":"/upnp/event/basicevent1","SCPDURL":"/eventservice.xml"},{"serviceType":"urn:Belkin:service:firmwareupdate:1","serviceId":"urn:Belkin:serviceId:firmwareupdate1","controlURL":"/upnp/control/firmwareupdate1","eventSubURL":"/upnp/event/firmwareupdate1","SCPDURL":"/firmwareupdate.xml"},{"serviceType":"urn:Belkin:service:rules:1","serviceId":"urn:Belkin:serviceId:rules1","controlURL":"/upnp/control/rules1","eventSubURL":"/upnp/event/rules1","SCPDURL":"/rulesservice.xml"},{"serviceType":"urn:Belkin:service:metainfo:1","serviceId":"urn:Belkin:serviceId:metainfo1","controlURL":"/upnp/control/metainfo1","eventSubURL":"/upnp/event/metainfo1","SCPDURL":"/metainfoservice.xml"},{"serviceType":"urn:Belkin:service:remoteaccess:1","serviceId":"urn:Belkin:serviceId:remoteaccess1","controlURL":"/upnp/control/remoteaccess1","eventSubURL":"/upnp/event/remoteaccess1","SCPDURL":"/remoteaccess.xml"},{"serviceType":"urn:Belkin:service:deviceinfo:1","serviceId":"urn:Belkin:serviceId:deviceinfo1","controlURL":"/upnp/control/deviceinfo1","eventSubURL":"/upnp/event/deviceinfo1","SCPDURL":"/deviceinfoservice.xml"},{"serviceType":"urn:Belkin:service:insight:1","serviceId":"urn:Belkin:serviceId:insight1","controlURL":"/upnp/control/insight1","eventSubURL":"/upnp/event/insight1","SCPDURL":"/insightservice.xml"},{"serviceType":"urn:Belkin:service:smartsetup:1","serviceId":"urn:Belkin:serviceId:smartsetup1","controlURL":"/upnp/control/smartsetup1","eventSubURL":"/upnp/event/smartsetup1","SCPDURL":"/smartsetup.xml"},{"serviceType":"urn:Belkin:service:manufacture:1","serviceId":"urn:Belkin:serviceId:manufacture1","controlURL":"/upnp/control/manufacture1","eventSubURL":"/upnp/event/manufacture1","SCPDURL":"/manufacture.xml"}]},"presentationURL":"/pluginpres.html","host":"10.0.0.51","port":"49153","callbackURL":"http://10.0.0.3:39158"}}}

then it starts to be discovered again with my check hack

@Sawtaytoes
Copy link

Can we get this in the next version? @timonreinhard, do you need someone to make a pull request?

@timonreinhard
Copy link
Contributor Author

PRs are highly appreciated, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants