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

Feature/dbus signals #2

Merged
merged 6 commits into from
Oct 25, 2015
Merged

Feature/dbus signals #2

merged 6 commits into from
Oct 25, 2015

Conversation

esiqveland
Copy link
Owner

This seems to work for me.
I have to add a buffered chan with close to length 5, because otherwise something hangs. This must be resolved before a merge.

I think it is related to this:

2015/10/24 14:20:12 got signal: 1 Signal: &{Sender::1.16 Path:/org/freedesktop/Notifications Name:org.freedesktop.Notifications.NotificationClosed Body:[133 2]}
2015/10/24 14:20:12 NotificationClosed: 133 Reason: 2
2015/10/24 14:20:12 got signal: 2 Signal: &{Sender::1.16 Path:/org/freedesktop/Notifications Name:org.freedesktop.Notifications.NotificationClosed Body:[133 2]}
2015/10/24 14:20:12 got signal: 3 Signal: &{Sender::1.16 Path:/org/freedesktop/Notifications Name:org.freedesktop.Notifications.NotificationClosed Body:[133 2]}
2015/10/24 14:20:12 got signal: 4 Signal: &{Sender::1.16 Path:/org/freedesktop/Notifications Name:org.freedesktop.Notifications.NotificationClosed Body:[133 2]}

Every signal gets delivered at least 3-4 times in a row. I'm not sure of why this happens.

  • Either dbus has registered lots of AddMatch rules for me and delivers many times because of that.
  • Or dbus-go is sending me events lots of times and I don't know why.

@esiqveland esiqveland mentioned this pull request Oct 24, 2015
@emersion
Copy link
Contributor

Every signal gets delivered at least 3-4 times in a row.

I had the same problem too. Strangely, when closing the notification from the notification pane (in GNOME Shell) triggers only one signal, and closing it from the banner triggers 3 signals. Maybe a GNOME Shell issue.

I have to add a buffered chan with close to length 5, because otherwise something hangs. This must be resolved before a merge.

Concerning the buffered channels length, it seems to be a known issue. See https://github.com/godbus/dbus/blob/master/_examples/signal.go#L19 and https://godoc.org/github.com/godbus/dbus#Conn.Signal

@esiqveland
Copy link
Owner Author

Hmm, don't think it says anything about duplicate signals being delivered, just that signals will be dropped if the channel is not ready.

So I'm thinking just adding a NewWithBufferSize(conn, bufferSize)
and leave the default at 10 as in the example above.
That leaves me still with the duplicate signals though.

  • Either dbus has registered lots of AddMatch rules for me and delivers many times because of that.
  • Or dbus-go is sending me events lots of times and I don't know why.
  • Or a bug in Gnome shell sending the signal multiple times? I am also using gnome-shell 3.18 on arch.

@emersion
Copy link
Contributor

Note that only one signal is received for ActionInvoked. So it may definitely be a GNOME issue.

(EDIT: added quotes to previous message for clarity)

@esiqveland
Copy link
Owner Author

Ok.
Maybe I'll try in KDE or something later today to see if the issue is the same with multiple notifications there.

Edit: Actions are working correctly for me too in gnome shell 3.18 at least.

@esiqveland
Copy link
Owner Author

Ok, I found something. It does indeed look like a gnome-shell bug:
I found a program called dbus-monitor:

$ dbus-monitor "interface='org.freedesktop.Notifications'"                                                                                              

When sending a notification and clicking it in gnome-shell, I see this in the monitor log:

signal time=1445765929.831279 sender=org.freedesktop.DBus -> destination=:1.190 serial=2 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameAcquired
   string ":1.190"
signal time=1445765929.831304 sender=org.freedesktop.DBus -> destination=:1.190 serial=4 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameLost
   string ":1.190"


method call time=1445765938.508469 sender=:1.191 -> destination=org.freedesktop.Notifications serial=3 path=/org/freedesktop/Notifications; interface=org.freedesktop.Notifications; member=Notify
   string "Test GO App"
   uint32 0
   string "mail-unread"
   string "Test"
   string "This is a test of the DBus bindings for go."
   array [
      string "cancel"
      string "Cancel"
      string "open"
      string "Open"
   ]
   array [
   ]
   int32 5000
signal time=1445765939.564258 sender=:1.21 -> destination=(null destination) serial=1877 path=/org/freedesktop/Notifications; interface=org.freedesktop.Notifications; member=NotificationClosed
   uint32 51
   uint32 2
signal time=1445765939.564456 sender=:1.21 -> destination=(null destination) serial=1879 path=/org/freedesktop/Notifications; interface=org.freedesktop.Notifications; member=NotificationClosed
   uint32 51
   uint32 2
signal time=1445765939.596462 sender=:1.21 -> destination=(null destination) serial=1880 path=/org/freedesktop/Notifications; interface=org.freedesktop.Notifications; member=NotificationClosed
   uint32 51
   uint32 2

You clearly see the NotificationClosed signal being sent 3 times by the same sender with a few nanosecond delay.

Edit: I have reported this to gnome-shell bugzilla: https://bugzilla.gnome.org/show_bug.cgi?id=757084

@esiqveland
Copy link
Owner Author

Also, I am not sure if signal handling should be mandatory for using this library.
Maybe have a way to turn it off?
As it is now, you will have to consume the channels for events, or they will fill up.

@emersion
Copy link
Contributor

Two solutions:

  • Write to channels in non-blocking mode
  • Do not enable signal handling by default. Add a private method enableSignals which is called with NotificationClosed and ActionInvoked

(The last one seems the best one)

@esiqveland
Copy link
Owner Author

I think it will be ok to keep it as is.

The users that only want to send notifications, can use the simple 'static' call:
notify.SendNotification(conn, n)
directly and not care/think about the output.

esiqveland added a commit that referenced this pull request Oct 25, 2015
@esiqveland esiqveland merged commit 38c5b2a into master Oct 25, 2015
@esiqveland esiqveland deleted the feature/dbus_signals branch October 25, 2015 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants