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

request()/broadcast() context manager. #49

Merged
merged 2 commits into from
Jul 8, 2019

Conversation

Adminiuga
Copy link
Collaborator

@Adminiuga Adminiuga commented Jun 21, 2019

zigpy_deconz still may leak transactions, if for example we timeout waiting an aps_data_request like in the example below:

16:02:19 WARNING (MainThread) [zigpy_deconz.api] No response to aps_data_request command

This PR implements a simple context manager that cleans up TSNs

@Adminiuga Adminiuga changed the title request()/broadcast() context manager. WIP request()/broadcast() context manager. Jun 21, 2019
@Adminiuga Adminiuga changed the title WIP request()/broadcast() context manager. request()/broadcast() context manager. Jun 21, 2019
@Adminiuga
Copy link
Collaborator Author

@damarco I did some more testing, if we swamp ConBee with aps_data_requests in short period of time, around 10th request we start getting STATUS.BUSY which leaks TSNs because await api.aps_data_request() throws en exception, set by

if status is not STATUS.SUCCESS.value:
fut.set_exception(Exception('%s, status: %s' % (command, status, )))
return

Maybe we should have a zigpy_deconz exception based on zigpy exception, which we could catch and process accordingly, without too broad Exception.

@Adminiuga Adminiuga changed the title request()/broadcast() context manager. WIP request()/broadcast() context manager. Jun 21, 2019
@Adminiuga Adminiuga force-pushed the fixes/context-manager branch from 846324a to e39359a Compare June 22, 2019 00:46
@Adminiuga Adminiuga force-pushed the fixes/context-manager branch from e39359a to d9c0ebd Compare June 28, 2019 19:39
@Adminiuga Adminiuga changed the title WIP request()/broadcast() context manager. request()/broadcast() context manager. Jun 28, 2019
@Adminiuga Adminiuga requested a review from damarco June 28, 2019 19:40
@Adminiuga Adminiuga merged commit 620475b into zigpy:master Jul 8, 2019
@Adminiuga Adminiuga deleted the fixes/context-manager branch July 8, 2019 00:33
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.

1 participant