Skip to content

Conversation

@mib1185
Copy link
Collaborator

@mib1185 mib1185 commented Aug 11, 2024

With this we can wait on every set_ method afterwards for txbusy becoming 0 again, 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 their txbusy is 0 right 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

@coveralls
Copy link

coveralls commented Aug 11, 2024

Coverage Status

coverage: 95.751% (+1.1%) from 94.652%
when pulling 7b0203b on mib1185:add-wait-for-txbusy
into 9f645b2 on hthiery:master.

Copy link
Contributor

@flabbamann flabbamann left a 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 👍

txbusy = dom.findall("txbusy")
if txbusy[0].text == "0":
return True
time.sleep(0.1)
Copy link
Contributor

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?

Copy link
Collaborator Author

@mib1185 mib1185 Oct 27, 2024

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 👍

@t785634
Copy link

t785634 commented Jan 8, 2025

Is there a plan for when this PR will be merged? Maybe it will solve the HA issue.

@mib1185
Copy link
Collaborator Author

mib1185 commented Jan 8, 2025

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

@mib1185 mib1185 closed this Jan 12, 2025
@mib1185 mib1185 reopened this Jan 12, 2025
@flabbamann
Copy link
Contributor

Hi @mib1185,
merging #106 and #107 should fix the CI issues 😊.

@mib1185
Copy link
Collaborator Author

mib1185 commented Jan 12, 2025

I've meanwhile started with #108 to solve all these issues 🙈

@mib1185 mib1185 force-pushed the add-wait-for-txbusy branch from db48017 to 862b3b3 Compare January 12, 2025 22:04
add tests

increase codeclimate argument-count threshold to 5

increase sleep from 0.1 to 0.2

add some additional tests
@mib1185 mib1185 force-pushed the add-wait-for-txbusy branch from 862b3b3 to 7b0203b Compare January 19, 2025 12:02
@mib1185
Copy link
Collaborator Author

mib1185 commented Jan 19, 2025

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.

@mib1185 mib1185 merged commit c2d065e into hthiery:master Jan 19, 2025
12 of 13 checks passed
@mib1185 mib1185 deleted the add-wait-for-txbusy branch January 19, 2025 12:06
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.

4 participants