Merge lp:~cjcurran/indicator-sound/notifications-moved-service-side into lp:indicator-sound/fourth

Proposed by Conor Curran
Status: Rejected
Rejected by: Ted Gould
Proposed branch: lp:~cjcurran/indicator-sound/notifications-moved-service-side
Merge into: lp:indicator-sound/fourth
Diff against target: 261 lines (+30/-86)
7 files modified
configure.ac (+2/-4)
src/device.c (+14/-2)
src/device.h (+1/-0)
src/indicator-sound.c (+1/-3)
src/slider-menu-item.c (+12/-2)
src/sound-state-manager.c (+0/-73)
src/sound-state-manager.h (+0/-2)
To merge this branch: bzr merge lp:~cjcurran/indicator-sound/notifications-moved-service-side
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
Review via email: mp+67141@code.launchpad.net

Description of the change

This moves the notification code service side, so as on a scroll the service triggers the notification and not the indicator.

Two benefits here:
1. The notifications are only triggered on a volume update confirmation from the pulse server. Before it was triggering the scroll on the GTK scroll event irregardless of whether the volume was updated or not. This breaks the MVC GUI pattern therefore no good.
2. Libnotify dependency is now on the service and not on the indicator in keeping with Ted's initial concept for how the indicator should be light.

To post a comment you must log in.
Revision history for this message
Conor Curran (cjcurran) wrote :

I need to do some other work around this to actually send the notification.
Its in some other branch, I'll find it. Either way this should go in.

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

It'd be nice to land both branches in the same release so that there isn't a feature regression. No issue in this change itself though.

review: Approve
Revision history for this message
Conor Curran (cjcurran) wrote :

Ill push this back till after we get the Oneiric release sorted.

Unmerged revisions

249. By Conor Curran

no whitespace in the handle event name please

248. By Conor Curran

