cjbassi / gotop

A terminal based graphical activity monitor inspired by gtop and vtop
GNU Affero General Public License v3.0
7.38k stars 444 forks source link

exit (crash) immediately to shell if -b is passed #100

Open bubbleguuum opened 5 years ago

bubbleguuum commented 5 years ago

Starting gotop -b exits to shell immediately (see error log below). It seems to try to read files in /sys/class/power_supply/hidpp_battery_0 that do not exist (although that directory exists).

Required information:

Also please copy or attach the following file if it exists and contains logs:

22:20:11 battery.go:30: failed to get battery info from system: [{Current:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Full:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Design:open /sys/class/power_supply/hidpp_battery_0/voltage_min_design: no such file or directory ChargeRate:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Voltage:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory DesignVoltage:open /sys/class/power_supply/hidpp_battery_0/voltage_min_design: no such file or directory}]
22:20:11 battery.go:56: failed to get battery info from system: [{Current:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Full:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Design:open /sys/class/power_supply/hidpp_battery_0/voltage_min_design: no such file or directory ChargeRate:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory Voltage:open /sys/class/power_supply/hidpp_battery_0/voltage_now: no such file or directory DesignVoltage:open /sys/class/power_supply/hidpp_battery_0/voltage_min_design: no such file or directory}]
panic: runtime error: index out of range

goroutine 1 [running]:
github.com/cjbassi/gotop/src/termui.(*LineGraph).Draw(0xc000090540, 0xc000104150)
    /home/cjbassi/playground/gotop/src/termui/linegraph.go:82 +0xdfe
github.com/gizak/termui.(*Grid).Draw(0xc000464000, 0xc000104150)
    /home/cjbassi/.local/share/go/pkg/mod/github.com/gizak/termui@v0.0.0-20190114080300-043950fed066/grid.go:154 +0x1a3
github.com/gizak/termui.Render(0xc0004efec8, 0x1, 0x1)
    /home/cjbassi/.local/share/go/pkg/mod/github.com/gizak/termui@v0.0.0-20190114080300-043950fed066/render.go:22 +0x1c0
main.main()
    /home/cjbassi/playground/gotop/main.go:468 +0x3a5
cjbassi commented 5 years ago

Can you paste the output of ls -Al /sys/class/power_supply/hidpp_battery_0?

bubbleguuum commented 5 years ago
ls -Al /sys/class/power_supply/hidpp_battery_0
lrwxrwxrwx 1 root root 0 Jan 28 11:46 /sys/class/power_supply/hidpp_battery_0 -> ../../devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.2/0003:046D:C52B.0003/0003:046D:4041.0005/power_supply/hidpp_battery_0

And the content of the whole directory:

ls -l /sys/class/power_supply/hidpp_battery_0/
total 0
-r--r--r-- 1 root root 4096 Jan 28 11:47 capacity_level
lrwxrwxrwx 1 root root    0 Jan 28 11:47 device -> ../../../0003:046D:4041.0005
-r--r--r-- 1 root root 4096 Jan 28 11:47 manufacturer
-r--r--r-- 1 root root 4096 Jan 28 11:47 model_name
-r--r--r-- 1 root root 4096 Jan 28 11:47 online
drwxr-xr-x 2 root root    0 Jan 28 11:47 power
lrwxrwxrwx 1 root root    0 Jan 28 11:47 powers -> ../../../0003:046D:4041.0005
-r--r--r-- 1 root root 4096 Jan 28 11:47 scope
-r--r--r-- 1 root root 4096 Jan 28 11:47 serial_number
-r--r--r-- 1 root root 4096 Jan 28 11:47 status
lrwxrwxrwx 1 root root    0 Jan 28 11:47 subsystem -> ../../../../../../../../../../class/power_supply
-r--r--r-- 1 root root 4096 Jan 28 11:47 type
-rw-r--r-- 1 root root 4096 Jan 28 11:47 uevent
bubbleguuum commented 5 years ago

I have the explanation. I also have the Logitech USB unifying receiver connected to my laptop. It reports battery level of the wireless mouse using it, and /sys/class/power_supply/hidpp_battery_0/ is associated to it. The real laptop battery is in /sys/class/power_supply/BAT0. Distro is openSUSE Tumbleweed if that matters. If I unplug the USB receiver, gotop -b works fine and picks BAT0.

cjbassi commented 5 years ago

Glad you figured that out, that's an interesting situation.

I just pushed a commit that should make sure gotop doesn't panic anymore with the battery widget by correctly handling an error we were receiving, but I still need to fix the error.

So the deal is that we're using distatus/battery to fetch battery information, and it tries to gather information like voltage etc, in addition to current usage. But voltage info isn't provided for your mouse. We don't need that info anyway tho, so I'm thinking we can just fork that project and remove the voltage stuff.

bubbleguuum commented 5 years ago

Great !
This USB battery device with no voltage should probably be discarded, because the real one we want is the laptop battery (BAT0). In other terms, I would not want the battery widget to display the mouse battery rather than the laptop battery.

cjbassi commented 5 years ago

Looks like there's already an open issue at distatus/battery: https://github.com/distatus/battery/issues/8.

KenjiTakahashi commented 5 years ago

Hey! The behaviour you're experiencing is, technically, correct. There's a battery(-like) device, but it lacks most of the battery info, hence it is reported, but mostly with errors. To workaround it, you should use granular error checking. I have just pushed an update for it to the CLI part: https://github.com/distatus/battery/commit/c96c6467d36a9e77c90af10722c8b47710cfcb38. Feel free to ping me, if you have any questions!

[BTW: The reason why most errors talk about voltage is because if battery fails to find info in Watts, it falls back to trying Amps*Volts, hence voltage error comes up as the last one. Just FYI ;-).]