Move notification code out of volume control object

Approved by: Charles Kerr, Jussi Pakkanen, PS Jenkins bot

4 files changed, 155 insertions, 164 deletions

diff --git a/data/com.canonical.indicator.sound.gschema.xml b/data/com.canonical.indicator.sound.gschema.xml
index 102a1db..a346c0d 100644
--- a/data/com.canonical.indicator.sound.gschema.xml
+++ b/data/com.canonical.indicator.sound.gschema.xml
@@ -32,15 +32,6 @@
         On start up volume should not be muted.
       </description>
     </key>
-    <key name="show-notify-osd-on-scroll" type="b">
-      <default>true</default>
-      <summary>Initial setting for showing notify-osd notification on scroll volume-change</summary>
-      <description>
-        When using the mouse scroll-wheel over the indicator-sound icon, the volume changes.
-        Enabling this setting, every scroll volume-change a notify-osd bubble with the
-        updated volume value will be shown (if supported by your notification daemon).
-      </description>
-    </key>
     <key name="visible" type="b">
       <default>true</default>
       <summary>Whether or not to show the sound indicator in the menu bar.</summary>
diff --git a/src/service.vala b/src/service.vala
index 3bb8b39..fb56215 100644
--- a/src/service.vala
+++ b/src/service.vala
@@ -19,6 +19,13 @@
 
 public class IndicatorSound.Service: Object {
 	public Service (MediaPlayerList playerlist) {
+		sync_notification = new Notify.Notification(_("Volume"), "", "audio-volume-muted");
+		this.notification_server_watch = GLib.Bus.watch_name(GLib.BusType.SESSION,
+			"org.freedesktop.Notifications",
+			GLib.BusNameWatcherFlags.NONE,
+			() => { check_sync_notification = false; },
+			() => { check_sync_notification = false; });
+
 		this.settings = new Settings ("com.canonical.indicator.sound");
 		this.sharedsettings = new Settings ("com.ubuntu.sound");
 
@@ -49,7 +56,7 @@ public class IndicatorSound.Service: Object {
 		this.actions.add_action (this.create_mute_action ());
 		this.actions.add_action (this.create_volume_action ());
 		this.actions.add_action (this.create_mic_volume_action ());
-		this.actions.add_action (this.create_high_volume_actions ());
+		this.actions.add_action (this.create_high_volume_action ());
 
 		this.menus = new HashTable<string, SoundMenu> (str_hash, str_equal);
 		this.menus.insert ("desktop_greeter", new SoundMenu (null, SoundMenu.DisplayFlags.SHOW_MUTE | SoundMenu.DisplayFlags.HIDE_PLAYERS | SoundMenu.DisplayFlags.GREETER_PLAYERS));
@@ -70,15 +77,19 @@ public class IndicatorSound.Service: Object {
 			this.sync_preferred_players ();
 		});
 
-		if (settings.get_boolean ("show-notify-osd-on-scroll")) {
-			List<string> caps = Notify.get_server_caps ();
-			if (caps.find_custom ("x-canonical-private-synchronous", strcmp) != null) {
-				this.notification = new Notify.Notification ("indicator-sound", "", "");
-				this.notification.set_hint ("x-canonical-private-synchronous", "indicator-sound");
-			}
-		}
-
 		sharedsettings.bind ("allow-amplified-volume", this, "allow-amplified-volume", SettingsBindFlags.GET);
+
+		/* Hide the notification when the menu is shown */
+		var shown_action = actions.lookup_action ("indicator-shown") as SimpleAction;
+		shown_action.change_state.connect ((state) => {
+			if (state.get_boolean()) {
+				try {
+					sync_notification.close();
+				} catch (Error e) {
+					warning("Unable to close synchronous volume notification: %s", e.message);
+				}
+			}
+		});
 	}
 
 	~Service() {
@@ -86,6 +97,11 @@ public class IndicatorSound.Service: Object {
 			Source.remove (this.sound_was_blocked_timeout_id);
 			this.sound_was_blocked_timeout_id = 0;
 		}
+
+		if (this.notification_server_watch != 0) {
+			GLib.Bus.unwatch_name(this.notification_server_watch);
+			this.notification_server_watch = 0;
+		}
 	}
 
 	bool greeter_show_track () {
@@ -143,7 +159,7 @@ public class IndicatorSound.Service: Object {
 			}
 
 			/* Normalize volume, because the volume action's state is [0.0, 1.0], see create_volume_action() */
-			this.actions.change_action_state ("volume", this.volume_control.get_volume () / this.max_volume);
+			this.actions.change_action_state ("volume", this.volume_control.volume / this.max_volume);
 		}
 	}
 