stripped the notification stuff from the indicator and put in place the flag to determine when to send notifications from the device object

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'configure.ac'
--- configure.ac 2011-06-28 13:50:44 +0000
+++ configure.ac 2011-07-07 09:28:32 +0000
@@ -52,15 +52,13 @@
52 [PKG_CHECK_MODULES(APPLET, gtk+-3.0 >= $GTK3_REQUIRED_VERSION52 [PKG_CHECK_MODULES(APPLET, gtk+-3.0 >= $GTK3_REQUIRED_VERSION
53 indicator3 >= $INDICATOR_REQUIRED_VERSION53 indicator3 >= $INDICATOR_REQUIRED_VERSION
54 dbusmenu-gtk3-0.4 >= $DBUSMENUGTK_REQUIRED_VERSION54 dbusmenu-gtk3-0.4 >= $DBUSMENUGTK_REQUIRED_VERSION
55 libido3-0.1 >= $INDICATOR_DISPLAY_OBJECTS55 libido3-0.1 >= $INDICATOR_DISPLAY_OBJECTS)
56 libnotify >= $LIBNOTIFY_REQUIRED_VERSION)
57 ],56 ],
58 [test "x$with_gtk" = x2],57 [test "x$with_gtk" = x2],
59 [PKG_CHECK_MODULES(APPLET, gtk+-2.0 >= $GTK_REQUIRED_VERSION58 [PKG_CHECK_MODULES(APPLET, gtk+-2.0 >= $GTK_REQUIRED_VERSION
60 indicator >= $INDICATOR_REQUIRED_VERSION59 indicator >= $INDICATOR_REQUIRED_VERSION
61 dbusmenu-gtk-0.4 >= $DBUSMENUGTK_REQUIRED_VERSION60 dbusmenu-gtk-0.4 >= $DBUSMENUGTK_REQUIRED_VERSION
62 libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS61 libido-0.1 >= $INDICATOR_DISPLAY_OBJECTS)
63 libnotify >= $LIBNOTIFY_REQUIRED_VERSION)
64 ],62 ],
65 [AC_MSG_FAILURE([Value for --with-gtk was neither 2 nor 3])]63 [AC_MSG_FAILURE([Value for --with-gtk was neither 2 nor 3])]
66)64)
6765
=== modified file 'src/device.c'
--- src/device.c 2011-05-18 09:45:49 +0000
+++ src/device.c 2011-07-07 09:28:32 +0000
@@ -34,6 +34,7 @@
34 VoipInputMenuItem* voip_input_menu_item;34 VoipInputMenuItem* voip_input_menu_item;
35 SoundState current_sound_state;35 SoundState current_sound_state;
36 SoundServiceDbus* service;36 SoundServiceDbus* service;
37 gboolean scroll_update;
37};38};
3839
39#define DEVICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), DEVICE_TYPE, DevicePrivate))40#define DEVICE_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), DEVICE_TYPE, DevicePrivate))
@@ -69,7 +70,8 @@
69 priv->voip_input_menu_item = NULL;70 priv->voip_input_menu_item = NULL;
70 priv->current_sound_state = UNAVAILABLE;71 priv->current_sound_state = UNAVAILABLE;
71 priv->service = NULL;72 priv->service = NULL;
7273 priv->scroll_update = FALSE;
74
73 // Init our menu items.75 // Init our menu items.
74 priv->mute_menuitem = g_object_new (MUTE_MENU_ITEM_TYPE, NULL);76 priv->mute_menuitem = g_object_new (MUTE_MENU_ITEM_TYPE, NULL);
75 priv->voip_input_menu_item = g_object_new (VOIP_INPUT_MENU_ITEM_TYPE, NULL);;77 priv->voip_input_menu_item = g_object_new (VOIP_INPUT_MENU_ITEM_TYPE, NULL);;
@@ -107,12 +109,22 @@
107}109}
108110
109void111void
112device_set_scroll_update (Device* self, gboolean update)
113{
114 DevicePrivate* priv = DEVICE_GET_PRIVATE(self);
115 priv->scroll_update = update;
116}
117
118void
110device_sink_update (Device* self,119device_sink_update (Device* self,
111 const pa_sink_info* update)120 const pa_sink_info* update)
112{121{
113 DevicePrivate* priv = DEVICE_GET_PRIVATE (self);122 DevicePrivate* priv = DEVICE_GET_PRIVATE (self);
114 slider_menu_item_update (priv->volume_slider_menuitem, update);123 slider_menu_item_update (priv->volume_slider_menuitem, update);
115124 if (priv->scroll_update == TRUE ){
125 g_debug ("Send out a notification !!");
126 priv->scroll_update = FALSE;
127 }
116 SoundState state = device_get_state_from_volume (self);128 SoundState state = device_get_state_from_volume (self);
117 if (priv->current_sound_state != state){129 if (priv->current_sound_state != state){
118 priv->current_sound_state = state;130 priv->current_sound_state = state;
119131
=== modified file 'src/device.h'
--- src/device.h 2011-03-24 15:22:30 +0000
+++ src/device.h 2011-07-07 09:28:32 +0000
@@ -65,6 +65,7 @@
65void device_sink_deactivated (Device* self);65void device_sink_deactivated (Device* self);
66void device_update_mute (Device* self, gboolean mute_update);66void device_update_mute (Device* self, gboolean mute_update);
67void device_ensure_sink_is_unmuted (Device* self);67void device_ensure_sink_is_unmuted (Device* self);
68void device_set_scroll_update (Device* self, gboolean scroll_update);
6869
69// source and sinkinput/client related for VOIP functionality70// source and sinkinput/client related for VOIP functionality
70void device_update_voip_input_source (Device* sink, const pa_source_info* update);71void device_update_voip_input_source (Device* sink, const pa_source_info* update);
7172
=== modified file 'src/indicator-sound.c'
--- src/indicator-sound.c 2011-06-22 19:19:49 +0000
+++ src/indicator-sound.c 2011-07-07 09:28:32 +0000
@@ -672,9 +672,7 @@
672 value -= gtk_adjustment_get_step_increment (adj);672 value -= gtk_adjustment_get_step_increment (adj);
673 }673 }
674 //g_debug("indicator-sound-scroll - update slider with value %f", value);674 //g_debug("indicator-sound-scroll - update slider with value %f", value);
675 volume_widget_update(VOLUME_WIDGET(priv->volume_widget), value, "scroll updates");675 volume_widget_update(VOLUME_WIDGET(priv->volume_widget), value, "scroll-updates");
676
677 sound_state_manager_show_notification (priv->state_manager, value);
678}676}
679677
680void678void
681679
=== modified file 'src/slider-menu-item.c'
--- src/slider-menu-item.c 2011-03-22 19:53:19 +0000
+++ src/slider-menu-item.c 2011-07-07 09:28:32 +0000
@@ -111,7 +111,17 @@
111 volume_input,111 volume_input,
112 name);112 name);
113*/113*/
114114 /**
115 We want to send notifications only when we get a scroll update
116 We also only want these notifications to be sent when the server confirms the
117 update => MVC
118 **/
119 g_debug ("Name handle on the handle event %s", name);
120 if (g_ascii_strncasecmp("scroll-updates", name, 14) == 0)
121 {
122 device_set_scroll_update (priv->a_sink, TRUE);
123 }
124
115 slider_menu_item_update_volume (SLIDER_MENU_ITEM (mi), volume_input);125 slider_menu_item_update_volume (SLIDER_MENU_ITEM (mi), volume_input);
116 device_ensure_sink_is_unmuted (priv->a_sink);126 device_ensure_sink_is_unmuted (priv->a_sink);
117}127}
@@ -158,7 +168,7 @@
158 pa_cvolume_set(&new_volume, 1, new_volume_value);168 pa_cvolume_set(&new_volume, 1, new_volume_value);
159169
160 SliderMenuItemPrivate* priv = SLIDER_MENU_ITEM_GET_PRIVATE (self);170 SliderMenuItemPrivate* priv = SLIDER_MENU_ITEM_GET_PRIVATE (self);
161171
162 pa_cvolume_set(&priv->volume, priv->channel_map.channels, new_volume_value);172 pa_cvolume_set(&priv->volume, priv->channel_map.channels, new_volume_value);
163 pm_update_volume (priv->index, new_volume);173 pm_update_volume (priv->index, new_volume);
164}174}
165175
=== modified file 'src/sound-state-manager.c'
--- src/sound-state-manager.c 2011-04-05 03:14:19 +0000
+++ src/sound-state-manager.c 2011-07-07 09:28:32 +0000
@@ -18,7 +18,6 @@
18*/18*/
1919
20#include <libindicator/indicator-image-helper.h>20#include <libindicator/indicator-image-helper.h>
21#include <libnotify/notify.h>
2221
23#include "config.h"22#include "config.h"
2423
@@ -35,7 +34,6 @@
35 GList* blocked_animation_list;34 GList* blocked_animation_list;
36 SoundState current_state;35 SoundState current_state;
37 GtkImage* speaker_image;36 GtkImage* speaker_image;
38 NotifyNotification* notification;
39 GSettings *settings_manager; 37 GSettings *settings_manager;
40};38};
4139
@@ -48,9 +46,6 @@
48static GList* blocked_iter = NULL;46static GList* blocked_iter = NULL;
49static gboolean can_animate = FALSE;47static gboolean can_animate = FALSE;
5048
51//Notifications
52static void sound_state_manager_notification_init (SoundStateManager* self);
53
54//Animation/State related49//Animation/State related
55static void sound_state_manager_prepare_blocked_animation (SoundStateManager* self);50static void sound_state_manager_prepare_blocked_animation (SoundStateManager* self);
56static gboolean sound_state_manager_start_animation (gpointer user_data);51static gboolean sound_state_manager_start_animation (gpointer user_data);
@@ -75,12 +70,9 @@
75 priv->volume_states = NULL;70 priv->volume_states = NULL;
76 priv->speaker_image = NULL;71 priv->speaker_image = NULL;
77 priv->blocked_animation_list = NULL;72 priv->blocked_animation_list = NULL;
78 priv->notification = NULL;
79 priv->settings_manager = NULL;73 priv->settings_manager = NULL;
8074
81 priv->settings_manager = g_settings_new("com.canonical.indicators.sound");75 priv->settings_manager = g_settings_new("com.canonical.indicators.sound");
82
83 sound_state_manager_notification_init (self);
84 76
85 sound_state_manager_prepare_state_image_names (self);77 sound_state_manager_prepare_state_image_names (self);
86 sound_state_manager_prepare_blocked_animation (self);78 sound_state_manager_prepare_blocked_animation (self);
@@ -108,10 +100,6 @@
108100
109 sound_state_manager_free_the_animation_list (self);101 sound_state_manager_free_the_animation_list (self);
110102
111 if (priv->notification) {
112 notify_uninit();
113 }
114
115 g_object_unref(priv->settings_manager);103 g_object_unref(priv->settings_manager);
116 104
117 G_OBJECT_CLASS (sound_state_manager_parent_class)->dispose (object);105 G_OBJECT_CLASS (sound_state_manager_parent_class)->dispose (object);
@@ -131,67 +119,6 @@
131 design_team_size = gtk_icon_size_register("design-team-size", 22, 22); 119 design_team_size = gtk_icon_size_register("design-team-size", 22, 22);
132}120}
133121
134static void
135sound_state_manager_notification_init (SoundStateManager* self)
136{
137 SoundStateManagerPrivate* priv = SOUND_STATE_MANAGER_GET_PRIVATE(self);
138
139 if (!notify_init(PACKAGE_NAME))
140 return;
141
142 GList* caps = notify_get_server_caps();
143 gboolean has_notify_osd = FALSE;
144
145 if (caps) {
146 if (g_list_find_custom(caps, "x-canonical-private-synchronous",
147 (GCompareFunc) g_strcmp0)) {
148 has_notify_osd = TRUE;
149 }
150 g_list_foreach(caps, (GFunc) g_free, NULL);
151 g_list_free(caps);
152 }
153
154 if (has_notify_osd) {
155 priv->notification = notify_notification_new(PACKAGE_NAME, NULL, NULL);
156 notify_notification_set_hint_string(priv->notification,
157 "x-canonical-private-synchronous", PACKAGE_NAME);
158 }
159}
160
161void
162sound_state_manager_show_notification (SoundStateManager *self,
163 double value)
164{
165 SoundStateManagerPrivate* priv = SOUND_STATE_MANAGER_GET_PRIVATE(self);
166
167 if (priv->notification == NULL ||
168 g_settings_get_boolean (priv->settings_manager, "show-notify-osd-on-scroll") == FALSE){
169 return;
170 }
171
172 char *icon;
173 const int notify_value = CLAMP((int)value, -1, 101);
174
175 SoundState state = sound_state_get_from_volume ((int)value);
176
177 if (state == ZERO_LEVEL) {
178 // Not available for all the themes
179 icon = "notification-audio-volume-off";
180 } else if (state == LOW_LEVEL) {
181 icon = "notification-audio-volume-low";
182 } else if (state == MEDIUM_LEVEL) {
183 icon = "notification-audio-volume-medium";
184 } else if (state == HIGH_LEVEL) {
185 icon = "notification-audio-volume-high";
186 } else {
187 icon = "notification-audio-volume-muted";
188 }
189
190 notify_notification_update(priv->notification, PACKAGE_NAME, NULL, icon);
191 notify_notification_set_hint_int32(priv->notification, "value", notify_value);
192 notify_notification_show(priv->notification, NULL);
193}
194
195122
196/*123/*
197Prepare states versus images names hash.124Prepare states versus images names hash.
198125
=== modified file 'src/sound-state-manager.h'
--- src/sound-state-manager.h 2011-02-11 15:21:00 +0000
+++ src/sound-state-manager.h 2011-07-07 09:28:32 +0000
@@ -58,8 +58,6 @@
58void sound_state_manager_get_state_cb (GObject *object,58void sound_state_manager_get_state_cb (GObject *object,
59 GAsyncResult *res,59 GAsyncResult *res,
60 gpointer user_data);60 gpointer user_data);
61void sound_state_manager_show_notification (SoundStateManager *self,
62 double value);
6361
6462
65G_END_DECLS63G_END_DECLS

Subscribers

People subscribed via source and target branches