Use system notifications

[David Prieto]

It would be really useful to have linuxdcpp trigger a system notification when each of the requested downloads has finished, like other p2p programs (e.g. transmission) do.

[Razzloss (razzloss)]

Could be handy. Is DBUS already a direct requirement? (no?)

--RZ

[David Prieto (frandavid100-gmail)]

I don't really know that, hope someone can answer.

[Razzloss (razzloss)]

Yeah, that was mostly to Steven. DBUS isn't directly linked to, but I was wondering if the minimum GTK version (or another library) already requires it, but apparently not. So this would add new dependency.

--RZ

[Steven Sheehy (steven-sheehy)]

It would require libnotify which would in turn return require libdbus. I don't really want to add another library dependency for such a minor feature. This would be suitable for a plugin though (once a plugin system is implemented).

[Jakh Daven (tuxcanfly)]

Hi, I added notification support with libnotify to stable version 1.0.3. This is my first contribution to LinuxDC++, so I was eager to branch it out and missed out on the dependency concerns.
Anyway its available at http://bazaar.launchpad.net/~tuxcanfly/linuxdcpp/linuxdcpp-libnotify
Hope to contribute more. Thanks!

[Razzloss (razzloss)]

OK, I don't have too strong objections for adding a libnotify as a dependency. As our plugin interface is still a far away I'm willing to accept something like this to the main program. (+ there were some rumours that dc++ might get a plugin interface also, and it would be better if ours were at least somewhat compatible.)

But there's some problems why the current code can't be accepted:
1) You should make your changes to the current bzr trunk, not the latest released version. Also the changes should be so, that it is possible to see what has changed. Either submitting patches or clone the main trunk, make the changes to it and publish the changed branch in launchpad.

2) No GUI/GTK code should be but to client/ (released version) or dcpp/ (current trunk) directories. The dcpp/ directory should come from the upstream (DC++) with as little changes as possible. And any changes made to dcpp/ should also be submitted and accepted in the DC++ also. So the changes you made should be moved to linux/

3) The notify settings should be configurable. Simple on/off for different type of notifications should be enough. And at least notification for private message should be added instead of just the download finished.

But if these points are corrected and Steven agrees, I think the notification support can be added to the main program. (Maybe converted to a plugin later, when/if the plugin system is implemented)

But feel free to grab the latest source and start hacking, and fixing bugs ;D. There's some developer info on the old berlios page http://openfacts.berlios.de/index-en.phtml?title=Ldcpp_Developer_information , It's a bit old (refers to cvs, etc.) but most of it should still be valid.

--RZ

[Steven Sheehy (steven-sheehy)]

I concur with my esteemed colleague, Dr. Razzloss.

[Jakh Daven (tuxcanfly)]

Thanks Razzloss and Steven, I will correct the problems and submit a patch
ASAP. It would be great to see this implemented in next version.

On Fri, Oct 2, 2009 at 5:03 AM, Steven Sheehy <email address hidden>wrote:

> I concur with my esteemed colleague, Dr. Razzloss.
>
> --
> Use system notifications
> https://bugs.launchpad.net/bugs/406302
> You received this bug notification because you are subscribed to
> LinuxDC++.
>
> Status in Linux DC++: Confirmed
>
> Bug description:
> It would be really useful to have linuxdcpp trigger a system notification
> when each of the requested downloads has finished, like other p2p programs
> (e.g. transmission) do.
>

[Jakh Daven (tuxcanfly)]

Hi all,
I'm pleased to announce that I have finally got it working! :phew:

It was great learning experience and greater fun for me. I can see why open source is so popular. :)

So here's whats working:

* Download notifications
* Private Message notifications (only when off focus)
* Both notifications are user configurable, can be enabled/disabled instantly

I would be only too happy to correct any mistakes/bugs/typos. Looking forward to squash more bugs and add more features :D

Thanks to Razzloss and Steven for pointing me in the right direction

[Razzloss (razzloss)]

OK,

