Merge lp:~ted/indicator-sound/mega-merge into lp:~ted/indicator-sound/remove-vala-warnings

Proposed by Ted Gould
Status: Superseded
Proposed branch: lp:~ted/indicator-sound/mega-merge
Merge into: lp:~ted/indicator-sound/remove-vala-warnings
Diff against target: 406 lines (+267/-2)
5 files modified
debian/changelog (+13/-0)
src/service.vala (+19/-0)
src/sound-menu.vala (+39/-1)
src/volume-control.vala (+97/-1)
tests/manual (+99/-0)
To merge this branch: bzr merge lp:~ted/indicator-sound/mega-merge
Reviewer Review Type Date Requested Status
Ted Gould Pending
Review via email: mp+240750@code.launchpad.net

This proposal has been superseded by a proposal from 2014-11-05.

Commit message

Combining several MRs to resolve conflicts manually

To post a comment you must log in.
lp:~ted/indicator-sound/mega-merge updated
468. By Ted Gould

Fixing bug number

469. By Ted Gould

Missed part of a merge

Unmerged revisions

469. By Ted Gould

Missed part of a merge

468. By Ted Gould

Fixing bug number

467. By Ted Gould

Ensure the greeter menu matches whether song metadata should be shown,
and update the metadata based on the new setting. (LP: #1358340)

466. By Ted Gould

Integration test for silent mode

465. By Ted Gould

Integration test for audio roles

464. By Ted Gould

service.vala: don't call set_volume unnecessarily (LP: #1381871)

463. By Ted Gould

Warn on high audio levels when using headphones (LP: #123633, #1373404)

462. By Ted Gould

Show notifications on volume change (LP: #1378564, #1378961)

461. By Ted Gould

Remove various Vala warnings

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2014-10-10 15:40:30 +0000
3+++ debian/changelog 2014-11-05 16:45:30 +0000
4@@ -1,3 +1,16 @@
5+indicator-sound (12.10.2+14.10.20141010-0ubuntu8) UNRELEASED; urgency=medium
6+
7+ * Remove various Vala warnings
8+ * Show notifications on volume change (LP: #1378564, #1378961)
9+ * Warn on high audio levels when using headphones (LP: #123633, #1373404)
10+ * service.vala: don't call set_volume unnecessarily (LP: #1381871)
11+ * Integration test for audio roles
12+ * Integration test for silent mode
13+ * Ensure the greeter menu matches whether song metadata should be shown,
14+ and update the metadata based on the new setting. (LP: #1358340)
15+
16+ -- Ted Gould <ted@ubuntu.com> Wed, 05 Nov 2014 10:42:12 -0600
17+
18 indicator-sound (12.10.2+14.10.20141010-0ubuntu1) utopic; urgency=low
19
20 [ Ricardo Salveti de Araujo ]
21
22=== modified file 'src/service.vala'
23--- src/service.vala 2014-10-22 20:00:18 +0000
24+++ src/service.vala 2014-11-05 16:45:30 +0000
25@@ -49,6 +49,7 @@
26 this.actions.add_action (this.create_mute_action ());
27 this.actions.add_action (this.create_volume_action ());
28 this.actions.add_action (this.create_mic_volume_action ());
29+ this.actions.add_action (this.create_high_volume_actions ());
30
31 this.menus = new HashTable<string, SoundMenu> (str_hash, str_equal);
32 this.menus.insert ("desktop_greeter", new SoundMenu (null, SoundMenu.DisplayFlags.SHOW_MUTE | SoundMenu.DisplayFlags.HIDE_PLAYERS | SoundMenu.DisplayFlags.GREETER_PLAYERS));
33@@ -60,6 +61,10 @@
34 this.volume_control.bind_property ("active-mic", menu, "show-mic-volume", BindingFlags.SYNC_CREATE);
35 });
36
37+ this.menus.@foreach ( (profile, menu) => {
38+ this.volume_control.bind_property ("high-volume", menu, "show-high-volume-warning", BindingFlags.SYNC_CREATE);
39+ });
40+
41 this.sync_preferred_players ();
42 this.settings.changed["interested-media-players"].connect ( () => {
43 this.sync_preferred_players ();
44@@ -126,6 +131,9 @@
45 }
46
47 set {
48+ if (this.allow_amplified_volume == value)
49+ return;
50+
51 if (value) {
52 /* from pulse/volume.h: #define PA_VOLUME_UI_MAX (pa_sw_volume_from_dB(+11.0)) */
53 this.max_volume = (double)PulseAudio.Volume.sw_from_dB(11.0) / PulseAudio.Volume.NORM;
54@@ -174,6 +182,8 @@
55 double v = this.volume_control.get_volume () + volume_step_percentage * delta;
56 this.volume_control.set_volume (v.clamp (0.0, this.max_volume));
57
58+ /* TODO: Don't want to mess up the desktop today, but we should remove this
59+ scrolling change and merge that into volume control's notification */
60 if (this.notification != null) {
61 string icon;
62 if (v <= 0.0)
63@@ -388,6 +398,15 @@
64 return volume_action;
65 }
66
67+ Action create_high_volume_actions () {
68+ var high_volume_action = new SimpleAction.stateful("high-volume", null, new Variant.boolean (this.volume_control.high_volume));
69+
70+ this.volume_control.notify["high-volume"].connect( () =>
71+ high_volume_action.set_state(new Variant.boolean (this.volume_control.high_volume)));
72+
73+ return high_volume_action;
74+ }
75+
76 void bus_acquired (DBusConnection connection, string name) {
77 try {
78 connection.export_action_group ("/com/canonical/indicator/sound", this.actions);
79
80=== modified file 'src/sound-menu.vala'
81--- src/sound-menu.vala 2014-10-08 01:57:56 +0000
82+++ src/sound-menu.vala 2014-11-05 16:45:30 +0000
83@@ -93,12 +93,49 @@
84 this.mic_volume_shown = true;
85 }
86 else if (!value && this.mic_volume_shown) {
87- this.volume_section.remove (this.volume_section.get_n_items () -1);
88+ int location = -1;
89+ while ((location = find_action(this.volume_section, "indicator.mic-volume")) != -1) {
90+ this.volume_section.remove (location);
91+ }
92 this.mic_volume_shown = false;
93 }
94 }
95 }
96
97+ public bool show_high_volume_warning {
98+ get {
99+ return this.high_volume_warning_shown;
100+ }
101+ set {
102+ if (value && !this.high_volume_warning_shown) {
103+ /* NOTE: Action doesn't really exist, just used to find below when removing */
104+ var item = new MenuItem(_("High volume can damage your hearing."), "indicator.high-volume-warning-item");
105+ volume_section.append_item (item);
106+ this.high_volume_warning_shown = true;
107+ }
108+ else if (!value && this.high_volume_warning_shown) {
109+ int location = -1;
110+ while ((location = find_action(this.volume_section, "indicator.high-volume-warning-item")) != -1) {
111+ this.volume_section.remove (location);
112+ }
113+ this.high_volume_warning_shown = false;
114+ }
115+ }
116+ }
117+
118+ int find_action (Menu menu, string in_action) {
119+ int n = menu.get_n_items ();
120+ for (int i = 0; i < n; i++) {
121+ string action;
122+ menu.get_item_attribute (i, "action", "s", out action);
123+ if (in_action == action)
124+ return i;
125+ }
126+
127+ return -1;
128+ }
129+
130+
131 public void add_player (MediaPlayer player) {
132 if (this.notify_handlers.contains (player))
133 return;
134@@ -141,6 +178,7 @@
135 Menu volume_section;
136 bool mic_volume_shown;
137 bool settings_shown = false;
138+ bool high_volume_warning_shown = false;
139 bool hide_inactive;
140 bool hide_players = false;
141 HashTable<MediaPlayer, ulong> notify_handlers;
142
143=== modified file 'src/volume-control.vala'
144--- src/volume-control.vala 2014-10-22 19:58:32 +0000
145+++ src/volume-control.vala 2014-11-05 16:45:30 +0000
146@@ -19,6 +19,7 @@
147 */
148
149 using PulseAudio;
150+using Notify;
151 using Gee;
152
153 [CCode(cname="pa_cvolume_set", cheader_filename = "pulse/volume.h")]
154@@ -67,6 +68,8 @@
155 private uint _accountservice_volume_timer = 0;
156 private bool _send_next_local_volume = false;
157 private double _account_service_volume = 0.0;
158+ private Notify.Notification _notification;
159+ private bool _active_port_headphone = false;
160
161 public signal void volume_changed (double v);
162 public signal void mic_volume_changed (double v);
163@@ -77,6 +80,9 @@
164 /** true when a microphone is active **/
165 public bool active_mic { get; private set; default = false; }
166
167+ /** true when high volume warnings should be shown */
168+ public bool high_volume { get; set; }
169+
170 public VolumeControl ()
171 {
172 if (loop == null)
173@@ -84,6 +90,13 @@
174
175 _mute_cancellable = new Cancellable ();
176 _volume_cancellable = new Cancellable ();
177+
178+ Notify.init ("Volume");
179+ _notification = new Notify.Notification(_("Volume"), "", "audio-volume-muted");
180+ _notification.set_hint ("value", 0);
181+ _notification.set_hint ("x-canonical-private-synchronous", "true");
182+ _notification.set_hint ("x-canonical-non-shaped-icon", "true");
183+
184 setup_accountsservice.begin ();
185
186 this.reconnect_to_pulse ();
187@@ -149,6 +162,8 @@
188
189 private void sink_info_cb_for_props (Context c, SinkInfo? i, int eol)
190 {
191+ bool old_active_port_headphone = this._active_port_headphone;
192+
193 if (i == null)
194 return;
195
196@@ -165,12 +180,27 @@
197 this.notify_property ("is-playing");
198 }
199
200+ /* Check if the current active port is headset/headphone */
201+ /* There is not easy way to check if the port is a headset/headphone besides
202+ * checking for the port name. On touch (with the pulseaudio droid element)
203+ * the headset/headphone port is called 'output-headset' and 'output-headphone'.
204+ * On the desktop this is usually called 'analog-output-headphones' */
205+ if (i.active_port.name == "output-wired_headset" ||
206+ i.active_port.name == "output-wired_headphone" ||
207+ i.active_port.name == "analog-output-headphones") {
208+ _active_port_headphone = true;
209+ } else {
210+ _active_port_headphone = false;
211+ }
212+
213 if (_pulse_use_stream_restore == false &&
214 _volume != volume_to_double (i.volume.max ()))
215 {
216 _volume = volume_to_double (i.volume.max ());
217 volume_changed (_volume);
218 start_local_volume_timer();
219+ } else if (this._active_port_headphone != old_active_port_headphone) {
220+ volume_changed (_volume);
221 }
222 }
223
224@@ -572,8 +602,74 @@
225
226 public void set_volume (double volume)
227 {
228- if (set_volume_internal (volume))
229+ /* Using this to detect whether we're on the phone or not */
230+ if (_pulse_use_stream_restore) {
231+<<<<<<< TREE
232+ if (volume == 0.0)
233+ _notification.update (_("Volume"), "", "audio-volume-muted");
234+ if (volume > 0.0 && volume <= 0.33)
235+ _notification.update (_("Volume"), "", "audio-volume-low");
236+ if (volume > 0.33 && volume <= 0.66)
237+ _notification.update (_("Volume"), "", "audio-volume-medium");
238+ if (volume > 0.66 && volume <= 1.0)
239+ _notification.update (_("Volume"), "", "audio-volume-high");
240+=======
241+ /* Watch for extreme */
242+ if (volume > 0.75 && _active_port_headphone)
243+ high_volume = true;
244+ else
245+ high_volume = false;
246+
247+ /* Determine Label */
248+ string volume_label = "";
249+ if (high_volume)
250+ volume_label = _("High volume");
251+
252+ /* Choose an icon */
253+ string icon = "audio-volume-muted";
254+ if (volume <= 0.0)
255+ icon = "audio-volume-muted";
256+ else if (volume <= 0.3)
257+ icon = "audio-volume-low";
258+ else if (volume <= 0.7)
259+ icon = "audio-volume-medium";
260+ else
261+ icon = "audio-volume-high";
262+
263+ /* Choose a sound */
264+ string? sound = null;
265+ if (!((_active_sink_input >= 0) && (_active_sink_input < _valid_roles.length)
266+ && (_valid_roles[_active_sink_input] == "multimedia")))
267+ sound = "/usr/share/sounds/ubuntu/stereo/message.ogg";
268+
269+ /* Check tint */
270+ string tint = "false";
271+ if (high_volume)
272+ tint = "true";
273+
274+ /* Put it all into the notification */
275+ _notification.clear_hints ();
276+ _notification.update (_("Volume"), volume_label, icon);
277+ _notification.set_hint ("value", (int32)(volume * 100.0));
278+ /* TODO: Removing sound until we can get all the roles cleaned up for
279+ when to play it. We expect this to come back, but in another landing.
280+ _notification.set_hint ("sound-file", sound);
281+ */
282+ _notification.set_hint ("x-canonical-value-bar-tint", tint);
283+ _notification.set_hint ("x-canonical-private-synchronous", "true");
284+ _notification.set_hint ("x-canonical-non-shaped-icon", "true");
285+
286+ /* Show it */
287+ try {
288+ _notification.show ();
289+ } catch (GLib.Error e) {
290+ warning("Unable to send volume change notification: %s", e.message);
291+ }
292+ }
293+
294+ if (set_volume_internal (volume)) {
295 start_local_volume_timer();
296+ }
297 }
298
299 void set_mic_volume_success_cb (Context c, int success)
300
301=== modified file 'tests/manual'
302--- tests/manual 2014-01-31 20:09:51 +0000
303+++ tests/manual 2014-11-05 16:45:30 +0000
304@@ -22,3 +22,102 @@
305 <dd>The menu is populated with items</dd>
306 </dl>
307
308+Test-case indicator-sound/unity8-sound-notifications
309+<dl>
310+ <dt>Adjust volume using HW keys if available</dt>
311+ <dd>A notification bubble should appear with the sound volume</dd>
312+ <dd>An audibule sound should play at the level of the audio</dd>
313+ <dt>Adjust volume with slider in sound indicator</dt>
314+ <dd>A notification bubble should appear with the sound volume</dd>
315+ <dd>An audibule sound should play at the level of the audio</dd>
316+ <dt>Open a video with sound and play in media player</dt>
317+ <dd>The video should play and the sound should be audible</dd>
318+ <dt>Adjust volume using HW keys if available</dt>
319+ <dd>A notification bubble should appear with the sound volume</dd>
320+ <dd>No notification sound should be heard</dd>
321+ <dt>Adjust volume with slider in sound indicator</dt>
322+ <dd>A notification bubble should appear with the sound volume</dd>
323+ <dd>No notification sound should be heard</dd>
324+</dl>
325+
326+Test-case indicator-sound/unity8-high-volume
327+<dl>
328+ <dt>Plug headphones into the headphone jack</dt>
329+ <dt>Adjust volume so that it is at the midpoint of volume range</dt>
330+ <dd>The slider should be in the middle of the scale</dd>
331+ <dt>Increase the volume once using HW keys if available</dt>
332+ <dd>A notification bubble should appear with the sound volume</dd>
333+ <dd>There should be no text on the notification</dd>
334+ <dt>Increase the volume using HW keys until it is roughly 90% of the range</dt>
335+ <dd>A notification bubble should appear with the sound volume</dd>
336+ <dd>The text on the notification should read "High volume"</dd>
337+ <dd>The range on the notification bubble should have a different color signifying the higher volume</dd>
338+ <dt>Decrease the volume using HW keys until it is roughly 50% of the range</dt>
339+ <dd>A notification bubble should appear with the sound volume</dd>
340+ <dd>There should be no text on the notification</dd>
341+ <dd>The range on the notification bubble should have a standard color</dd>
342+</dl>
343+
344+Test-case indicator-sound/unity8-silent-mode
345+<dl>
346+ <dt>NOTE: This test currently doesn't work because of a bug: http://pad.lv/1336715</dt>
347+ <dt>Open the Sound menu</dt>
348+ <dd>The sound menu includes an item "Silent Mode" which is a check box</dd>
349+ <dd>The checkbox is not checked</dd>
350+ <dt>Enable silent mode</dt>
351+ <dd>Selecting the "Silent Mode" item should cause the box to be checked</dd>
352+ <dt>Open the sound panel in system settings</dt>
353+ <dd>The sound panel includes an item "Silent Mode" which is a check box</dd>
354+ <dd>The checkbox is checked</dd>
355+ <dt>Disable silent mode in system settings</dt>
356+ <dd>The checkbox is not checked</dd>
357+ <dt>Open the Sound menu</dt>
358+ <dd>The sound menu includes an item "Silent Mode" which is a check box</dd>
359+ <dd>The checkbox is not checked</dd>
360+</dl>
361+
362+Test-case indicator-sound/unity8-audio-roles
363+<dl>
364+ <dt>Without playing anything (no active audio stream), change the volume on the indicator or with the volume buttons and then try playing one of the following audio streams: camera shutter, ringtone, message notification, dtmf</dt>
365+ <dd>The audio stream should reflect the volume set on the indicator</dd>
366+ <dt>Without playing anything (no active audio stream), change the volume on the indicator or with volume buttons and then try playing one of the following audio streams: music-app, webrowser (youtube)</dt>
367+ <dd>The audio stream should not be affected by the volume set on the indicator when there was no other active stream</dt>
368+ <dt>Play a multimedia stream (music-app, webrowser) and change the volume on the indicator when the stream is active</dt>
369+ <dd>The multimedia audio stream should reflect the volume set on the indicator</dd>
370+ <dd>When stopping/closing the multimedia stream, it should automatically show up the volume for the alert role (ringtone, notification, etc)</dd>
371+ <dd>No other role should be affected by the volume level used by the multimedia role</dd>
372+ <dt>Play a alarm stream (clock-app) and change the volume on the indicator when the stream is active</dt>
373+ <dd>The alarm audio stream should reflect the volume set on the indicator</dd>
374+ <dd>When stopping/closing the alarm stream, it should automatically show up the volume for the alert role (ringtone, notification, etc)</dd>
375+ <dd>No other role should be affected by the volume level used by the alarm role</dd>
376+ <dt>Start a voice call using the dialer-app and change the volume on the indicator when the call is active</dt>
377+ <dd>The phone audio stream should reflect the volume set on the indicator</dd>
378+ <dd>When hanging up the voice call it should automatically show up the volume for the alert role (ringtone, notification, etc)</dd>
379+ <dd>No other role should be affected by the volume level used by the phone role</dd>
380+</dl>
381+
382+Test-case indicator-sound/unity8-embedded-greeter
383+<dl>
384+ <dt>NOTE: Only works with embedded greeter, split greeter will require modifications to this test</dt>
385+ <dt>Ensure System Settings is set to "Show Messages on Greeter"</dt>
386+ <dt>Play a song in the media player</dt>
387+ <dd>The song should be heard</dd>
388+ <dd>There should be an entry in the sound menu with the meta data for the song being played</dd>
389+ <dt>Go to the greeter. This can be done by hitting the lock button twice.</dt>
390+ <dt>Ensure the sound menu has song meta data</dt>
391+ <dd>There should be an entry in the sound menu with the meta data for the song being played</dd>
392+ <dt>Pause the song in the greeter</dt>
393+ <dd>The song should stop playing</dd>
394+ <dt>Resume the song in the greeter</dt>
395+ <dd>The song should continue to play</dd>
396+ <dt>Disable System Settings value "Show Messages on Greeter"</dt>
397+ <dt>Ensure the sound menu has song meta data</dt>
398+ <dd>There should be an entry in the sound menu with the meta data for the song being played</dd>
399+ <dt>Go to the greeter. This can be done by hitting the lock button twice.</dt>
400+ <dt>Ensure the sound menu does not have song meta data</dt>
401+ <dd>There should be an entry for the player but it should have no information on the song being played</dd>
402+ <dt>Pause the song in the greeter</dt>
403+ <dd>The song should stop playing</dd>
404+ <dt>Resume the song in the greeter</dt>
405+ <dd>The song should continue to play</dd>
406+</dl>

Subscribers

People subscribed via source and target branches

to all changes: