valr / cbatticon

A lightweight and fast battery icon that sits in your system tray
GNU General Public License v2.0
170 stars 40 forks source link

Notifications should close when no longer applicable #44

Closed l0b0 closed 6 years ago

l0b0 commented 6 years ago

cbatticon's low battery warnings have to be dismissed manually, at least in Awesome WM. However, the message about the battery charging will disappear after a few seconds. So it's completely possible (for example if I plug in the machine while not looking or while the screensaver is active) to end up with only a battery warning while the battery is charging. This is misleading.

[Other than minor issues like this cbatticon is a great, reliable little utility. Thank you very much!]

dglava commented 6 years ago

I assume that's intentional to make it require acknowledgement.

For the time being, you can change the source file and replace NOTIFY_EXPIRES_NEVER with NOTIFY_EXPIRES_DEFAULT in line 866. If you don't want any permanent notifications, replace every occurrence.

l0b0 commented 6 years ago

Why would it be intentional? For whose benefit do I acknowledge that the battery at some time was low? Not mine, since the information is no longer relevant.

Changing the notification to expire after some time is even worse: then I might never see a warning while the battery is still discharging.

dglava commented 6 years ago

Why would it be intentional? For whose benefit do I acknowledge that the battery at some time was low?

Probably as a way to make you go "okay, I understood the battery level is low" AS IT pops up and to avoid your second complaint of accidentally missing the warning when you come back later to a low-level battery.

I don't really understand what behaviour you want. I assume it's for the low battery notification to get destroyed once a new notification pops up (or charging event happens).

l0b0 commented 6 years ago

To be precise, the message should stay there until either the user acknowledges (clicks) it or the message is no longer relevant (the battery is charging).

valr commented 6 years ago

@l0b0, well, it's actually supposed to already do what you want as the notify_message function takes as input the previous notification entry and updates it. However, I see that this variable 'notification' is not defined as static, meaning it's not persistent across the calls to update_tray_icon_status.

So, what I have to do is: 1 - define the variable as static 2 - verify all calls to notify_message to ensure that one notification will not crush any important previous notification (i.e. no key information is lost). 3 - test the change

Can you tell me which notification server you are using? Is it something specific to AwesomeWM?

l0b0 commented 6 years ago

I'm afraid I have no clue - it must be whatever is installed with Awesome. Do any of these ring a bell?

$ pacman --query --info awesome | awk -F'[:<=>]' '/^(Depends|Optional Deps)/ {print $1 $2}' | tr -s ' '
Depends On cairo dbus gdk-pixbuf2 imlib2 libxdg-basedir lua lua-lgi pango startup-notification xcb-util-cursor xcb-util-keysyms xcb-util-wm xorg-xmessage libxkbcommon-x11 libxkbcommon xcb-util-xrm
Optional Deps rlwrap
dglava commented 6 years ago

Doesn't look like there is one amongst those packages listed. Go through your installed packages with Pacman and look for "Provides: notification-daemon". All packages from the official repositories which provide that functionality should have that (custom packages from the AUR might not).

Alternatively trigger a notification with notify-send test and look at the running processes to see if there's anything with notify in its name.

l0b0 commented 6 years ago

Thanks for the tip! Turns out the "awesome" package itself provides notification-daemon.

valr commented 6 years ago

@l0b0 : I've released a fix for this issue. I've tested it on my setup (xfce with xfce4-notifyd as notification daemon). Let me know if you still find problems or if the fix isn't working as expected with your setup. Cheers, V.