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
Refactor timeout handling #523
Conversation
xknx/knxip/__init__.py
Outdated
__all__ = [ | ||
"KNXIPBody", | ||
"CEMIFrame", | ||
"ConnectRequest", | ||
"ConnectResponse", | ||
"ConnectionStateRequest", | ||
"ConnectionStateResponse", | ||
"DIB", | ||
"DIBDeviceInformation", | ||
"DIBGeneric", | ||
"DIBSuppSVCFamilies", | ||
"DisconnectRequest", | ||
"DisconnectResponse", | ||
"ErrorCode", | ||
"KNXIPHeader", | ||
"HPAI", | ||
"KNXIPFrame", | ||
"APCICommand", | ||
"CEMIFlags", | ||
"CEMIMessageCode", | ||
"ConnectRequestType", | ||
"DIBServiceFamily", | ||
"DIBTypeCode", | ||
"KNXIPServiceType", | ||
"KNXMedium", | ||
"RoutingIndication", | ||
"SearchRequest", | ||
"SearchResponse", | ||
"TunnellingAck", | ||
"TunnellingRequest", | ||
] |
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 don't like this, but it seems
Line 51 in 0defb26
implicit_reexport = true |
isn't doing what I think it should do.
try: | ||
await asyncio.wait_for( | ||
self.response_received_or_timeout.wait(), | ||
timeout=self.timeout_in_seconds, | ||
) | ||
except asyncio.TimeoutError: | ||
logger.warning( | ||
"Error: KNX bus did not respond in time (%s secs) to GroupValueRead request for: %s", | ||
self.timeout_in_seconds, | ||
self.group_address, | ||
) | ||
finally: | ||
# cleanup to not leave callbacks (for asyncio.CancelledError) | ||
self.xknx.telegram_queue.unregister_telegram_received_cb(cb_obj) |
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.
The core of this PR is always this pattern.
Instead of using loop.call_later(self.timeout_in_seconds, self.timeout)
in self.start_timeout()
with self.stop_timeout()
and self.timeout()
we use asyncio.wait_for()
with a timeout argument and move the contents of self.timeout()
into the exception clause so asyncio takes care of handling the timeout for us. This way we can use finally
to always remove our callbacks.
72f492c
to
69435c9
Compare
69435c9
to
4bcb1b2
Compare
a64dd0a
to
a978650
Compare
Description
Fixes # (issue)
Type of change
Checklist: