This allows one to subscribe to notifications from notification server.
Details
- Reviewers
broulik davidedmundson - Group Reviewers
Plasma - Commits
- R120:2357e6f2e134: [libnotificationmanager] introduce the WatchedNotificationsModel
tested using very simple QML
import QtQuick 2.0 import org.kde.notificationmanager 1.1 as Notifications ... Notifications.NotificationWatchedModel { id: model }
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- bshah/notification-watcher
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 26682 Build 26700: arc lint + arc unit
libnotificationmanager/dbus/org.freedesktop.Notifications.xml | ||
---|---|---|
55 ↗ | (On Diff #79122) | Please don't put that on the fdo service |
libnotificationmanager/notificationwatcher.cpp | ||
45 ↗ | (On Diff #79122) | Do we not want to check if that succeeded? I think having a valid property on the watcher could be nice |
libnotificationmanager/notificationwatcher.h | ||
41 ↗ | (On Diff #79122) | Don't leak this implementation detail into the public API. Instead, put all of the dbus handling into a private class, like is done with Server vs ServerPrivate |
47 ↗ | (On Diff #79122) | Use d-pointer for exported class |
libnotificationmanager/server_p.cpp | ||
256 | Don't block | |
283 | Don't we want to adhere to an interface? | |
536 | Then you can probably also use the watcher's "watched services" list rather than storing it in a member |
- kill separate watcher class
- introduce AbstractNotificationsModel
- introduce WatchedNotificationsModel
Great! Since phab ate my nice commit message here we go,
I've re-worked previous revision of patch little bit differently,
- Instead of originally planned Watcher class, now there is WatchedNotificationsModel
- Most if not all of original NotificationsModel code is now moved to AbstractNotificationsModel (this move is why the patch looks giant, but there's not that much extra code)
- WatchedNotificationsModel extends AbstractNM, and instead of connecting to notification server instance directly, it talks with notificationmanager dbus interface
- And when methods on this bus are called, it adds data in model.
There's some bugs and todos, but they are small enough:
- Unregister watcher when dbus service disappears
- Fix the notification body being weird xml
- Maybe add the test.qml I used for testing in repository
I particularly need direction/help for issue 2.
As for testing this, I made sure that all functions of notifications are functional in the notification applet in plasmoidviewer and on watcher side checked if actions are correctly invoked, notifications are added removed properly.
libnotificationmanager/CMakeLists.txt | ||
---|---|---|
25 | Remove | |
libnotificationmanager/abstractnotificationsmodel.h | ||
41 | protected constructor? | |
libnotificationmanager/notification.h | ||
133 | Maybe add a new rawHints() const? Not a fan of ref return | |
libnotificationmanager/notification_p.h | ||
102 | No need to explicitly initialize | |
libnotificationmanager/notificationsmodel.h | ||
4 | wrong order? | |
libnotificationmanager/server_p.cpp | ||
237 | const QString &service : qAsConst(m_notificationWatchers) | |
238 | Remove, or use categorized logging | |
239 | Can you use generated XML interface for this, maybe? | |
278 | Same as above | |
536 | Yes, please do. | |
libnotificationmanager/server_p.h | ||
25 | Include QStringList | |
libnotificationmanager/watchednotificationsmodel.cpp | ||
60 | I think we should have a valid property or any means for the user to check whether this succeeds / the model is working |
address various comments
- adhere to interface name when registering watcher
- remove unused code
- use qdbusservicewatcher
- add valid property
libnotificationmanager/notification.h | ||
---|---|---|
133 | I avoided using "raw" word in the function name, because when we process hints we don't actually modify it. We just process rest of notifications data, so it is still same thing. I will drop ref return however. | |
libnotificationmanager/server_p.cpp | ||
239 | It would introduce weird dependency loop so I'd rather not. |