Tunnel Refactoring #443
Tunnel Refactoring #443
Conversation
farmio
force-pushed
the
knxip-interface
branch
from
c49488b
to
59009c3
Compare
There was a problem hiding this comment.
I like the idea, however there are still some open points and I also haven't tested this yet.
It would make sense to add a dedicated context manager.
Also, from experience I can safely say that refactoring a mostly untested class is very highly going to cause integration issues.
xknx/io/tunnel.py
Outdated
| # | ||
| # CONNECT DISCONNECT | ||
| # | ||
| #################### | ||
|
|
||
| async def start(self): |
There was a problem hiding this comment.
I like the idea. Since you now have dedicated start and stop functions it would probably make sense to add an async context manager to it and adjust the examples as well?
There was a problem hiding this comment.
I sure like the idea. But for now I aim for a stable and reliable connection. Since this is not very likely to be used directly I think syntactic sugar can be added later.
examples/example_tunnel.py
Outdated
| @@ -34,16 +34,14 @@ async def main(): | |||
| gateway_port=gateway.port, | |||
| ) | |||
|
|
|||
| await tunnel.connect_udp() | |||
| await tunnel.connect() | |||
| await tunnel.start() | |||
farmio
force-pushed
the
knxip-interface
branch
from
59009c3
to
6fad017
Compare
|
I think it works fine now. Maybe someone else can test it for a while? One thing I still want to change here is:
But this should maybe be done in an extra PR. |
farmio
force-pushed
the
knxip-interface
branch
from
7ae5ebe
to
f299d45
Compare
| try: | ||
| await self.udp_client.connect() | ||
| await self._connect_request() | ||
| except (OSError, CommunicationError) as ex: |
- Tunnel connections process DisconnectRequest now and closes/reconnects the tunnel - xknx.connected Event can be used in future to await for a working connection - lower heartbeat rate from every 15sec to 70 sec
farmio
force-pushed
the
knxip-interface
branch
from
f299d45
to
255e7ea
Compare
closes #89
TODO:
on Error shut down the whole interface (and UDPClient) and create a new one from KNXIPInterface so the connection_config can be read again. This would enable to connect to (or from) a different IP, KNX/IP Interface or even connection type when one interface fails.
CONNECT_REQUEST_TIMEOUT should be 10 seconds instead of 1 according to the specs
call Tunnel._tunnel_lost() from UDPClient error_received ( callback / raise / contextmanager ...) - same for Routing
recreate log messages and errors
write tests!
Known issues:
When using daemon_mode and auto_reconnect=False (which is a very unlikely scenario imho) the XKNX context manager does not return on a lost connection. It hangs waiting for SIGINT. On SIGINT it tries to call Tunnel.stop() which fails.
This probably needs a lot of testing under rough conditions 😬