-
Couldn't load subscription status.
- Fork 38
Add wait_device_txbusy method #103
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mib1185 I don't know if this is the best solution, but I don't have a better Idea. Let's go for it and see if this can fix the HA issue. Thank you 👍
pyfritzhome/fritzhome.py
Outdated
| txbusy = dom.findall("txbusy") | ||
| if txbusy[0].text == "0": | ||
| return True | ||
| time.sleep(0.1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you how long the execution usually takes? In the linked issue people talk about 1-2 seconds. With the values you choose here, they would get false after 1s.
Also I don't know how many calls the fritzbox can handle, especially older models.
I suggest setting sleep to 0.2. This should still be fast enough and has a total wait time up to 2s. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the execution will take quite less than one second (it is an assumption, based on the permanent connection of these devices and that everything longer would be a very poor user experience).
The 1-2s are mostly the time between toggle the switch in HA and the state of the switch toggles back - the time between toggle the switch in HA and the real device action is mostly much shorter from the user perspective.
unfortunately I can't test it, since i do not own any of these stationary fritz dect devices. To limit the load on the fritzbox, i've also implemented get_device_infos method, which should only query the single device, not all.
Anyway, I agree with increasing to 0.2s, since this should still be ok from a UX perspective 👍
|
Is there a plan for when this PR will be merged? Maybe it will solve the HA issue. |
|
it can't be merged in the current state, since the test ci fails. I just didn't find the time yet to resolve this |
|
I've meanwhile started with #108 to solve all these issues 🙈 |
db48017 to
862b3b3
Compare
add tests increase codeclimate argument-count threshold to 5 increase sleep from 0.1 to 0.2 add some additional tests
862b3b3 to
7b0203b
Compare
|
I'm going to merge this now, even the codeclimate check fails. These findings would need a bigger refactoring, which is out of scope of this PR. |
With this we can wait on every
set_method afterwards fortxbusybecoming0again, which indicates, that the command execution has been finished. This will most properly affect stationary powered devices (eq. Fritz!DECT 200 plugs or Fritz!DECT 500 bulbs), since they start the command execution immediately, while for battery powered devices (eq. Fritz!DECT 301 thermostats) the commands are queued in the Fritzbox and executed as soon as the devices wake up, therefore theirtxbusyis0right away, until they wake up.To avoid a breaking change, the wait method is disabled per default for any
set_method.reference: home-assistant/core#113749