Merge lp://staging/~larsu/indicator-messages/add-simple-dbus-api into lp://staging/indicator-messages/13.10

Proposed by Lars Karlitski
Status: Work in progress
Proposed branch: lp://staging/~larsu/indicator-messages/add-simple-dbus-api
Merge into: lp://staging/indicator-messages/13.10
Diff against target: 545 lines (+407/-20)
7 files modified
m4/gcov.m4 (+1/-1)
src/Makefile.am (+3/-1)
src/im-application-list.c (+108/-18)
src/im-application-list.h (+9/-0)
src/im-notifications.c (+241/-0)
src/im-notifications.h (+40/-0)
src/messages-service.c (+5/-0)
To merge this branch: bzr merge lp://staging/~larsu/indicator-messages/add-simple-dbus-api
Reviewer Review Type Date Requested Status
Ted Gould (community) Disapprove
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+224622@code.staging.launchpad.net

Commit message

Add com.canonical.Notifications dbus API

That API is geared towards a more traditional notification center rather than the messaging menu as known on the desktop. It can thus be much simpler.

It primarily benefits services that send messages on behalf of applications.

Description of the change

This is a proposal for a new API for the messaging menu.

libmessaging-menu is hard to use from go (main loop and all) and fairly inefficient for services that want to send messages on behalf of many applications. This new API is dbus-only and trimmed down to only support the notification center style of the messaging menu that we have on the phone. I assume we'll get the same on the desktop at some point.

Going forward, I suggest that unity8 exposes this API itself and uses it for both the messaging menu and notifications. That's why I called it com.canonical.Notifications. There's no documentation yet, because I want to have a discussion about it first. The gist:

- AddNotification(s app_id, s notification_id, a{sv} notification) adds a notification/message. The id must be unique for the application. The dictionary contains a description of the notifications, such as "title", "body", "icon", "actions", etc.

- RemoveNotification(s app_id, s notification_id) removes a previously added notification.

- The signal NotificationActivated(s app_id, s notification_id, s action_name, av parameter) is emitted when a notification or one of its actions was tapped on.

Unfortunately, unity8 currently only supports very few action configurations in the messaging menu. It should probably just use the same components as it uses for notifications.

We did something similar in GNOME a while back. We could join forces and move a new spec to freedesktop to replace the current org.freedesktop.Notifications. This would require our apps to be d-bus activatable and exposing org.freedesktop.Application. Not sure if we already have that or are planning to.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
John Lenton (chipaca) wrote :

Oh, yes, pretty please. This would make things *so* much easier for us in push notifications land.

416. By Lars Karlitski

Remove passive aggressive comment about g_dbus_connection_register_object()

It's incorrect: register_object() does take a ref on the interface info. The
unref is unnecessary because g_dbus_node_info_lookup_interface() doesn't return
a ref.

417. By Lars Karlitski

m4/gcov.m4: add lcov version 1.11

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
John Lenton (chipaca) wrote :

A nice to have that I just realised is missing is a (public) signal for when the notification is dismissed by swiping left or tapping "Clear All". I can see the Dismiss on dbus, fwiw:

method call sender=:1.36 -> dest=:1.70 serial=279 path=/com/canonical/indicator/messages/ubuntu_system_settings_desktop; interface=com.canonical.indicator.messages.application; member=Dismiss
   array [
   ]
   array [
      string "msg-id"
   ]

Revision history for this message
Ted Gould (ted) wrote :

I don't think that we want an interface that is based on passing dictionaries around. I'd rather have something that is more explicit about the structure of the notification, what is required and what the behavior is. I think without that we end up with the same "hints problem" we have with fd.o Notifications today (which is a complete mess).

review: Disapprove
Revision history for this message
Lars Karlitski (larsu) wrote :

> I don't think that we want an interface that is based on passing dictionaries
> around. I'd rather have something that is more explicit about the structure of
> the notification, what is required and what the behavior is.

Dictionary keys are well-defined and expected to be implemented by the server. What do you suggest? A D-Bus method call with everything as arguments?

> I think without that we end up with the same "hints problem" we have with fd.o
> Notifications today (which is a complete mess).

No, it wouldn't. Unity8 is _required_ to implement everything in the dictionary. There's nothing hint-like about these keys.

Revision history for this message
Ted Gould (ted) wrote :

On Wed, 2014-09-17 at 23:02 +0000, Lars Uebernickel wrote:

> > I don't think that we want an interface that is based on passing dictionaries
> > around. I'd rather have something that is more explicit about the structure of
> > the notification, what is required and what the behavior is.
>
> Dictionary keys are well-defined and expected to be implemented by the server. What do you suggest? A D-Bus method call with everything as arguments?

Yes, I think being explicit is better in cases like this. But I'm also
not convinced there's room to have one end point for all notifications.
There's lots of ways to notify, transient, etc. It perhaps would make
more sense to have an endpoint for each.

Unmerged revisions

417. By Lars Karlitski

m4/gcov.m4: add lcov version 1.11

416. By Lars Karlitski

Remove passive aggressive comment about g_dbus_connection_register_object()

It's incorrect: register_object() does take a ref on the interface info. The
unref is unnecessary because g_dbus_node_info_lookup_interface() doesn't return
a ref.

415. By Lars Karlitski

Add com.canonical.Notifications dbus API

That API is geared towards a more traditional notification center rather than
the messaging menu as known on the desktop. It can thus be much simpler.

It primarily benefits services that send messages on behalf of applications.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches