ValueError exception on `async_get_rx` using telnet

[JJdeVries]

Hi,

I've been using the asuswrt integration on homeassistant, but the logging has been overflowing with a ValueError exception:

Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 278, in async_update_ha_state
    await self.async_device_update()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 469, in async_device_update
    await self.async_update()  # type: ignore
  File "/usr/src/homeassistant/homeassistant/components/asuswrt/sensor.py", line 90, in async_update
    await super().async_update()
  File "/usr/src/homeassistant/homeassistant/components/asuswrt/sensor.py", line 68, in async_update
    self._rates = await self._api.async_get_bytes_total()
  File "/usr/local/lib/python3.8/site-packages/aioasuswrt/asuswrt.py", line 336, in async_get_bytes_total
    rx = await self.async_get_rx()
  File "/usr/local/lib/python3.8/site-packages/aioasuswrt/asuswrt.py", line 344, in async_get_rx
    return float(data[0]) if data[0] != '' else None
ValueError: could not convert string to float: '/net/eth2/statistics/rx_bytes\r'

The issue is present on the master version. I've been interfacing with an Asus RT-AC51U, firmware version '3.0.0.4.380_8591'. As this router does not support ssh (at least I cannot find it in the configuration) I've been interfacing with telnet which is supported.

I can reproduce the issue easily using this script:

#!/usr/bin/env python3
import asyncio
import logging

import sys

from aioasuswrt.asuswrt import AsusWrt

component = AsusWrt('192.168.2.1', 23, username='admin', password='....', interface='eth2', use_telnet=True)
logging.basicConfig(stream=sys.stdout, level=logging.DEBUG)
logger = logging.getLogger(__name__)

async def print_data():
    logger.debug("get_rx_stuff")
    logger.debug(await component.async_get_rx())

loop = asyncio.get_event_loop()

loop.run_until_complete(print_data())
loop.close()

The main issue is (as expected) in the async_run_command of the TelnetConnection class. I've added logging to the method to see what happens:

DEBUG:__main__:get_rx_stuff
The input is: (cat /sys/class/net/eth2/statistics/rx_bytes)
The send command is: (b'PATH=$PATH:/bin:/usr/sbin:/sbin && cat /sys/class/net/eth2/statistics/rx_bytes\n')
The data before splitting: b' PATH=$PATH:/bin:/usr/sbin:/sbin && cat /sys/class\r\r\n/net/eth2/statistics/rx_bytes\r\n81115610\r\nadmin@RT-AC51U:/tmp/home/root#'
The data after splitting: [b'/net/eth2/statistics/rx_bytes\r', b'81115610\r']
[b'/net/eth2/statistics/rx_bytes\r', b'81115610\r']

My expectation is that the main cause of problems is the \r\r\n (especially the \n) causing an extra unexpected split. My best guess is that this is added by someone due to a character limit of 50 per line (as it seems to happen exactly after 50 characters, if I counted correctly).

I've not had any inspiration on how to fix the split to give the correct output, especially as I'm unsure of the exact results of all the different functions calling this method.

For the rx (and probably tx) methods specifically this problem might be easily fixed in the calling method, eg. by replacing lines 344 and 350

return float(data[0]) if data[0] != '' else None

with

return float(data[-1]) if data[-1] != '' else None

Although I'm unsure whether this will cause problems for the ssh version.

[JJdeVries]

Another approach could be to do

try:
    return float(data[0])
except ValueError:
    return None

This will at least keep the behavior identical, although the rx/tx/transmit sensors will not work for the telnet version (or at least for me). I do think the homeassistant asuswrt than needs to be updated to properly handle None values in the tuple returned from async_get_bytes_total, namely these two lines: https://github.com/home-assistant/core/blob/dev/homeassistant/components/asuswrt/sensor.py#L161 https://github.com/home-assistant/core/blob/dev/homeassistant/components/asuswrt/sensor.py#L184

If you state preference for either approach I can make the pull request(s), or if you have idea's on finding out the cause for the added \r\r\n I might have time to further dive into it.

[kennedyshead]

I would like the fix moved to the telnet connection (if possible), so far I have not gotten any great ideas on how to achieve this.

[kennedyshead]

And I'm thinking about rewrite the tx and rx see #44

[JJdeVries]

Hmm yeah makes sense that the solution would be preferable in the Telnet connection.

Interestingly enough the \r\r\n addition only seems to be added to the command part. I was playing around with the \proc\net\dev stuff and got this output:

DEBUG:__main__:get dev stuff                                                                                                                                                                                                             
The input is: (cat /proc/net/dev)                                                                                                                                                                                                        
The send command is: (b'PATH=$PATH:/bin:/usr/sbin:/sbin && cat /proc/net/dev\n')                                                                                                                                                         
The data before splitting:                                                                                                                                                                                                               
 PATH=$PATH:/bin:/usr/sbin:/sbin && cat /proc/net/                                                                                                                                                                                       
dev                                                                                                                                                                                                                                      
Inter-|   Receive                                                |  Transmit                                                                                                                                                             
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed                                                                                                               
    _.... nicely formatted output here ...._
admin@RT-AC51U:/tmp/home/root#

The cutoff of the command

 PATH=$PATH:/bin:/usr/sbin:/sbin && cat /proc/net/dev    

again happens exactly at 50 characters.

EDIT: actually I forgot about the self._prompt_string, which means the actual cut-off happens at 80 characters (which is also usually the default width for terminals).

If this happens consistently for all telnet connections we could extend the data.split('\n')[1:-1], a bit obfuscated could be something like data.split('\n')[ceil(len(self._prompt_string + command) / 80): -1]. Another approach could be a more explicit regular expression to capture exactly the command string with added \r\n strings, but for that I would have to brush up on my regular expression knowledge.

[JJdeVries]

So essentially I think that regardless of the reimplementation of the rx/tx method using the /proc/net/dev input, the TelnetConnection should fix the linebreak error found above (as it's happening regardless of the command).

We could even determine the 50 using some arbitrary long command during connect. Something like:

_LENGTH_COMMAND = ":" + " && :" * 100

This could be send to the writer after connection, and from the reader input we should be able to determine what linelength the writer, or the telnet client inserts the linebreaks.

[kennedyshead]

This I like! Open for PR! I dont run telnet, and every bug found in that connection forces me to setup a new testing environment, so what ever you can find and help with here is much appreciated.

[kennedyshead]

So this is fixed now huh?

[JJdeVries]

Yes!