@@ -152,6 +168,7 @@ public class IndicatorSound.Service: Object {
 		{ "scroll", activate_scroll_action, "i", null, null },
 		{ "desktop-settings", activate_desktop_settings, null, null, null },
 		{ "phone-settings", activate_phone_settings, null, null, null },
+		{ "indicator-shown", null, null, "@b false", null },
 	};
 
 	MainLoop loop;
@@ -164,10 +181,11 @@ public class IndicatorSound.Service: Object {
 	uint player_action_update_id;
 	bool mute_blocks_sound;
 	uint sound_was_blocked_timeout_id;
-	Notify.Notification notification;
 	bool syncing_preferred_players = false;
 	AccountsServiceUser? accounts_service = null;
 	bool export_to_accounts_service = false;
+	private Notify.Notification sync_notification;
+	private uint notification_server_watch;
 
 	/* Maximum volume as a scaling factor between the volume action's state and the value in
 	 * this.volume_control. See create_volume_action().
@@ -179,31 +197,8 @@ public class IndicatorSound.Service: Object {
 	void activate_scroll_action (SimpleAction action, Variant? param) {
 		int delta = param.get_int32(); /* positive for up, negative for down */
 
-		double v = this.volume_control.get_volume () + volume_step_percentage * delta;
-		this.volume_control.set_volume (v.clamp (0.0, this.max_volume));
-
-		/* TODO: Don't want to mess up the desktop today, but we should remove this
-		   scrolling change and merge that into volume control's notification */
-		if (this.notification != null) {
-			string icon;
-			if (v <= 0.0)
-				icon = "notification-audio-volume-off";
-			else if (v <= 0.3)
-				icon = "notification-audio-volume-low";
-			else if (v <= 0.7)
-				icon = "notification-audio-volume-medium";
-			else
-				icon = "notification-audio-volume-high";
-
-			this.notification.update ("indicator-sound", "", icon);
-			this.notification.set_hint ("value", ((int32) (100 * v / this.max_volume)).clamp (-1, 101));
-			try {
-				this.notification.show ();
-			}
-			catch (Error e) {
-				warning ("unable to show notification: %s", e.message);
-			}
-		}
+		double v = this.volume_control.volume + volume_step_percentage * delta;
+		this.volume_control.volume = v.clamp (0.0, this.max_volume);
 	}
 
 	void activate_desktop_settings (SimpleAction action, Variant? param) {
@@ -238,7 +233,7 @@ public class IndicatorSound.Service: Object {
 	}
 
 	void update_root_icon () {
-		double volume = this.volume_control.get_volume ();
+		double volume = this.volume_control.volume;
 		string icon;
 		if (this.volume_control.mute)
 			icon = this.mute_blocks_sound ? "audio-volume-muted-blocking-panel" : "audio-volume-muted-panel";
@@ -273,6 +268,62 @@ public class IndicatorSound.Service: Object {
 		root_action.set_state (builder.end());
 	}
 
+	private bool check_sync_notification = false;
+	private bool support_sync_notification = false;
+
+	void update_sync_notification () {
+		if (!check_sync_notification) {
+			List<string> caps = Notify.get_server_caps ();
+			if (caps.find_custom ("x-canonical-private-synchronous", strcmp) != null) {
+				support_sync_notification = true;
+			}
+			check_sync_notification = true;
+		}
+
+		if (!support_sync_notification)
+			return;
+
+		var shown_action = actions.lookup_action ("indicator-shown") as SimpleAction;
+		if (shown_action != null && shown_action.get_state().get_boolean())
+			return;
+
+		/* Determine Label */
+		string volume_label = "";
+		if (volume_control.high_volume)
+			volume_label = _("High volume");
+
+		/* Choose an icon */
+		string icon = "audio-volume-muted";
+		if (volume_control.volume <= 0.0)
+			icon = "audio-volume-muted";
+		else if (volume_control.volume <= 0.3)
+			icon = "audio-volume-low";
+		else if (volume_control.volume <= 0.7)
+			icon = "audio-volume-medium";
+		else
+			icon = "audio-volume-high";
+
+		/* Check tint */
+		string tint = "false";
+		if (volume_control.high_volume)
+			tint = "true";
+
+		/* Put it all into the notification */
+		sync_notification.clear_hints ();
+		sync_notification.update (_("Volume"), volume_label, icon);
+		sync_notification.set_hint ("value", (int32)(volume_control.volume * 100.0));
+		sync_notification.set_hint ("x-canonical-value-bar-tint", tint);
+		sync_notification.set_hint ("x-canonical-private-synchronous", "true");
+		sync_notification.set_hint ("x-canonical-non-shaped-icon", "true");
+
+		/* Show it */
+		try {
+			sync_notification.show ();			
+		} catch (GLib.Error e) {
+			warning("Unable to send volume change notification: %s", e.message);
+		}
+	}
+
 	Action create_silent_mode_action () {
 		bool silentNow = false;
 		if (this.accounts_service != null) {
@@ -347,15 +398,6 @@ public class IndicatorSound.Service: Object {
 		return mute_action;
 	}
 
-	void volume_changed (double volume) {
-		var volume_action = this.actions.lookup_action ("volume") as SimpleAction;
-
-		/* Normalize volume, because the volume action's state is [0.0, 1.0], see create_volume_action() */
-		volume_action.set_state (new Variant.double (volume / this.max_volume));
-
-		this.update_root_icon ();
-	}
-
 	Action create_volume_action () {
 		/* The action's state is between be in [0.0, 1.0] instead of [0.0,
 		 * max_volume], so that we don't need to update the slider menu item
@@ -364,23 +406,31 @@ public class IndicatorSound.Service: Object {
 		 * volume_control.set_volume().
 		 */
 
-		double volume = this.volume_control.get_volume () / this.max_volume;
+		double volume = this.volume_control.volume / this.max_volume;
 
 		var volume_action = new SimpleAction.stateful ("volume", VariantType.INT32, new Variant.double (volume));
 
 		volume_action.change_state.connect ( (action, val) => {
 			double v = val.get_double () * this.max_volume;
-			volume_control.set_volume (v.clamp (0.0, this.max_volume));
+			volume_control.volume = v.clamp (0.0, this.max_volume);
 		});
 
 		/* activating this action changes the volume by the amount given in the parameter */
 		volume_action.activate.connect ( (action, param) => {
 			int delta = param.get_int32 ();
-			double v = volume_control.get_volume () + volume_step_percentage * delta;
-			volume_control.set_volume (v.clamp (0.0, this.max_volume));
+			double v = volume_control.volume + volume_step_percentage * delta;
+			volume_control.volume = v.clamp (0.0, this.max_volume);
 		});
 
-		this.volume_control.volume_changed.connect (volume_changed);
+		this.volume_control.notify["volume"].connect (() => {
+			var vol_action = this.actions.lookup_action ("volume") as SimpleAction;
+
+			/* Normalize volume, because the volume action's state is [0.0, 1.0], see create_volume_action() */
+			vol_action.set_state (new Variant.double (this.volume_control.volume / this.max_volume));
+
+			this.update_root_icon ();
+			this.update_sync_notification ();
+		});
 
 		this.volume_control.bind_property ("ready", volume_action, "enabled", BindingFlags.SYNC_CREATE);
 
@@ -388,14 +438,14 @@ public class IndicatorSound.Service: Object {
 	}
 
 	Action create_mic_volume_action () {
-		var volume_action = new SimpleAction.stateful ("mic-volume", null, new Variant.double (this.volume_control.get_mic_volume ()));
+		var volume_action = new SimpleAction.stateful ("mic-volume", null, new Variant.double (this.volume_control.mic_volume));
 
 		volume_action.change_state.connect ( (action, val) => {
-			volume_control.set_mic_volume (val.get_double ());
+			volume_control.mic_volume = val.get_double ();
 		});
 
-		this.volume_control.mic_volume_changed.connect ( (volume) => {
-			volume_action.set_state (new Variant.double (volume));
+		this.volume_control.notify["mic-volume"].connect ( () => {
+			volume_action.set_state (new Variant.double (this.volume_control.mic_volume));
 		});
 
 		this.volume_control.bind_property ("ready", volume_action, "enabled", BindingFlags.SYNC_CREATE);
@@ -403,11 +453,13 @@ public class IndicatorSound.Service: Object {
 		return volume_action;
 	}
 
-	Action create_high_volume_actions () {
+	Action create_high_volume_action () {
 		var high_volume_action = new SimpleAction.stateful("high-volume", null, new Variant.boolean (this.volume_control.high_volume));
 
-		this.volume_control.notify["high-volume"].connect( () =>
-			high_volume_action.set_state(new Variant.boolean (this.volume_control.high_volume)));
+		this.volume_control.notify["high-volume"].connect( () => {
+			high_volume_action.set_state(new Variant.boolean (this.volume_control.high_volume));
+			update_sync_notification();
+		});
 
 		return high_volume_action;
 	}
diff --git a/src/sound-menu.vala b/src/sound-menu.vala
index 3881faf..96dd143 100644
--- a/src/sound-menu.vala
+++ b/src/sound-menu.vala
@@ -60,6 +60,7 @@ public class SoundMenu: Object
 		root_item.set_attribute ("x-canonical-type", "s", "com.canonical.indicator.root");
 		root_item.set_attribute ("x-canonical-scroll-action", "s", "indicator.scroll");
 		root_item.set_attribute ("x-canonical-secondary-action", "s", "indicator.mute");
+		root_item.set_attribute ("submenu-action", "s", "indicator.indicator-shown");
 		root_item.set_submenu (this.menu);
 
 		this.root = new Menu ();
diff --git a/src/volume-control.vala b/src/volume-control.vala
index 6201935..c5e1883 100644
--- a/src/volume-control.vala
+++ b/src/volume-control.vala
@@ -68,12 +68,8 @@ public class VolumeControl : Object
 	private uint _accountservice_volume_timer = 0;
 	private bool _send_next_local_volume = false;
 	private double _account_service_volume = 0.0;
-	private Notify.Notification _notification;
 	private bool _active_port_headphone = false;
 
-	public signal void volume_changed (double v);
-	public signal void mic_volume_changed (double v);
-
 	/** true when connected to the pulse server */
 	public bool ready { get; set; }
 
@@ -81,7 +77,11 @@ public class VolumeControl : Object
 	public bool active_mic { get; private set; default = false; }
 
 	/** true when high volume warnings should be shown */
-	public bool high_volume { get; set; }
+	public bool high_volume {
+		get {
+			return this._volume > 0.75 && _active_port_headphone;	
+		}
+	}
 
 	public VolumeControl ()
 	{
@@ -91,12 +91,6 @@ public class VolumeControl : Object
 		_mute_cancellable = new Cancellable ();
 		_volume_cancellable = new Cancellable ();
 
-		Notify.init ("Volume");
-		_notification = new Notify.Notification(_("Volume"), "", "audio-volume-muted");
-		_notification.set_hint ("value", 0);
-		_notification.set_hint ("x-canonical-private-synchronous", "true");
-		_notification.set_hint ("x-canonical-non-shaped-icon", "true");
-
 		setup_accountsservice.begin ();
 
 		this.reconnect_to_pulse ();
@@ -162,7 +156,7 @@ public class VolumeControl : Object
 
 	private void sink_info_cb_for_props (Context c, SinkInfo? i, int eol)
 	{
-		bool old_active_port_headphone = this._active_port_headphone;
+		bool old_high_volume = this.high_volume;
 
 		if (i == null)
 			return;
@@ -198,10 +192,12 @@ public class VolumeControl : Object
 				_volume != volume_to_double (i.volume.max ()))
 		{
 			_volume = volume_to_double (i.volume.max ());
-			volume_changed (_volume);
+			this.notify_property("volume");
 			start_local_volume_timer();
-		} else if (this._active_port_headphone != old_active_port_headphone) {
-			volume_changed (_volume);
+		} 
+		
+		if (this.high_volume != old_high_volume) {
+			this.notify_property("high-volume");
 		}
 	}
 
@@ -213,7 +209,7 @@ public class VolumeControl : Object
 		if (_mic_volume != volume_to_double (i.volume.values[0]))
 		{
 			_mic_volume = volume_to_double (i.volume.values[0]);
-			mic_volume_changed (_mic_volume);
+			this.notify_property ("mic-volume");
 		}
 	}
 
@@ -269,7 +265,7 @@ public class VolumeControl : Object
 					if (volume != cvolume) {
 						/* Someone else changed the volume for this role, reflect on the indicator */
 						_volume = volume_to_double (volume);
-						volume_changed (_volume);
+						this.notify_property("volume");
 						start_local_volume_timer();
 					}
 				}
@@ -312,7 +308,7 @@ public class VolumeControl : Object
 				iter.next ("(uu)", &type, &volume);
 
 				_volume = volume_to_double (volume);
-				volume_changed (_volume);
+				this.notify_property("volume");
 				start_local_volume_timer();
 			} catch (GLib.Error e) {
 				warning ("unable to get volume for active role %s (%s)", sink_input_objp, e.message);
@@ -529,7 +525,7 @@ public class VolumeControl : Object
 	private void set_volume_success_cb (Context c, int success)
 	{
 		if ((bool)success)
-			volume_changed (_volume);
+			this.notify_property("volume");
 	}
 
 	private void sink_info_set_volume_cb (Context c, SinkInfo? i, int eol)
@@ -575,7 +571,7 @@ public class VolumeControl : Object
 					new Variant ("(ssv)", "org.PulseAudio.Ext.StreamRestore1.RestoreEntry", "Volume", volume),
 					null, DBusCallFlags.NONE, -1);
 
-			volume_changed (_volume);
+			this.notify_property("volume");
 		} catch (GLib.Error e) {
 			lock (_pa_volume_sig_count) {
 				_pa_volume_sig_count--;
@@ -590,83 +586,29 @@ public class VolumeControl : Object
 			return false;
 
 		if (_volume != volume) {
+			var old_high_volume = this.high_volume;
+
 			_volume = volume;
 			if (_pulse_use_stream_restore)
 				set_volume_active_role.begin ();
 			else
 				context.get_server_info (server_info_cb_for_set_volume);
+
+			this.notify_property("volume");
+
+			if (this.high_volume != old_high_volume)
+				this.notify_property("high-volume");
+
 			return true;
 		} else {
 			return false;
 		}
 	}
 
-	public void set_volume (double volume)
-	{
-		/* Using this to detect whether we're on the phone or not */
-		if (_pulse_use_stream_restore) {
-			/* Watch for extreme */
-			if (volume > 0.75 && _active_port_headphone)
-				high_volume = true;
-			else
-				high_volume = false;
-
-			/* Determine Label */
-			string volume_label = "";
-			if (high_volume)
-				volume_label = _("High volume");
-
-			/* Choose an icon */
-			string icon = "audio-volume-muted";
-			if (volume <= 0.0)
-				icon = "audio-volume-muted";
-			else if (volume <= 0.3)
-				icon = "audio-volume-low";
-			else if (volume <= 0.7)
-				icon = "audio-volume-medium";
-			else
-				icon = "audio-volume-high";
-
-			/* Choose a sound */
-			string? sound = null;
-			if (!((_active_sink_input >= 0) && (_active_sink_input < _valid_roles.length)
-					&& (_valid_roles[_active_sink_input] == "multimedia")))
-				sound = "/usr/share/sounds/ubuntu/stereo/message.ogg";
-
-			/* Check tint */
-			string tint = "false";
-			if (high_volume)
-				tint = "true";
-
-			/* Put it all into the notification */
-			_notification.clear_hints ();
-			_notification.update (_("Volume"), volume_label, icon);
-			_notification.set_hint ("value", (int32)(volume * 100.0));
-			/* TODO: Removing sound until we can get all the roles cleaned up for
-			   when to play it. We expect this to come back, but in another landing.
-			_notification.set_hint ("sound-file", sound);
-			 */
-			_notification.set_hint ("x-canonical-value-bar-tint", tint);
-			_notification.set_hint ("x-canonical-private-synchronous", "true");
-			_notification.set_hint ("x-canonical-non-shaped-icon", "true");
-
-			/* Show it */
-			try {
-				_notification.show ();			
-			} catch (GLib.Error e) {
-				warning("Unable to send volume change notification: %s", e.message);
-			}
-		}
-
-		if (set_volume_internal (volume)) {
-			start_local_volume_timer();
-		}
-	}
-
 	void set_mic_volume_success_cb (Context c, int success)
 	{
 		if ((bool)success)
-			mic_volume_changed (_mic_volume);
+			this.notify_property ("mic-volume");
 	}
 
 	void set_mic_volume_get_server_info_cb (PulseAudio.Context c, PulseAudio.ServerInfo? i) {
@@ -677,23 +619,28 @@ public class VolumeControl : Object
 		}
 	}
 
-	public void set_mic_volume (double volume)
-	{
-		return_if_fail (context.get_state () == Context.State.READY);
-
-		_mic_volume = volume;
-
-		context.get_server_info (set_mic_volume_get_server_info_cb);
+	public double volume {
+		get {
+			return _volume;
+		}
+		set {
+			if (set_volume_internal (value)) {
+				start_local_volume_timer();
+			}
+		}
 	}
 
-	public double get_volume ()
-	{
-		return _volume;
-	}
+	public double mic_volume {
+		get {
+			return _mic_volume;
+		}
+		set {
+			return_if_fail (context.get_state () == Context.State.READY);
 
-	public double get_mic_volume ()
-	{
-		return _mic_volume;
+			_mic_volume = value;
+
+			context.get_server_info (set_mic_volume_get_server_info_cb);
+		}
 	}
 
 	/* PulseAudio Dbus (Stream Restore) logic */