Few comments:
1) Do we really need a new class with just one static method? IMO notification showing could as well be put to MainWindow as a member function.
2) In Notification::show the icon path must not be hardcoded. Icon path should be derived from WulforManager::getPath() or if possible from the current icon theme. (I'm not too clear on how IconThemes work so it might not be possible)

--RZ

[Steven Sheehy (steven-sheehy)]

- There is a memory leak. You need to free NotifyNotification after use (g_object_free).
- Don't call gtk_init multiple times, it has already been invoked in wulfor.cc.
- As Razzloss said, use icon theme. You should be able to pass just "linuxdcpp" for the icon.
- Use notify_notification_new_with_status_icon if status icon is present (or attach with notify_notification_attach_to_status_icon)
- I'm not sure whether libnotify should be invoked outside of the GDK locking mechanism. We may need to dispatch the call in MainWindow::on(QueueManagerListener::Finished to the GUI.
- Notification options should be in appearance tab in preferences.

[Steven Sheehy (steven-sheehy)]

Sorry, I meant g_object_ unref.
>

[Jakh Daven (tuxcanfly)]

Hi,

Fixed most of the above issues. :)

** Notification icon is now fetched from the theme
** function call is dispatched to GUI
** Added statusIcon to notification
** Plugged most of the memleaks
** Lots of code clean up

It works a lot better now. Thanks Steven and Razzloss!

[lys (lys)]

to apply this patch is this required ?

* x11-libs/libnotify
     Available versions: 0.4.5
     Homepage: http://www.galago-project.org/
     Description: Notifications library
?

[lys (lys)]

only if thats the case its pretty much gnome dependent am using kde here on gentoo linux and for that little feature it wants all this installing :

UniMatrix0 ~ # emerge -p libnotify

These are the packages that would be merged, in order:

Calculating dependencies ... done!
[ebuild N ] app-text/iso-codes-3.10 5,226 kB
[ebuild N ] dev-libs/libIDL-0.8.13 USE="-debug" 378 kB
[ebuild N ] x11-libs/libsexy-0.1.11-r1 USE="-debug -doc" 262 kB
[ebuild N ] gnome-base/orbit-2.14.17 USE="-doc" 730 kB
[ebuild N ] gnome-base/gconf-2.26.2-r1 USE="-debug -doc -ldap -policykit" 1,441 kB
[ebuild N ] x11-libs/libnotify-0.4.5 356 kB
[ebuild N ] x11-misc/notification-daemon-0.4.0-r1 USE="-debug -gstreamer" 395 kB

Total: 7 packages (7 new), Size of downloads: 8,785 kB

n thats before they compiled so be alot more than 8mb after compiling them bits, i gotta say i really wouldnt want all those dependencies tbh

[lys (lys)]

shame as it looks nice :-/

[Jakh Daven (tuxcanfly)]

Thats sad. Highlights the need for a desktop manager independent notification library. KDE has KNotify but it would be a bit of an overkill to support it since linuxdc++ is so dependent on gtk+. It would be great if we had something like xdg-open (xdg-notify?) which is desktop manager agnostic.

[Steven Sheehy (steven-sheehy)]

Do you think we should put an #IFDEF HAVE_LIBNOTIFY (which can be set automatically in SConstruct) around the calls to libnotify until we get a plugin system going? RZ, what are your thoughts?

Sorry for the trouble, but I found some additional issues:

- showNotification should use std::string for title. const char* is not guaranteed to be around after being queued.
- use notify_notification_new_with_status_icon(title.c_str(), body.c_str(), "linuxdcpp", statusIcon); no need to use gtk_icon_theme_load_icon that way.
- Don't need to queue showNotification from inside other *_gui() functions. So you can just call showNotification() from addPrivateMessage_gui (but keep the on(finished) the same)
- Move the call to #include <libnotify/notify.h> to mainwindow.cc to reduce the scope of the include

[Razzloss (razzloss)]

Steven: My thoughts exactly when I read lys' opinion. I think #IFDEFfing the contents of showNotify, #include and the settingsmanager options should be enough? Defaults still need to be there, and maybe defaulting to 0 (or default based on #IFDEF)? I think this would be neater than #IFDEFing all the calls to showNotification and it's contents (sure adds few unneeded function calls if defaults are 1, but won't affect performance).

With #IFDEFs Gentoo ebuilds can set the use flags correctly which will pull libnotify and its dependencies when needed.

--RZ

[Jakh Daven (tuxcanfly)]

Hi,

@Steven Fixed the latest issues ;) But

