Move to asyncio Protocol instead of reader/writer

[wlcrs]

This is a complete rewrite of the connection system, which is based on a proper asyncio.Protocol implementation instead of doing the reading/writing ourselves.

This rewrite was inspired on the work done by https://github.com/nugget/python-insteonplm, and includes improvements like better fault tolerance on connection loss, retry mechanisms to write to the bus, etc.

It's not very polished (duplicate functionality in Message and RawMessage for example), but it works a lot better on my installation.

As most of the routines have been well tested in the python-insteonplm project, I don't expect much issues.

This fixes #9 for me.

[cereal2nd]

@brefra can you also have a look?

[cereal2nd]

for me this system seems to work correctly can we run pre-commit on all files?

loguru should be removed

[cereal2nd]

some messages do not have data

2021-10-11 10:18:52.345 | DEBUG | velbusaio.raw_message:create:66 - Receive: bytearray(b'\x0f\xfb\x01\x08\xbe(\x00H\xa4*\x9d<\x18\x04') 2021-10-11 10:18:52.346 | DEBUG | velbusaio.protocol:_process_message:150 - RX: RawMessage(priority=fb, address=00, rtr=True, data=b'') ERROR:asyncio:Task exception was never retrieved future: <Task finished name='Task-163' coro=<VelbusProtocol._process_message() done, defined at /home/cereal/velbus-aio/velbusaio/protocol.py:149> exception=IndexError('bytearray index out of range')> Traceback (most recent call last): File "/home/cereal/velbus-aio/velbusaio/protocol.py", line 151, in _process_message await self._message_received_callback(msg) File "/home/cereal/velbus-aio/velbusaio/controller.py", line 57, in _on_message_received await self._handler.handle(msg) File "/home/cereal/velbus-aio/velbusaio/handler.py", line 50, in handle command_value = rawmsg.data[0] IndexError: bytearray index out of range

2021-10-11 10:12:41.617 | DEBUG | velbusaio.protocol:_process_message:156 - RX: RawMessage(priority=fb, address=00, rtr=True, data=b'') ERROR:asyncio:Task exception was never retrieved future: <Task finished name='Task-180' coro=<VelbusProtocol._process_message() done, defined at /home/cereal/velbus-aio/velbusaio/protocol.py:155> exception=IndexError('bytearray index out of range')> Traceback (most recent call last): File "/home/cereal/velbus-aio/velbusaio/protocol.py", line 157, in _process_message await self._message_received_callback(msg) File "/home/cereal/velbus-aio/velbusaio/controller.py", line 57, in _on_message_received await self._handler.handle(msg) File "/home/cereal/velbus-aio/velbusaio/handler.py", line 50, in handle command_value = rawmsg.data[0] IndexError: bytearray index out of range

[cereal2nd]

backuoff < 1.11 needs ot be removed from the requirements and setup file, as this conflicts with home-assistant

[cereal2nd]

i got your code into a base branch for this repo and modied it https://github.com/Cereal2nd/velbus-aio/tree/asyncio-protocol

[wlcrs]

Hi @Cereal2nd ,

Thank you for getting my contribution up to the necessary standards!

I've been looking into the issue wrt the messages with missing data.

If I test with the exact bytes from your logs, I get a perfectly valid RawMessage:

>>> from velbusaio.raw_message import _parse
>>> input=bytearray(b'\x0f\xfb\x01\x08\xbe(\x00H\xa4*\x9d<\x18\x04')
>>> _parse(input)
(RawMessage(priority=fb, address=01, rtr=False, command=190, data=b'be 28 00 48 a4 2a 9d 3c'), bytearray(b''))

In the two first rules of the logging output that you've put in your comment, you can see that also the address and RTR do not match. This really confuses me. Do you happen to have some more extensive logging lying around that I could look into?

I'm almost suspecting a race condition, as it seems that the contents of the rawmessage bytes changes during the parsing. I've been reading the relevant source code that call the get_buffer and buffer_updated methods of asyncio.BufferedProtocol, and can not really imagine how such a race condition can happen.

[cereal2nd]

I just fixed the problem :)

This is a real-time data request message. It's broadcasted and does not use any data. So we can safely ignore it.

Before we continue I would like @brefra to test the branch out before we push. How faster we can do it the faster we get this in hass.

[brefra]

I'll test it tomorrow 👍

[gverbist]

Will this impact connections via velserv?

[cereal2nd]

This will impact every connection type, but will be transparant

[brefra]

I did some testing and discovered serial connection was not working . Created a PR #12 (targeted to the asyncio-protocol branch) to resolve this. Still in the progress of doing some additional testing.