zigpy / bellows

A Python 3 project to implement EZSP for EmberZNet devices
GNU General Public License v3.0
177 stars 87 forks source link

Only wait for send status confirmation for unicasts #537

Closed puddly closed 1 year ago

puddly commented 1 year ago

This allows for the downstream application to send two consecutive group commands without waiting for a send confirmation for the first.

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e700caf) 99.75% compared to head (350936d) 99.75%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #537 +/- ## ======================================= Coverage 99.75% 99.75% ======================================= Files 57 57 Lines 4537 4539 +2 ======================================= + Hits 4526 4528 +2 Misses 11 11 ``` | [Impacted Files](https://codecov.io/gh/zigpy/bellows/pull/537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy) | Coverage Δ | | |---|---|---| | [bellows/zigbee/application.py](https://codecov.io/gh/zigpy/bellows/pull/537?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy#diff-YmVsbG93cy96aWdiZWUvYXBwbGljYXRpb24ucHk=) | `100.00% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zigpy)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

TheJulianJES commented 1 year ago

Changes seem to work as expected btw

pipiche38 commented 1 year ago

I don't know how it works with ezsp, but my experience on zigate is that by relying on this we were exhausting internal resources PDUs So it might be interesting to see the impact on large heavy network. And maybe having an easy way to disable this

puddly commented 1 year ago

On EmberZNet the only effect is that you run into NETWORK_BUSY more quickly, which is expected since more group commands can be sent at once (e.g. group light color commands usually send on() + move_to_color()).

pipiche38 commented 1 year ago

And which layer handle the NETWORK_BUSY issue ? Is zigpy will do the retry or is that the above layer ?

puddly commented 1 year ago

Bellows tries three times if it's a transient error: https://github.com/zigpy/bellows/blob/e700caf2a50454d0060c3fcd5f67f4b49ebfce46/bellows/zigbee/application.py#L760-L786