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

fix: skip unsupported devices #77

Merged

Conversation

cps5155
Copy link
Contributor

@cps5155 cps5155 commented Jan 11, 2019

Fix for issue #68 - add ability to ignore unsupported devices. This doesn't add support for KC120 camera, but by ignoring an unsupported device it allows homebridge-tplink-smarthome (and subsequently the rest of homebridge) to boot/run.

@plasticrake
Copy link
Owner

I actually started looking into this yesterday, trying to decide if I wanted to filter here or in the api package. I'll look at some options

@plasticrake plasticrake self-assigned this Jan 11, 2019
@plasticrake
Copy link
Owner

plasticrake commented Jan 16, 2019

Hi @cps5155,

Can you test the changes I made to your PR and let me know if its working?

I changed it to use the startDiscovery filterCallback. And changed to log and skip a device if the deviceId is missing instead of throwing an error.

Thanks!

Copy link
Contributor Author

@cps5155 cps5155 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @plasticrake,

Changes work well and Homebridge can boot. Two comments, if a may.

  1. Looks addAccessory method is executed every ~10s (looks like because of this.client.on('device-online') executes), and thus it logs to the error log multiple times. Over the course of days while Homebridge is running, this might become unnecessary noise in the error log. Perhaps change it to Debug if it's executing device-online and keep it as error if it's executing in device-new ?

  2. When logging to error it may be beneficial to user if the device IP which is missing the deviceId is also logged in the error. Also something to correlate this error with why you're not seeing your new accessory may be helpful, too.

tl;dr - Yes, the changes work OK and Homebridge can boot

@cps5155
Copy link
Contributor Author

cps5155 commented Jan 17, 2019

Looks good -- no error message is seen, and the unsupported device doesn't show up with the rest of TP-Link Accessories when it boots up. Previously below could be seen in homebridge.log, but now is no longer seen - its as if the device doesn't exist on the network at all. Homebridge can boot appropriately so this should resolve #68 appropriately IMO.

[2019-1-16 23:37:07] [TplinkSmarthome] New Device Online: [undefined] device [undefined] 192.168.1.99 9999

@plasticrake plasticrake merged commit 1faafbf into plasticrake:master Jan 17, 2019
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

Successfully merging this pull request may close these issues.

2 participants