jimmywarting / wemo-client

JavaScript client library for controlling and subscribing to Wemo Devices
MIT License
132 stars 40 forks source link

Incorrect enddevice.internalState['10006'] reported on Bulb #16

Closed rudders closed 8 years ago

rudders commented 8 years ago

Have you noticed discrepancies between the actual state of a bulb (on/off) and what is returned in the interState? I think the Wemo iOS gets it wrong occasionally too so probably an platform issue. My try my getter to see if I can get a more reliable result...

Specifically we get a 0 when the build is clearly illuminated!!!

{ '10006': '0',
'10008': '145:0',
'30008': '',
'30009': '',
'3000A': '' } ```
timonreinhard commented 8 years ago

Can you give any instructions how to reproduce? I guess it may be wrong when turning on the bulb manually (by powering it), but I didn't yet experience any issues with the device state.

rudders commented 8 years ago

That's interesting - reproducing it seems pretty random, I haven't found a pattern but it you've not seen it I'll continue to try and track it down! I'll get back to you!

rudders commented 8 years ago

And now I'm getting the wrong brightness from the client initialisation call..... I'll investigate! Demo iOS app is reporting the wrong value also so it's not us it's Belkin.

timonreinhard commented 8 years ago

I noticed the same oddities when looking at the deviceInfo returned by the discovery. Seems those initial states are reported incorrectly by the Wemo device – it's just the events that report correct values for the device state. I'll add this as a known issue to the docs as it doesn't make much sense to work around those device bugs in the client API, imo.

timonreinhard commented 8 years ago

I hacked together some e2e tests for this issue ($ npm run test-e2e), it turned out they can't be reproduced within my setup: getDeviceStatus reliably returns correct values for me. Please feel free to reopen this issue if those tests fail for you.

rudders commented 8 years ago

So are you saying that you don't think it's a problem Timon? I'll try your testjng and see if can reproduce. One more question though, is it expected to get a statusCgange event multiple times ? I have two bulbs so I get 2 calls and I have noticed another log where someone with three gulf gets three calls - for a single change to a single bulb. The calls are all for the same enddevice and are all the same and aren't a problem, but wondering is this is expected behaviour.

timonreinhard commented 8 years ago

I'd stay away from the capability values in endDeviceInfo and rely on what getDeviceStatus and the statusChange events report. That way it isn't a problem and anyway we can't do anything about it as the issue is rooted in Wemo's device API. I added a comment to the docs to make that clear (be05a11). Agree?

Regarding the statusChange events fired multiple times: That should not happen. Sounds a bit like you created a new instance of the WemoClient for each bulb?

rudders commented 8 years ago

Ahhh, yes I did! That's a fail then. D'oh!!!!! I'll address that and then chase down my other issues. wrt Get status etc I'll refactor based on that advice. Thanks!

timonreinhard commented 8 years ago

I looked into it and actually it was a bug in the client. Subscriptions were established each time an event listener was bound. Just released 0.6.1 which contains this fix.

rudders commented 8 years ago

Thanks for that but your advice is to use the same Link Client for every enddevice on that Link? I had two bulbs but one died so I can't test at the moment. Presumably need to allow for multiple Links with their own Clients?

timonreinhard commented 8 years ago

I had a quick peek at your homebridge module and think you used the client library as intended already. Wemo.client() manages those client instances for you, so you will always get a unique instance per deviceInfo. So to sum it up: Yes, use a single client for all connected end-devices as you already did by getting the respective client via Wemo.client(deviceInfo). So after bumping to 0.6.1 everything should work (you may stumble over some minor API alignments I did recently, e.g. internalState is now capabilities, but that's in the current README).

rudders commented 8 years ago

Thanks Timon. Appreciate your help mate. Cheers.