* notify_notification_new_with_status_icon(title.c_str(), body.c_str(), "linuxdcpp", statusIcon)

isnt working. Notification turns up without any icon.

So I have kept the previous gtk_theme_load_icon as it is.

@Razzloss Yeah #IFDEF'ing would prevent forcing libnotify as a dependency :D

[Jakh Daven (tuxcanfly)]

Edit: Formatting

[Jakh Daven (tuxcanfly)]

*Fixed (removed) notifications for match_queue filelists and others. Notifications now only for FLAG_NORMAL queue items.

[Steven Sheehy (steven-sheehy)]

Committed to trunk. I added the #ifdef HAVE_LIBNOTIFY so that it's only enabled if libnotify is present at the time of compilation. I also removed the pixbuf in favor of using icon name. The reason it wasn't working is because we're not storing the application icon in the icon theme search path visible to other programs (our custom path is only visible to linuxdcpp, not notification-daemon). I will be fixing it in an upcoming commit to install the application icon to /usr/share/icons and keep all other icons at /usr/share/linuxdcpp/icons/.

Thanks for the patch Jakh Daven! If you're looking for possible things to work on next, best place would be to take a look at the open bugs on launchpad. We generally try to put more of a focus on bugs before features, but you're of course free to work on whatever you like. Hope we'll see more from you. :)

[lys (lys)]

i thought this was fixed to not depend on libnotify if not wanted :

loz@UniMatrix0 ~/linuxdcpp $ scons --help
scons: Reading SConscript files ...

scons: warning: The Options class is deprecated; use the Variables class instead.
File "/home/loz/linuxdcpp/SConstruct", line 54, in <module>

scons: warning: The BoolOption() function is deprecated; use the BoolVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 57, in <module>

scons: warning: The PathOption() function is deprecated; use the PathVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 60, in <module>
Checking for g++ >= 4.1...(cached) yes
Checking for pkg-config... yes
Checking for gtk+-2.0 >= 2.10... yes
Checking for gthread-2.0 >= 2.4... yes
Checking for libglade-2.0 >= 2.4... yes
Checking for libnotify >= 0.4.1... no
        libnotify >= 0.4.1 not found, disabling notifications.
        Note: You might have the lib but not the headers
Checking for C++ header file boost/version.hpp... yes
Checking for C header file time.h... yes
Checking for C header file signal.h... yes
Checking for C header file unistd.h... yes
Checking for C library pthread... yes
Checking for C library z... yes
Checking for C library bz2... yes
Checking for C library crypto... yes
Checking for C library ssl... yes
Checking for C header file iconv.h... yes
Checking for iconv(0, (const char **)0, 0, (char**)0, 0) in C library iconv... no
Checking for C header file net/if.h... yes
Package libnotify was not found in the pkg-config search path.
Perhaps you should add the directory containing `libnotify.pc'
to the PKG_CONFIG_PATH environment variable
No package 'libnotify' found
OSError: 'pkg-config --libs libnotify' exited 1:
  File "/home/loz/linuxdcpp/SConstruct", line 232:
    env.ParseConfig('pkg-config --libs libnotify')
  File "/usr/lib/scons-1.2.0/SCons/Environment.py", line 1447:
    return function(self, self.backtick(command))
  File "/usr/lib/scons-1.2.0/SCons/Environment.py", line 585:
    raise OSError("'%s' exited %d" % (command, status))

[lys (lys)]

oh and sorry it's : Tree is up to date at revision 333.

[Jakh Daven (tuxcanfly)]

Well I checked the code and I guess you're right. We are asking for
libnotify includes unconditionally. I don't have the latest trunk so could
you please make the changes given below and re-build and let me know the
output?

Please change line 232, Sconstruct
        env.ParseConfig('pkg-config --libs libnotify')

to look like

if env.has_key('HAVE_LIBNOTIFY') and env['HAVE_LIBNOTIFY']:
        env.ParseConfig('pkg-config --libs libnotify')

Please reply with the output.

[Jakh Daven (tuxcanfly)]

Here's the patch if you need it.

[lys (lys)]

loz@UniMatrix0 ~/linuxdcpp $ rm -r build/ ; rm linuxdcpp ; scons PREFIX=/usr
rm: cannot remove `build/': No such file or directory
rm: cannot remove `linuxdcpp': No such file or directory
scons: Reading SConscript files ...

scons: warning: The Options class is deprecated; use the Variables class instead.
File "/home/loz/linuxdcpp/SConstruct", line 54, in <module>

scons: warning: The BoolOption() function is deprecated; use the BoolVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 57, in <module>

scons: warning: The PathOption() function is deprecated; use the PathVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 60, in <module>
Checking for g++ >= 4.1...(cached) yes
Checking for pkg-config... yes
Checking for gtk+-2.0 >= 2.10... yes
Checking for gthread-2.0 >= 2.4... yes
Checking for libglade-2.0 >= 2.4... yes
Checking for libnotify >= 0.4.1... no
        libnotify >= 0.4.1 not found, disabling notifications.
        Note: You might have the lib but not the headers
Checking for C++ header file boost/version.hpp... yes
Checking for C header file time.h... yes
Checking for C header file signal.h... yes
Checking for C header file unistd.h... yes
Checking for C library pthread... yes
Checking for C library z... yes
Checking for C library bz2... yes
Checking for C library crypto... yes
Checking for C library ssl... yes
Checking for C header file iconv.h... yes
Checking for iconv(0, (const char **)0, 0, (char**)0, 0) in C library iconv... no
Checking for C header file net/if.h... yes
scons: done reading SConscript files.
scons: Building targets ...
scons: `.' is up to date.
scons: done building targets.

