Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_on_battery_created created too many similar notifications in quick succession #2168

Closed
1 task done
gunchev opened this issue Oct 14, 2023 · 9 comments
Closed
1 task done
Labels

Comments

@gunchev
Copy link

gunchev commented Oct 14, 2023

blueman: blueman-2.3.5-5.fc38.x86_64
BlueZ: bluez-5.70-1.fc38.x86_64
Distribution: On login _on_battery_created generates "created too many similar notifications in quick succession".
Desktop environment: KDE

This issue has been happening for a while now (at least since 2022-07-28). Usually happens after boot and login, when I connect a Bluetooth mouse that reports battery percentage.

There are no "negative effects" except ABRT complaining now and then. I could do some python debugging but I never really worked with Bluetooth or gdbus.

@infirit infirit added the bug label Oct 16, 2023
@infirit
Copy link
Contributor

infirit commented Oct 16, 2023

It's mostly harmless and I have seen it on plasma a couple times. I haven't been able to reliably reproduce it myself. The least we can do is not have abrt pester users.

I suspect it doesn't like that, iirc we replace the initial notification with one that has battery level info, which can be quite fast after the initial notification.

@cschramm
Copy link
Member

cschramm commented Oct 17, 2023

I think this comes from https://invent.kde.org/plasma/plasma-workspace/-/blob/301a3d60514628794252d5f28f08afe5f46aeed6/libnotificationmanager/server_p.cpp#L214-215 but looking at that part, I do not really understand what triggers it as the check seems to target two identical consecutive notifications within 1 second. It ignores the icon but it does check the body and the body should change as the percentage gets added.

https://github.com/blueman-project/blueman/blob/main/blueman/plugins/applet/ConnectionNotifier.py#L39-L40 probably sends two messages and the Plasma daemon just does not like our set_notification_icon. I'd actually consider it a bug that it blocks an isolated icon change.

@gunchev
Copy link
Author

gunchev commented Oct 17, 2023

I haven't been able to reliably reproduce it myself.

I can reproduce it every time - login to KDE, sudo journalctl -xf and turn on one of the two BT mice. Turn off and on the mouse to get it again. No problem with XFCE, LXDE and other DEs.

@gunchev
Copy link
Author

gunchev commented Oct 17, 2023

I added some code to catch the exception and log notification.__dict__ and value to a unique open('...', 'xb') timestamped file in /tmp - only one file is created per mouse connection.

The exception happens on the notification.set_notification_icon("battery") line, commenting that out "solves" the problem. I get the notification pop-up too.

@infirit
Copy link
Contributor

infirit commented Oct 17, 2023

This will properly handle the error and send the traceback to syslog/terminal as error (not trigger abrt).

diff --git a/blueman/plugins/applet/ConnectionNotifier.py b/blueman/plugins/applet/ConnectionNotifier.py
index 35c202f1c..20ec5eded 100644
--- a/blueman/plugins/applet/ConnectionNotifier.py
+++ b/blueman/plugins/applet/ConnectionNotifier.py
@@ -1,3 +1,4 @@
+import logging
 from gettext import gettext as _
 from typing import Any, Dict, Union
 
@@ -55,5 +56,8 @@ class ConnectionNotifier(AppletPlugin):
         if key == "Percentage":
             notification = self._notifications[path]
             if notification:
-                notification.set_message(f"{_('Connected')} {value}%")
-                notification.set_notification_icon("battery")
+                try:
+                    notification.set_message(f"{_('Connected')} {value}%")
+                    notification.set_notification_icon("battery")
+                except GLib.Error:
+                    logging.error("Failed to update notification", exc_info=True)

@infirit
Copy link
Contributor

infirit commented Oct 18, 2023

Here is a workaround #2175 for KDE's behaviour on icon updates. Don't like it but outside of only catching the error and logging it I don't see nicer solution.

@cschramm
Copy link
Member

I submitted a merge request to KDE to hopefully get this fixed: https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/3425

@infirit
Copy link
Contributor

infirit commented Oct 23, 2023

The PR was accepted by the plasma team and we handle the error gracefully with 7951b49.

@cschramm
Copy link
Member

If I get things right, the fix will be part of 5.27.9, scheduled for tomorrow.

To be honest, I do not really understand the intention of what Plasma is doing there at all. The trigger seems to have been a Chrome fingerprint protection extension that triggered 5+ identical notifications when visiting a Microsoft page. I'd actually classify that as "works as intended" or maybe something that should be optimized in the extension, but KDE actually built a spam protection mechanism where a couple of notification properties get checked for changes to the previous notification, not even considering if the current one is a replacement or not. The whole system seems like a bug to me. 😅

Anyway, should be fine for blueman with the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants