Rework checkLaptop function

[SharkPlush]

PR opened to fix two bugs.

Bug 1:

/usr/bin/upower -e|grep -E 'AC|BAT' on some devices the line_power path doesn't end with AC we can observe that with my device.

aeon@localhost:~/Downloads> /usr/bin/upower -e|grep -E 'AC|BAT'
/org/freedesktop/UPower/devices/battery_BAT0
aeon@localhost:~/Downloads> /usr/bin/upower -e|grep -E 'line_power|BAT'
/org/freedesktop/UPower/devices/battery_BAT0
/org/freedesktop/UPower/devices/line_power_ADP1

And on some devices there are multiple line_power paths so for example if one path is for a barrel jack adapter and another is for a USB c depending on the one the script selects and the one the user is charging with it might not work properly.

Bug 2:

Multiple battery device might not be handled well because sometimes their batteries charge one after the other not at the same time so when the script checks if the laptop is charging it might miss it by checking the wrong battery. Of course there is the AC check as back up but if you combine that with bug 1 everything breaks.

Solution:

I dropped checking for line_power and switched upower to find "/org/freedesktop/UPower/devices/DisplayDevice" which should "combine" any potential multi batteries into one.

This is also a guaranteed path so it's safe and in theory we can make the code even shorter by just using the full path instead of finding it.

if [[ /usr/bin/upower -i $updevices|awk '/state:/ {print $2}' == discharging ]]; then

If the output is anything but discharging then we can assume the user is charging and continue.

This also prevents any issues on laptop without their battery/s because we can just assume they are already on AC power since their device must be powered without a battery somehow.

I also simplified the code a lot while I was try to fix the bugs.

[SharkPlush]

I did test and here are the outputs if you are curious.

Off charger:
aeon@localhost:~/Downloads> bash -x ./testing.sh 
+ checkLaptop
++ cat /sys/class/dmi/id/chassis_type
+ chassis=10
+ [[ 10 =~ ^(8|9|10|11)$ ]]
++ /usr/bin/upower -e
++ grep -E DisplayDevice
+ updevices=/org/freedesktop/UPower/devices/DisplayDevice
++ /usr/bin/upower -i /org/freedesktop/UPower/devices/DisplayDevice
++ awk '/state:/ {print $2}'
+ [[ discharging == discharging ]]
+ echo 'This is a test'
This is a test

On charger:
aeon@localhost:~/Downloads> bash -x ./testing.sh 
+ checkLaptop
++ cat /sys/class/dmi/id/chassis_type
+ chassis=10
+ [[ 10 =~ ^(8|9|10|11)$ ]]
++ /usr/bin/upower -e
++ grep -E DisplayDevice
+ updevices=/org/freedesktop/UPower/devices/DisplayDevice
++ /usr/bin/upower -i /org/freedesktop/UPower/devices/DisplayDevice
++ awk '/state:/ {print $2}'
+ [[ pending-charge == discharging ]]

[SharkPlush]

If needed I can drop upower in favor of systemd-ac-power -v it will take all available AC lines and check them for power the output will be a simple yes or no. Advantages to this are that it just reads AC power not the battery status and devices with multiple line_power_'s will be checked for AC reliably.

[sysrich]

I think I'd prefer the systemd-ac-power approach, seems cleanest.. are you ok taking care of it?

Thanks so much for taking the initiative on this, really appreciated!

[SharkPlush]

Done here is the test just for confidence sake:

Off AC
aeon@localhost:~/Downloads> bash -x ./test.sh 
+ checkLaptop
++ cat /sys/class/dmi/id/chassis_type
+ chassis=10
+ [[ 10 =~ ^(8|9|10|11)$ ]]
++ systemd-ac-power -v
+ [[ no == no ]]
+ echo 'this is a test'
this is a test

On AC
aeon@localhost:~/Downloads> bash -x ./test.sh 
+ checkLaptop
++ cat /sys/class/dmi/id/chassis_type
+ chassis=10
+ [[ 10 =~ ^(8|9|10|11)$ ]]
++ systemd-ac-power -v
+ [[ yes == no ]]

[SharkPlush]

Wait oops I made a mistake

[SharkPlush]

Ok should be ok now my bad haha

Edit:

I did some more thinking and if you want we can make it super simple like this:

checkLaptop() {
  [[ "$(< /sys/class/dmi/id/chassis_type)" =~ ^(8|9|10|11|14|30|31|32)$ ]] || return 
    if [[ "$(systemd-ac-power -v)" == "no" ]]; then
      log "AC Power disconnected and Battery is not charging"
      d --warning --no-wrap --title="AC Power Recommended" --text="Runnning on battery power detected\n\nIt is recommended to connect the system to AC power during the install"
    fi
}

This also adds extra device types that can possibly have battery this is based off this documentation: (Check 7.4.1 System Enclosure or Chassis Types)

https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.8.0.pdf

Some of these device like "14 / Sub Notebook" are for outdated devices but some like "31 / Convertible" are modern device that have a battery.

Doing it like this allow us to get rid of the displayACWarningMsg() function and support a few more devices with potential batteries properly, I personally see it as simplification and less to maintain in the future but also I figured checking in with you would be a good idea since its your project and also that it won't break anything.