[lys (lys)]

sorry nm it didnt paste right into the SConstruct file

:

loz@UniMatrix0 ~/linuxdcpp $ patch -p0 < Sconstruct.patch
patching file SConstruct
loz@UniMatrix0 ~/linuxdcpp $ rm -r build/ ; rm linuxdcpp ; time scons PREFIX=/usr
rm: cannot remove `build/': No such file or directory
rm: cannot remove `linuxdcpp': No such file or directory
scons: Reading SConscript files ...

scons: warning: The Options class is deprecated; use the Variables class instead.
File "/home/loz/linuxdcpp/SConstruct", line 54, in <module>

scons: warning: The BoolOption() function is deprecated; use the BoolVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 57, in <module>

scons: warning: The PathOption() function is deprecated; use the PathVariable() function instead.
File "/home/loz/linuxdcpp/SConstruct", line 60, in <module>
Checking for g++ >= 4.1...(cached) yes
Checking for pkg-config... yes
Checking for gtk+-2.0 >= 2.10... yes
Checking for gthread-2.0 >= 2.4... yes
Checking for libglade-2.0 >= 2.4... yes
Checking for libnotify >= 0.4.1... no
        libnotify >= 0.4.1 not found, disabling notifications.
        Note: You might have the lib but not the headers
Checking for C++ header file boost/version.hpp... yes
Checking for C header file time.h... yes
Checking for C header file signal.h... yes
Checking for C header file unistd.h... yes
Checking for C library pthread... yes
Checking for C library z... yes
Checking for C library bz2... yes
Checking for C library crypto... yes
Checking for C library ssl... yes
Checking for C header file iconv.h... yes
Checking for iconv(0, (const char **)0, 0, (char**)0, 0) in C library iconv... no
Checking for C header file net/if.h... yes
scons: done reading SConscript files.
scons: Building targets ...

[lys (lys)]

shame there's no <code> blah </code> tags in this bug tracker that did it that time, thanks :)

[Jakh Daven (tuxcanfly)]

Seems it builds fine with the SConstruct fix. Do you think it works OK now?

On Wed, Oct 14, 2009 at 8:41 PM, lys <email address hidden> wrote:

