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

Support configuring multiple devices #14

Merged
merged 1 commit into from Mar 3, 2019

Conversation

Projects
None yet
2 participants
@mairas
Copy link
Owner

mairas commented Mar 3, 2019

Configuration file format has been modified to support multiple devices.

Note that this PR breaks existing configurations.

Also, support for multiple devices is still somewhat flaky. I think there's some kind of a concurrency issue in the upstream broadlink package due to simultaneous calls to get_full_status in multiple threads. The component is able to recover from those errors so I'll be just opening an issue as a reminder.

@mairas mairas requested a review from mcc05 Mar 3, 2019

@mcc05

mcc05 approved these changes Mar 3, 2019

Copy link
Collaborator

mcc05 left a comment

All looks good to me, happy with the changes.
Unfortunately I don't have multiple devices to test with at present.

On the threading issue on update, why not try a random small delay on each call on top the update interval such as

import time
import random
time.sleep(random.uniform(0.1, 0.5))

@mcc05 mcc05 merged commit 8247752 into master Mar 3, 2019

@mairas

This comment has been minimized.

Copy link
Owner Author

mairas commented Mar 3, 2019

Such a delay would probably hide the problem, yes, but I think it's better to fix it in broadlink itself. Now that I have a rough idea about what the problem might be, it shouldn't be too difficult to create a separate minimal test case and fix the root cause. (Famous last words.)

@mcc05

This comment has been minimized.

Copy link
Collaborator

mcc05 commented Mar 3, 2019

ok let me know how you get on

@mcc05

This comment has been minimized.

Copy link
Collaborator

mcc05 commented Mar 4, 2019

@mairas, so just to let you know I created 10 thermostats all pointing at the same physical device, and added the random wait code, and the problem as you say probably masked, but seems to goes away :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.