backintime icon indicating copy to clipboard operation
backintime copied to clipboard

Fix: Send notifies directly using python-dbus replacing notify-send

Open Zocker1999NET opened this issue 4 years ago • 11 comments

This allows to configure the APP_NAME, so notification servers / user can better distinguish Back In Time notifications from other script's notifications and removes the dependency on notify-send. Probably it also makes sending notifications a little bit faster.

The change was tested on Debian Unstable with KDE's Plasma 5. Plasma reflected the app name correctly.

Zocker1999NET avatar Apr 23 '21 20:04 Zocker1999NET

Interesting PR, it replaces command line tool call notify-send by its corresponding DBus call.

I have found the API doc here:

https://specifications.freedesktop.org/notification-spec/latest/ar01s09.html

The source code of of libnotify (which is called by notify-send is here (a Gnome project!):

https://gitlab.gnome.org/GNOME/libnotify

I think the code could probably need a small refactoring to be fail safe in case of DBus errors like eg. this code from common/tools.py:

def onBattery():
    if dbus:
        try:
            bus = dbus.SystemBus()
            proxy = bus.get_object('org.freedesktop.UPower',
                                   '/org/freedesktop/UPower')
            return bool(proxy.Get('org.freedesktop.UPower',
                                  'OnBattery',
                                  dbus_interface = 'org.freedesktop.DBus.Properties'))
        except dbus.exceptions.DBusException:
            pass
    return False

Also we need to check if notify-send uses the same DBus function call on all Linux platforms otherwise we would end up with a lot of "if OS = ..." statements and would repeat the work of notify-send...

aryoda avatar Sep 11 '22 14:09 aryoda

OK, the PR code uses the same DBus interface and we could use this PR as basis for the improved notification plugin of BiT (as written above after improving the code and checking the unit tests).

https://github.com/bit-team/backintime/pull/1156/files#diff-9cc4469f72c632f589802f261e273bd96e5f3b85b0c8feb8f8b94e14c5cdf99eR53

self.notify_interface = dbus.Interface(object=dbus.SessionBus().get_object("org.freedesktop.Notifications", "/org/freedesktop/Notifications"), dbus_interface="org.freedesktop.Notifications")
#define NOTIFY_DBUS_NAME           "org.freedesktop.Notifications"
#define NOTIFY_DBUS_CORE_INTERFACE "org.freedesktop.Notifications"
#define NOTIFY_DBUS_CORE_OBJECT    "/org/freedesktop/Notifications"
#define NOTIFY_PORTAL_DBUS_NAME           "org.freedesktop.portal.Desktop"
#define NOTIFY_PORTAL_DBUS_CORE_INTERFACE "org.freedesktop.portal.Notification"
#define NOTIFY_PORTAL_DBUS_CORE_OBJECT    "/org/freedesktop/portal/desktop"

 _proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SESSION,
                                                G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES,
                                                NULL,
                                                NOTIFY_DBUS_NAME,
                                                NOTIFY_DBUS_CORE_OBJECT,
                                                NOTIFY_DBUS_CORE_INTERFACE,
                                                NULL,
                                                error);

https://gitlab.gnome.org/GNOME/libnotify/-/blob/master/libnotify/notify.c#L567 https://gitlab.gnome.org/GNOME/libnotify/-/blob/master/libnotify/internal.h#L27

aryoda avatar Sep 11 '22 15:09 aryoda

Interesting PR, it replaces command line tool call notify-send by its corresponding DBus call.

Yeah, you're right. I forgot to mention that as it seemed obvious to me (because I learned using that interface at that time), sorry for not explaining it further in my PR.

I added the exception handling because that really was a good point. I also decided to move the "resolving the interface" part from init to the msg function because while thinking about error handling, it occurred to me that the notification daemon could be started after backintime or it could crash away (both especially as it could run for a longer time). Both may require a re-"resolving", at least as I understand the library and based on what errors I saw using that elsewhere. This may may make sending a notification run slower, but it should still also be faster than notify-send, as it had to do the same.

About the unit tests: I cannot run them on my system as they throw a bunch of errors like AttributeError: module 'collections' has no attribute 'MutableSet', and I don't understand why the code contains such references. I'm using cpython 3.10.7 on a Debian Testing system, is my python version wrong or is it something else?

Zocker1999NET avatar Sep 25 '22 22:09 Zocker1999NET

I added the exception handling because that really was a good point.

Thanks a lot for your contribution! We are currently stabilizing and fixing BiT before adding new features (or improving them) but your PR is not forgotten :reminder_ribbon:

About the unit tests: I cannot run them on my system as they throw a bunch of errors like AttributeError: module 'collections' has no attribute 'MutableSet', and I don't understand why the code contains such references.

This is a known "bug" causes by a change in Python 3.10 (see #1245). A fix is integrated in the master branch of BiT so if you pull this upstream version into your branch the tests should work too.

I'm using cpython 3.10.7 on a Debian Testing system, is my python version wrong or is it something else?

Yes, 3.10 had a breaking change.

aryoda avatar Sep 25 '22 23:09 aryoda

Thank you from me also for your contribution!

I learned using that interface at that time)

Of course I can search myself. But if you have a very good tutorial or ressource about python and dbus in mind please let me know. "Learning" DBus is on my ToDo list also.

About the unit tests

Please let me know if you need further assistance with the unittesting. You can open a new Issue and mention me (via @buhtzz ) or you can use the bit-dev mailing list.

buhtz avatar Sep 26 '22 07:09 buhtz

My apologize. Something went wrong.

I just tried to update your Zocker1999NET:dbus-notifications to the current state of bit-team/backintime:dev.

I rolled back my last commit with

git reset --hard HEAD^ 
git push -f 

I think it is fine again.

Can you please try to sync your fork yourself? image

buhtz avatar May 08 '23 13:05 buhtz

I figured out how to update/sync the fork with current upstream state. I also modified the TravisCI setup with apt install libdbus-glib* and pip install dbus-python. The tests on AMD64 are green. So It seems to me that we can use DBUS in testing environments. ~But on ppc6le the machine doesn't boot anymore. 😄 I asked the Travis support.~ Because of the quick and good reply of the TravisCI folks the tests on ppc64le machine now are also green.

There are some other code parts distinguishing behaviour if they are run on Travis (via ON_TRAVIS) or not. But I didn't touched them.

~On the other hand we need to check it ReadTheDocs can build with dbus when the PR is merged. It is not clear yet if this works.~ I also tested this with ReadTheDocs. It builds even with DBUS. I see no need to use ON_TRAVIS and ON_RTD anymore.

buhtz avatar Jun 05 '23 12:06 buhtz