> sorry nm it didnt paste right into the SConstruct file
>
> :
>
> loz@UniMatrix0 ~/linuxdcpp $ patch -p0 < Sconstruct.patch
> patching file SConstruct
> loz@UniMatrix0 ~/linuxdcpp $ rm -r build/ ; rm linuxdcpp ; time scons
> PREFIX=/usr
> rm: cannot remove `build/': No such file or directory
> rm: cannot remove `linuxdcpp': No such file or directory
> scons: Reading SConscript files ...
>
> scons: warning: The Options class is deprecated; use the Variables class
> instead.
> File "/home/loz/linuxdcpp/SConstruct", line 54, in <module>
>
> scons: warning: The BoolOption() function is deprecated; use the
> BoolVariable() function instead.
> File "/home/loz/linuxdcpp/SConstruct", line 57, in <module>
>
> scons: warning: The PathOption() function is deprecated; use the
> PathVariable() function instead.
> File "/home/loz/linuxdcpp/SConstruct", line 60, in <module>
> Checking for g++ >= 4.1...(cached) yes
> Checking for pkg-config... yes
> Checking for gtk+-2.0 >= 2.10... yes
> Checking for gthread-2.0 >= 2.4... yes
> Checking for libglade-2.0 >= 2.4... yes
> Checking for libnotify >= 0.4.1... no
> libnotify >= 0.4.1 not found, disabling notifications.
> Note: You might have the lib but not the headers
> Checking for C++ header file boost/version.hpp... yes
> Checking for C header file time.h... yes
> Checking for C header file signal.h... yes
> Checking for C header file unistd.h... yes
> Checking for C library pthread... yes
> Checking for C library z... yes
> Checking for C library bz2... yes
> Checking for C library crypto... yes
> Checking for C library ssl... yes
> Checking for C header file iconv.h... yes
> Checking for iconv(0, (const char **)0, 0, (char**)0, 0) in C library
> iconv... no
> Checking for C header file net/if.h... yes
> scons: done reading SConscript files.
> scons: Building targets ...
>
> --
> Use system notifications
> https://bugs.launchpad.net/bugs/406302
> You received this bug notification because you are subscribed to
> LinuxDC++.
>
> Status in Linux DC++: Fix Committed
>
> Bug description:
> It would be really useful to have linuxdcpp trigger a system notification
> when each of the requested downloads has finished, like other p2p programs
> (e.g. transmission) do.
>

[Jakh Daven (tuxcanfly)]

Thanks for testing it out lys! Good to know it works for you now :)
On Wed, Oct 14, 2009 at 8:47 PM, lys <email address hidden> wrote:

> shame there's no <code> blah </code> tags in this bug tracker that did
> it that time, thanks :)
>
> --
> Use system notifications
> https://bugs.launchpad.net/bugs/406302
> You received this bug notification because you are subscribed to
> LinuxDC++.
>
> Status in Linux DC++: Fix Committed
>
> Bug description:
> It would be really useful to have linuxdcpp trigger a system notification
> when each of the requested downloads has finished, like other p2p programs
> (e.g. transmission) do.
>

[Jakh Daven (tuxcanfly)]

Yeah, <code> tags would help. Glad it works for you! Thanks for testing it out, lys :)

@Steven Please review the patch

[Steven Sheehy (steven-sheehy)]

Thanks. Should be fixed in trunk now.

[Steven Sheehy (steven-sheehy)]

With the latest commit, linuxdcpp icon should now show up in notification. Please test. Note that you have to install the application for the icon to get moved to the icon theme search path. If you are running from your build directory, you can copy linuxdcpp.(svg|png) to ~/.icons/ as a workaround to get it to show up.

[lys (lys)]

cant enable the boxes for notification stuff on Tree is up to date at revision 337. crashes with this msg:

(linuxdcpp:3277): Gtk-CRITICAL **: gtk_box_reorder_child: assertion `GTK_IS_WIDGET (child)' failed
Segmentation fault

[Steven Sheehy (steven-sheehy)]

lys, I thought you were not going to install libnotify because of too many dependencies? If you don't have libnotify the notification boxes shouldn't even show up. Do you mean to say that you did end up installing libnotify to try it and when you try to enable it the program crashes? Can you provide a stacktrace in debug mode?

[lys (lys)]

didnt want it one one pc cause its a minimal desktop i rarely use and its always nice to be given the choice, it actually comes in handy on my main workstation, and by the way i dont know why it was doing that, i deleted the bzr src n grabbed it again and compiled it and it worked, dont know why it wasnt the day i reported this, sorry all's fine now