~kennylevinsen/poweralertd-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
7 2

[PATCH v2] feat: Add option '-o' to ignore notifications over a certain battery-level

Details
Message ID
<20230823113436.326136-4-mail@sebastian-sellmeier.de>
DKIM signature
missing
Download raw message
Patch: +28 -19
First proposal of a feature-request discuessed some time ago in irc as my devices flipps state at around 98/99%.
It does not fix the issue fully as I also would like to supress online/offline notifications of the charger when over certain battery-level,
but at the time I don't have access to the current battery-level as there could be mutliple. For device notifications it only supresses the one
for devices with state over -o value.

Open for feedback/suggestions, as it's my first C-involving contribution :v:

Signed-off-by: Sebastian Sellmeier <mail@sebastian-sellmeier.de>
---
 main.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/main.c b/main.c
index 3dadda1..4ded249 100644
--- a/main.c
+++ b/main.c
@@ -142,19 +142,21 @@ static int send_warning_update(sd_bus *bus, struct upower_device *device) {
}

static const char usage[] = "usage: %s [options]\n"
"  -h				show this help message\n"
"  -s				ignore the events at startup\n"
"  -i <device_type>		ignore this device type, can be use several times\n";
"  -h					show this help message\n"
"  -i <device_type>		ignore this device type, can be use several times\n"
"  -s					ignore the events at startup\n"
"  -o <percentage>		ignores events over specified battery percentage\n";


