pop-os / system76-driver

System76 Driver for Pop!_OS
Other
112 stars 29 forks source link

udev power_supply rule for system76-adjust-tdp fires on mouse battery changes #202

Open rkern opened 3 years ago

rkern commented 3 years ago

Distribution (run cat /etc/os-release):

PRETTY_NAME="Pop!_OS 20.10"

Related Application and/or Package Version (run apt policy $PACKAGE NAME):

system76-driver:
  Installed: 20.04.31~1617661455~20.10~b3a83f8

Issue/Bug Description:

I use a Logitech MX Master 3 mouse with the "Unifying Receiver" USB dongle on my Oryx Pro laptop. I've noticed periodic mouse stuttering for some time now. I eventually tracked it down that it co-occurs with system76-adjust-tdp running over-frequently.

The current udev rules look at power_supply subsytem change events in order to determine whether the laptop is running on battery or AC. However, the battery on my mouse also reports its status through udev events that trigger these rules too (specifically, it has both SUBSYSTEM=="power_supply" and ATTR{online}).

Steps to reproduce (if you know):

Run tail -f /var/log/syslog in order to verify the at jobs that system76-adjust-tdp creates flow past, sending "Job terminated" emails to root. Then monitor the relevant udev events, and wiggle the mouse some to use up some battery and trigger events.

❯ sudo udevadm monitor -u -p -s power_supply
...
UDEV  [81286.405651] change   /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.2/0003:046D:C52B.0020/0003:046D:4082.0021/power_supply/hidpp_battery_4 (power_supply)
ACTION=change
DEVPATH=/devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.2/0003:046D:C52B.0020/0003:046D:4082.0021/power_supply/hidpp_battery_4
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=hidpp_battery_4
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_ONLINE=0
POWER_SUPPLY_STATUS=Unknown
POWER_SUPPLY_SCOPE=Device
POWER_SUPPLY_MODEL_NAME=Wireless Mouse MX Master 3
POWER_SUPPLY_MANUFACTURER=Logitech
POWER_SUPPLY_SERIAL_NUMBER=4082-83-0e-81-03
POWER_SUPPLY_CAPACITY_LEVEL=Unknown
SEQNUM=6436
USEC_INITIALIZED=81286403707

UDEV  [81286.484854] change   /devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.2/0003:046D:C52B.0020/0003:046D:4082.0021/power_supply/hidpp_battery_4 (power_supply)
ACTION=change
DEVPATH=/devices/pci0000:00/0000:00:14.0/usb1/1-3/1-3:1.2/0003:046D:C52B.0020/0003:046D:4082.0021/power_supply/hidpp_battery_4
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=hidpp_battery_4
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_ONLINE=1
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_SCOPE=Device
POWER_SUPPLY_MODEL_NAME=Wireless Mouse MX Master 3
POWER_SUPPLY_MANUFACTURER=Logitech
POWER_SUPPLY_SERIAL_NUMBER=4082-83-0e-81-03
POWER_SUPPLY_CAPACITY_LEVEL=Full
SEQNUM=6437
USEC_INITIALIZED=81286460762

And unplug and replug the AC power to see the events that we want to respond to:

UDEV  [81491.994725] change   /devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0003:00/power_supply/AC (power_supply)
ACTION=change
DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0003:00/power_supply/AC
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=AC
POWER_SUPPLY_TYPE=Mains
POWER_SUPPLY_ONLINE=0
SEQNUM=6443
USEC_INITIALIZED=81491992795

UDEV  [81492.822777] change   /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0 (power_supply)
ACTION=change
DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=BAT0
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_VOLTAGE_MIN_DESIGN=15200000
POWER_SUPPLY_VOLTAGE_NOW=16774000
POWER_SUPPLY_CURRENT_NOW=1303000
POWER_SUPPLY_CHARGE_FULL_DESIGN=3500000
POWER_SUPPLY_CHARGE_FULL=3626000
POWER_SUPPLY_CHARGE_NOW=3626000
POWER_SUPPLY_CAPACITY=100
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_MODEL_NAME=BAT
POWER_SUPPLY_MANUFACTURER=Notebook
POWER_SUPPLY_SERIAL_NUMBER=0001
SEQNUM=6444
USEC_INITIALIZED=81492822614

UDEV  [81493.142330] change   /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0 (power_supply)
ACTION=change
DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=BAT0
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_STATUS=Discharging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_VOLTAGE_MIN_DESIGN=15200000
POWER_SUPPLY_VOLTAGE_NOW=16774000
POWER_SUPPLY_CURRENT_NOW=1303000
POWER_SUPPLY_CHARGE_FULL_DESIGN=3500000
POWER_SUPPLY_CHARGE_FULL=3626000
POWER_SUPPLY_CHARGE_NOW=3626000
POWER_SUPPLY_CAPACITY=100
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_MODEL_NAME=BAT
POWER_SUPPLY_MANUFACTURER=Notebook
POWER_SUPPLY_SERIAL_NUMBER=0001
SEQNUM=6445
USEC_INITIALIZED=81493142199

UDEV  [81494.424886] change   /devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0003:00/power_supply/AC (power_supply)
ACTION=change
DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0003:00/power_supply/AC
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=AC
POWER_SUPPLY_TYPE=Mains
POWER_SUPPLY_ONLINE=1
SEQNUM=6446
USEC_INITIALIZED=81494400443

UDEV  [81495.257941] change   /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0 (power_supply)
ACTION=change
DEVPATH=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT0
SUBSYSTEM=power_supply
POWER_SUPPLY_NAME=BAT0
POWER_SUPPLY_TYPE=Battery
POWER_SUPPLY_STATUS=Charging
POWER_SUPPLY_PRESENT=1
POWER_SUPPLY_TECHNOLOGY=Li-ion
POWER_SUPPLY_CYCLE_COUNT=0
POWER_SUPPLY_VOLTAGE_MIN_DESIGN=15200000
POWER_SUPPLY_VOLTAGE_NOW=16523000
POWER_SUPPLY_CURRENT_NOW=372000
POWER_SUPPLY_CHARGE_FULL_DESIGN=3500000
POWER_SUPPLY_CHARGE_FULL=3626000
POWER_SUPPLY_CHARGE_NOW=3587000
POWER_SUPPLY_CAPACITY=98
POWER_SUPPLY_CAPACITY_LEVEL=Normal
POWER_SUPPLY_MODEL_NAME=BAT
POWER_SUPPLY_MANUFACTURER=Notebook
POWER_SUPPLY_SERIAL_NUMBER=0001
SEQNUM=6448
USEC_INITIALIZED=81495257818

Expected behavior:

system76-adjust-tdp should only be triggered when the laptop's battery is (dis)engaged, not any other power supplies with ATTR{online}. I suspect adding ATTR{type}=="Mains" would do it (at least for how these devices are set up on my laptop).

Other Notes:

My mouse stuttering may not be be specifically caused by system76-adjust-tdp running, but it doesn't hurt to make the rule correctly-specified. Researching this, it seems like a lot of Logitech peripherals report their battery status similarly, and a number of under-specified rules intended for the main power supply tend to get triggered.