kennedyshead / aioasuswrt

MIT License
24 stars 24 forks source link

Sensors don't work on RT-N66U with merlin #21

Closed wdullaer closed 5 years ago

wdullaer commented 5 years ago

I am running the latest home assistant (0.84.6) and the sensors are always returning 0 with my RT-N66U running merlin 380.70

I have done some investigation and home assistant is correctly calling async_update() (https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/sensor/asuswrt.py#L51) in sensors/asuswrt.py

Looking into the implementation of async_get_packets_total, it seems to be failing at this line in this library: https://github.com/kennedyshead/aioasuswrt/blob/master/aioasuswrt/asuswrt.py#L194 Essentially _IP_LINK_CMD is defined as ip -rc 1024 -s link

Running that on the router yields:

admin@RT-N66U:/tmp/home/root# ip -rc 1024 -s link
Option "-rc" is unknown, try "ip -help".

ip -s link does work, I assume returning the result in bits rather than bytes.

A potential fix could be to convert to bytes in python rather than the shell command (there's a lot of math already happening in python already, so it's not really making the solution less elegant I think).

If this sounds sensible to you, I can supply a PR with this fix.

kennedyshead commented 5 years ago

It do sound sensible, and the pr is welcome 👍

aarya123 commented 5 years ago

Was just taking a look at this. Would it make sense just to align with async_get_rx and async_get_tx and use the data stored in /sys/class/net/eth0/statistics/rx_packets and /sys/class/net/eth0/statistics/tx_packets, respectively?

wdullaer commented 5 years ago

I don't think all packets are equal in size, so I'm not sure if those metrics are useful for the sensor data.

I think the output of this command could be useful

cat /proc/net/dev

It gives all the statistics in a tabular form (both bytes and packets sent/received). You can parse this with an awk oneliner.

# Rx bytes
cat /proc/net/dev | awk '/eth0/ {print $1}' | sed 's/^eth0://g' # sed is necessary to trim off the line header

# Tx bytes
cat /proc/net/dev | awk '/eth0/ {print $9}'

which interface would we be interested in? (my wan traffic goes over a pppoe connection, so I would like to see the ppp0 interface)

kennedyshead commented 5 years ago
$ cat /proc/dev/net | awk '/eth0/ {print $1}' | sed 's/^eth0://g'
cat: can't open '/proc/dev/net': No such file or directory

:/

kennedyshead commented 5 years ago
cat /proc/net/dev
Inter-|   Receive                                                |  Transmit
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
    lo: 301680465 1645733    0    0    0     0          0         0 301680465 1645733    0    0    0     0       0          0
  ifb0:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  ifb1:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  fwd0:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  fwd1:       0 139691457    0    0    0     0          0         0 3115462719 52423718    0    0    0     0       0          0
   agg:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  eth0: 1556761086 522308687    0    0    0     0          0         0 2001812636 599581384    0    0    0     0       0          0
 dpsta:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
  eth1: 1042839149 7158195    0    0    0     0          0   1145438 4013927024 236828324    0 1521132    0     0       0          0
  eth2:       0       0    0    0    0     0          0         0 2984728309 139740069    0 26780    0     0       0          0
 vlan1: 85463760460 287041834    0    0    0     0          0    604459 295832141932 337522238    0    0    0     0       0          0
 vlan2:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0
   br0: 10735542464 38276327    0    0    0     0          0         0 7539443015 25531216    0    0    0     0       0          0
 wl0.1: 41478432  324804    0    0    0     0          0     13679 3349966156 5998468    0 438881    0     0       0          0
 tun21:   24632     332    0    0    0     0          0         0     1483      16    0    0    0     0       0          0

Worked though

kennedyshead commented 5 years ago

Would be nice to get errs and drops to!

kennedyshead commented 5 years ago

And for the devices I think we should add all available and then rework homeassistant for an interface option

wdullaer commented 5 years ago

Sorry, I made a typo there and didn't proofread.

I think it would be nice if we could export all this into home assistant and then just read some config to see which ones the user wants to show.

We might want to show more userfriendly names :-)

Design wise, would you want to run multiple shell commands and parse the numbers with awk, or just run the command once and parse in python?

kennedyshead commented 5 years ago

Well, the router is not the best place to run calculations imo, so I’d say it should be done in python.

On 8 Jan 2019, at 17:10, wdullaer notifications@github.com wrote:

Sorry, I made a typo there and didn't proofread.

I think it would be nice if we could export all this into home assistant and then just read some config to see which ones the user wants to show.

We might want to show more userfriendly names :-)

Design wise, would you want to run multiple shell commands and parse the numbers with awk, or just run the command once and parse in python?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/kennedyshead/aioasuswrt/issues/21#issuecomment-452355319, or mute the thread https://github.com/notifications/unsubscribe-auth/AAMaak6xGdaahtnjYrqLtUnTPUvJiBARks5vBMMLgaJpZM4ZybMr.

wdullaer commented 5 years ago

awk is very lightweight and the table is trivially easy to parse with it. I'm not fluent in python, so my gut feel is that it might be harder / more code, but I'll give it a shot.

I propose to first tweak the existing code to use this file (and just report on eth0, like it does now), and then in a second stage try to extend with more config options.