int main(int argc, char *argv[]) {
	int opt = 0;
	int device_type = 0;
	int ignore_types_mask = 0;
	int ignore_over = 0;
	bool ignore_initial = false;
	bool initialized = false;

	while ((opt = getopt(argc, argv, "hsi:")) != -1) {
	while ((opt = getopt(argc, argv, "hi:so:")) != -1) {
		switch (opt) {
		case 'i':
			device_type = upower_device_type_int(optarg);
@@ -168,6 +170,9 @@ int main(int argc, char *argv[]) {
		case 's':
			ignore_initial = true;
			break;
		case 'o':
			ignore_over = atoi(optarg);
			break;
		case 'h':
		default:
			fprintf(stderr, usage, argv[0]);
@@ -212,23 +217,27 @@ int main(int argc, char *argv[]) {
				goto next_device;
			}

			if (upower_device_has_battery(device)) {
				ret = send_state_update(user_bus, device);
				if (ret < 0) {
					fprintf(stderr, "could not send state update notification: %s\n", strerror(-ret));
					goto finish;
				}
				ret = send_warning_update(user_bus, device);
				if (ret < 0) {
					fprintf(stderr, "could not send warning update notification: %s\n", strerror(-ret));
					goto finish;
			if (ignore_over == 0 || device->current.percentage < ignore_over) {
				if (upower_device_has_battery(device)) {
					ret = send_state_update(user_bus, device);
					if (ret < 0) {
						fprintf(stderr, "could not send state update notification: %s\n", strerror(-ret));
						goto finish;
					}
					ret = send_warning_update(user_bus, device);
					if (ret < 0) {
						fprintf(stderr, "could not send warning update notification: %s\n", strerror(-ret));
						goto finish;
					}
				} else {
					ret = send_online_update(user_bus, device);
					if (ret < 0) {
						fprintf(stderr, "could not send online update notification: %s\n", strerror(-ret));
						goto finish;
					}
				}
			} else {
				ret = send_online_update(user_bus, device);
				if (ret < 0) {
					fprintf(stderr, "could not send online update notification: %s\n", strerror(-ret));
					goto finish;
				}
				fprintf(stderr, "Ignored because -o: %i | percentage: %f\n", ignore_over, device->current.percentage);
			}
next_device:
			device->last = device->current;
-- 
2.41.0
Details
Message ID
<a494b52f-e766-415c-ad79-88250c1d18f5@kl.wtf>
In-Reply-To
<20230823113436.326136-4-mail@sebastian-sellmeier.de> (view parent)
DKIM signature
missing
Download raw message
On 8/23/23 13:34, Sebastian Sellmeier wrote:
> First proposal of a feature-request discuessed some time ago in irc as my devices flipps state at around 98/99%.
> It does not fix the issue fully as I also would like to supress online/offline notifications of the charger when over certain battery-level,
> but at the time I don't have access to the current battery-level as there could be mutliple. For device notifications it only supresses the one
> for devices with state over -o value.

We could do one of two things:

1. Something a bit more elaborate in send_state_update, where we like 
here ignore "safe" transitions. This might be similar to the proposed 
threshold, but let's see what's possible.

Having `upower --monitor-detail` output during the charge/discharge 
transitions would be useful to ensure we have the right states.

2. Have a mode where only low/critical states and events that clear them 
are shown. This is simpler, but more extreme.
Antoine Beaupré <anarcat@orangeseeds.org>
Details
Message ID
<87ttqo7gfu.fsf@angela.anarc.at>
In-Reply-To
<a494b52f-e766-415c-ad79-88250c1d18f5@kl.wtf> (view parent)
DKIM signature
missing
Download raw message
It seems to me that poweralertd could do some "hysterisis", and by that
I mean that poweralertd could simply tolerate discharge signals for a
certain duration of time without triggering a notification.

The proposed patch is an interesting solution, but it requires the user
to actually do something. In my case, I *think* I would use `-o 99`
because that's where I get those annoying warnings all the time. But
really, poweralertd shouldn't need to be configured to do this properly,
IMHO.
Antoine Beaupré <anarcat@orangeseeds.org>
Details
Message ID
<8734rdxux7.fsf@angela.anarc.at>
In-Reply-To
<a494b52f-e766-415c-ad79-88250c1d18f5@kl.wtf> (view parent)
DKIM signature
missing
Download raw message
> Having `upower --monitor-detail` output during the charge/discharge 
> transitions would be useful to ensure we have the right states.

So here's an example, with the patch I see:

Apr 22 11:43:14 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000
Apr 22 11:43:14 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000

and upower says:

[11:43:14.808]	device changed:     /org/freedesktop/UPower/devices/battery_BAT1
  native-path:          BAT1
  vendor:               NVT
  model:                Framewo
  serial:               01A0
  power supply:         yes
  updated:              lun 22 avr 2024 11:43:14 (0 seconds ago)
  has history:          yes
  has statistics:       yes
  battery
    present:             yes
    rechargeable:        yes
    state:               fully-charged
    warning-level:       none
    energy:              48,0018 Wh
    energy-empty:        0 Wh
    energy-full:         48,1096 Wh
    energy-full-design:  55,0088 Wh
    energy-rate:         0,9702 W
    voltage:             17,344 V
    charge-cycles:       45
    percentage:          100%
    capacity:            87,234%
    technology:          lithium-ion
    icon-name:          'battery-full-charged-symbolic'

[11:43:14.811]	device changed:     /org/freedesktop/UPower/devices/battery_BAT1
  native-path:          BAT1
  vendor:               NVT
  model:                Framewo
  serial:               01A0
  power supply:         yes
  updated:              lun 22 avr 2024 11:43:14 (0 seconds ago)
  has history:          yes
  has statistics:       yes
  battery
    present:             yes
    rechargeable:        yes
    state:               fully-charged
    warning-level:       none
    energy:              48,0018 Wh
    energy-empty:        0 Wh
    energy-full:         48,1096 Wh
    energy-full-design:  55,0088 Wh
    energy-rate:         0,9702 W
    voltage:             17,344 V
    charge-cycles:       45
    percentage:          100%
    capacity:            87,234%
    technology:          lithium-ion
    icon-name:          'battery-full-charged-symbolic'

This happens every thirty seconds, which makes the software basically
unusable as it is, because it's generating endless notifications when
the battery is charged.

That's 5000 notifications since last boot, about a week ago, and keep in
mind the laptop sleeps at night:

anarcat@angela:~$ journalctl --user -b -u poweralertd | grep -c Ignored 
5190
anarcat@angela:~$ journalctl --user -b -u poweralertd | head -1
avr 17 10:26:41 angela systemd[3162]: Started poweralertd.service - UPower-powered power alerter.

a.

-- 
A riot is the language of the unheard.
                         - Martin Luther King, Jr.
Details
Message ID
<f0e9b46d-2e0e-48fb-a221-caafb6505c2a@kl.wtf>
In-Reply-To
<8734rdxux7.fsf@angela.anarc.at> (view parent)
DKIM signature
pass
Download raw message
> [11:43:14.808]	device changed:     /org/freedesktop/UPower/devices/battery_BAT1
>    native-path:          BAT1
>    vendor:               NVT
>    model:                Framewo
>    serial:               01A0
>    power supply:         yes
>    updated:              lun 22 avr 2024 11:43:14 (0 seconds ago)
>    has history:          yes
>    has statistics:       yes
>    battery
>      present:             yes
>      rechargeable:        yes
>      state:               fully-charged
>      warning-level:       none
>      energy:              48,0018 Wh
>      energy-empty:        0 Wh
>      energy-full:         48,1096 Wh
>      energy-full-design:  55,0088 Wh
>      energy-rate:         0,9702 W
>      voltage:             17,344 V
>      charge-cycles:       45
>      percentage:          100%
>      capacity:            87,234%
>      technology:          lithium-ion
>      icon-name:          'battery-full-charged-symbolic'
>
> [11:43:14.811]	device changed:     /org/freedesktop/UPower/devices/battery_BAT1
>    native-path:          BAT1
>    vendor:               NVT
>    model:                Framewo
>    serial:               01A0
>    power supply:         yes
>    updated:              lun 22 avr 2024 11:43:14 (0 seconds ago)
>    has history:          yes
>    has statistics:       yes
>    battery
>      present:             yes
>      rechargeable:        yes
>      state:               fully-charged
>      warning-level:       none
>      energy:              48,0018 Wh
>      energy-empty:        0 Wh
>      energy-full:         48,1096 Wh
>      energy-full-design:  55,0088 Wh
>      energy-rate:         0,9702 W
>      voltage:             17,344 V
>      charge-cycles:       45
>      percentage:          100%
>      capacity:            87,234%
>      technology:          lithium-ion
>      icon-name:          'battery-full-charged-symbolic'

Hmm, these are not the events we are looking for, as the states and 
levels are identical. These two events were likely just emitted because 
the "updated" field got a new timestamp due to a new measurement. There 
will also be events when voltage or levels change slightly.

We are only interested in the events correlating with the annoying 
notifications, when changing between the states your system flipflops in 
between. `sudo busctl monitor` might prove more useful, if more noisy, 
as it says exactly what fields updated rather than having to derive it 
from the `upower --monitor-detail` output...
Antoine Beaupré <anarcat@orangeseeds.org>
Details
Message ID
<87zftlweps.fsf@angela.anarc.at>
In-Reply-To
<f0e9b46d-2e0e-48fb-a221-caafb6505c2a@kl.wtf> (view parent)
DKIM signature
missing
Download raw message
Okay let's see then, i have:

Apr 22 12:19:16 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000
Apr 22 12:19:16 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000


and with that same timestamp, i have:

‣ Type=signal  Endian=l  Flags=1  Version=1 Cookie=7186  Timestamp="Mon 2024-04-22 16:19:16.519300 UTC"
  Sender=:1.30  Path=/org/freedesktop/UPower/devices/battery_BAT1  Interface=org.freedesktop.DBus.Properties  Member=PropertiesChanged
  UniqueName=:1.30
  MESSAGE "sa{sv}as" {
          STRING "org.freedesktop.UPower.Device";
          ARRAY "{sv}" {
                  DICT_ENTRY "sv" {
                          STRING "UpdateTime";
                          VARIANT "t" {
                                  UINT64 1713802756;
                          };
                  };
          };
          ARRAY "s" {
          };
  };

everything else in busctl seems unrelated
(org.freedesktop.NetworkManager.AccessPoint, org.bluez.Device1, etc)
Details
Message ID
<c6efb324-6efb-45df-a0e4-b939a7bc4719@kl.wtf>
In-Reply-To
<87zftlweps.fsf@angela.anarc.at> (view parent)
DKIM signature
pass
Download raw message
On 4/22/24 6:21 PM, Antoine Beaupré wrote:
> Okay let's see then, i have:
>
> Apr 22 12:19:16 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000
> Apr 22 12:19:16 angela poweralertd[3253]: Ignored because -o: 99 | percentage: 100.000000


Ah, don't use the timestamps of the "Ignored" logging from this patch. 
The patch prints on *every* power-related messages received when the 
battery is over the threshold, which is not indicative of events that 
poweralertd send notifications for. poweralertd observes all changes to 
power devices (it's all or nothing unfortunately), but it only sends 
notifications if an important state changed whereas the majority of the 
messages are just an updated readout timestamp or a voltage change.

Without the patch active, `sudo busctl monitor` will include both the 
device change and the poweralertd notification when it happens, and the 
visible notification can be used as cue to check the log. Alternatively, 
if that proves too cumbersome, maybe just collect the output of `upower 
-d` (i.e., the current state dump) after a few of the notifications to 
see the states that the device flipflops between.
Antoine Beaupré <anarcat@orangeseeds.org>
Details
Message ID
<87wmopw6hl.fsf@angela.anarc.at>
In-Reply-To
<c6efb324-6efb-45df-a0e4-b939a7bc4719@kl.wtf> (view parent)
DKIM signature
missing
Download raw message
Understood, I have downgraded back to the package without the patch and
will keep monitoring for changes.
Reply to thread Export thread (mbox)