Merge lp:~charlesk/indicator-datetime/rtm-14.09-lp-1410874-alarm-sounds-vs-calendar-sounds into lp:indicator-datetime/rtm-14.09

Proposed by Charles Kerr on 2015-01-21
Status: Merged
Approved by: Pat McGowan on 2015-01-25
Approved revision: 394
Merged at revision: 393
Proposed branch: lp:~charlesk/indicator-datetime/rtm-14.09-lp-1410874-alarm-sounds-vs-calendar-sounds
Merge into: lp:indicator-datetime/rtm-14.09
Diff against target: 281 lines (+128/-14)
6 files modified
include/notifications/sound.h (+1/-1)
src/CMakeLists.txt (+3/-0)
src/com.ubuntu.touch.AccountsService.Sound.xml (+42/-0)
src/snap.cpp (+57/-6)
src/sound.cpp (+12/-7)
tests/manual (+13/-0)
To merge this branch: bzr merge lp:~charlesk/indicator-datetime/rtm-14.09-lp-1410874-alarm-sounds-vs-calendar-sounds
Reviewer Review Type Date Requested Status
Antti Kaijanmäki (community) 2015-01-21 Approve on 2015-01-21
Review via email: mp+247229@code.launchpad.net

Commit Message

Don't play calendar alarm sounds if Silent Mode is enabled. Use the alarm sound role for both clock and calendar alarms.

Description of the Change

== Change Description

1. Unconditionally set the alarm role for sounds played from indicator-datetime, whether they're clock alarms or calendar event alarms

2. Don't play calendar event alarms if AccountServices.Sound.SilentMode is set.

== Checklist

> Are there any related MPs required for this MP to build/function as expected? Please list.

No prerequisites

> Is your branch in sync with latest trunk? (e.g. bzr pull lp:trunk -> no changes)

Yes

> Did the code build without warnings?

Yes

> Did the tests run successfully?

Yes

> Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

> If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

N/A

> What device (or emulator) has your component test plan been executed successfully on?

Krillin 14.10 r198

> What manual tests are relevant for this MP?

indicator-datetime/silent-mode

> Did you include a link to the MR Review Checklist Template to make your reviewer's life easier?

https://wiki.ubuntu.com/Process/Merges/Checklists/indicator-datetime

To post a comment you must log in.
Antti Kaijanmäki (kaijanmaki) wrote :

LGTM.

review: Approve
394. By Charles Kerr on 2015-01-22

in Snap's Sound() ctor, use an 'alarm' role for the alarm clock and 'alert' for calendar events

Charles Kerr (charlesk) wrote :

The r394 push tested & passed on Krillin 14.04 r207

Pat McGowan (pat-mcgowan) wrote :

approving as its the same as approved for trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/notifications/sound.h'
2--- include/notifications/sound.h 2014-07-27 04:35:38 +0000
3+++ include/notifications/sound.h 2015-01-22 04:37:48 +0000
4@@ -41,7 +41,7 @@
5 class Sound
6 {
7 public:
8- Sound(const std::string& uri, unsigned int volume, bool loop);
9+ Sound(const std::string& role, const std::string& uri, unsigned int volume, bool loop);
10 ~Sound();
11
12 private:
13
14=== modified file 'src/CMakeLists.txt'
15--- src/CMakeLists.txt 2014-09-02 16:15:58 +0000
16+++ src/CMakeLists.txt 2015-01-22 04:37:48 +0000
17@@ -47,6 +47,9 @@
18 add_gdbus_codegen(SERVICE_GENERATED_SOURCES dbus-alarm-properties
19 com.canonical.indicator
20 ${CMAKE_SOURCE_DIR}/data/com.canonical.indicator.datetime.AlarmProperties.xml)
21+add_gdbus_codegen(SERVICE_GENERATED_SOURCES dbus-accounts-sound
22+ com.ubuntu.touch
23+ ${CMAKE_SOURCE_DIR}/src/com.ubuntu.touch.AccountsService.Sound.xml)
24
25 # add warnings/coverage info on handwritten files
26 # but not the autogenerated ones...
27
28=== added file 'src/com.ubuntu.touch.AccountsService.Sound.xml'
29--- src/com.ubuntu.touch.AccountsService.Sound.xml 1970-01-01 00:00:00 +0000
30+++ src/com.ubuntu.touch.AccountsService.Sound.xml 2015-01-22 04:37:48 +0000
31@@ -0,0 +1,42 @@
32+<node>
33+ <interface name="com.ubuntu.touch.AccountsService.Sound">
34+
35+ <annotation name="org.freedesktop.Accounts.VendorExtension" value="true"/>
36+
37+ <!-- Muted is all sound, SilentMode is only non-user-initiated sounds -->
38+ <property name="SilentMode" type="b" access="readwrite">
39+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="false"/>
40+ </property>
41+
42+ <property name="IncomingCallSound" type="s" access="readwrite">
43+ <annotation name="org.freedesktop.Accounts.DefaultValue.String"
44+ value="/usr/share/sounds/ubuntu/ringtones/Ubuntu.ogg"/>
45+ </property>
46+
47+ <property name="IncomingMessageSound" type="s" access="readwrite">
48+ <annotation name="org.freedesktop.Accounts.DefaultValue.String"
49+ value="/usr/share/sounds/ubuntu/notifications/Xylo.ogg"/>
50+ </property>
51+
52+ <property name="IncomingCallVibrate" type="b" access="readwrite">
53+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
54+ </property>
55+
56+ <property name="IncomingCallVibrateSilentMode" type="b" access="readwrite">
57+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
58+ </property>
59+
60+ <property name="IncomingMessageVibrate" type="b" access="readwrite">
61+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
62+ </property>
63+
64+ <property name="IncomingMessageVibrateSilentMode" type="b" access="readwrite">
65+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
66+ </property>
67+
68+ <property name="DialpadSoundsEnabled" type="b" access="readwrite">
69+ <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
70+ </property>
71+
72+ </interface>
73+</node>
74
75=== modified file 'src/snap.cpp'
76--- src/snap.cpp 2014-12-08 02:48:13 +0000
77+++ src/snap.cpp 2015-01-22 04:37:48 +0000
78@@ -17,6 +17,8 @@
79 * Charles Kerr <charles.kerr@canonical.com>
80 */
81
82+#include "dbus-accounts-sound.h"
83+
84 #include <datetime/snap.h>
85 #include <datetime/utils.h> // is_locale_12h()
86
87@@ -32,6 +34,9 @@
88 #include <set>
89 #include <string>
90
91+#include <unistd.h> // getuid()
92+#include <sys/types.h> // getuid()
93+
94 namespace uin = unity::indicator::notifications;
95
96 namespace unity {
97@@ -49,12 +54,26 @@
98 Impl(const std::shared_ptr<unity::indicator::notifications::Engine>& engine,
99 const std::shared_ptr<const Settings>& settings):
100 m_engine(engine),
101- m_settings(settings)
102+ m_settings(settings),
103+ m_cancellable(g_cancellable_new())
104 {
105+ auto object_path = g_strdup_printf("/org/freedesktop/Accounts/User%lu", (gulong)getuid());
106+ accounts_service_sound_proxy_new_for_bus(G_BUS_TYPE_SYSTEM,
107+ G_DBUS_PROXY_FLAGS_GET_INVALIDATED_PROPERTIES,
108+ "org.freedesktop.Accounts",
109+ object_path,
110+ m_cancellable,
111+ on_sound_proxy_ready,
112+ this);
113+ g_free(object_path);
114 }
115
116 ~Impl()
117 {
118+ g_cancellable_cancel(m_cancellable);
119+ g_clear_object(&m_cancellable);
120+ g_clear_object(&m_accounts_service_sound_proxy);
121+
122 for (const auto& key : m_notifications)
123 m_engine->close (key);
124 }
125@@ -72,11 +91,16 @@
126 // force the system to stay awake
127 auto awake = std::make_shared<uin::Awake>(m_engine->app_name());
128
129- // create the sound...
130- const auto uri = get_alarm_uri(appointment, m_settings);
131- const auto volume = m_settings->alarm_volume.get();
132- const bool loop = interactive;
133- auto sound = std::make_shared<uin::Sound>(uri, volume, loop);
134+ // calendar events are muted in silent mode; alarm clocks never are
135+ std::shared_ptr<uin::Sound> sound;
136+ if (appointment.is_ubuntu_alarm() || !silent_mode()) {
137+ // create the sound.
138+ const auto role = appointment.is_ubuntu_alarm() ? "alarm" : "alert";
139+ const auto uri = get_alarm_uri(appointment, m_settings);
140+ const auto volume = m_settings->alarm_volume.get();
141+ const bool loop = interactive;
142+ sound = std::make_shared<uin::Sound>(role, uri, volume, loop);
143+ }
144
145 // create the haptic feedback...
146 const auto haptic_mode = m_settings->alarm_haptic.get();
147@@ -131,6 +155,31 @@
148
149 private:
150
151+ static void on_sound_proxy_ready(GObject* /*source_object*/, GAsyncResult* res, gpointer gself)
152+ {
153+ GError * error;
154+
155+ error = nullptr;
156+ auto proxy = accounts_service_sound_proxy_new_for_bus_finish (res, &error);
157+ if (error != nullptr)
158+ {
159+ if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
160+ g_warning("%s Couldn't find accounts service sound proxy: %s", G_STRLOC, error->message);
161+
162+ g_clear_error(&error);
163+ }
164+ else
165+ {
166+ static_cast<Impl*>(gself)->m_accounts_service_sound_proxy = proxy;
167+ }
168+ }
169+
170+ bool silent_mode() const
171+ {
172+ return (m_accounts_service_sound_proxy != nullptr)
173+ && (accounts_service_sound_get_silent_mode(m_accounts_service_sound_proxy));
174+ }
175+
176 std::string get_alarm_uri(const Appointment& appointment,
177 const std::shared_ptr<const Settings>& settings) const
178 {
179@@ -167,6 +216,8 @@
180 const std::shared_ptr<unity::indicator::notifications::Engine> m_engine;
181 const std::shared_ptr<const Settings> m_settings;
182 std::set<int> m_notifications;
183+ GCancellable * m_cancellable {nullptr};
184+ AccountsServiceSound * m_accounts_service_sound_proxy {nullptr};
185 };
186
187 /***
188
189=== modified file 'src/sound.cpp'
190--- src/sound.cpp 2014-09-17 16:51:51 +0000
191+++ src/sound.cpp 2015-01-22 04:37:48 +0000
192@@ -38,9 +38,11 @@
193 {
194 public:
195
196- Impl(const std::string& uri,
197+ Impl(const std::string& role,
198+ const std::string& uri,
199 unsigned int volume,
200 bool loop):
201+ m_role(role),
202 m_uri(uri),
203 m_volume(volume),
204 m_loop(loop)
205@@ -98,8 +100,9 @@
206 static gboolean bus_callback(GstBus*, GstMessage* msg, gpointer gself)
207 {
208 auto self = static_cast<Impl*>(gself);
209+ const auto message_type = GST_MESSAGE_TYPE(msg);
210
211- if ((GST_MESSAGE_TYPE(msg) == GST_MESSAGE_EOS) && (self->m_loop))
212+ if ((message_type == GST_MESSAGE_EOS) && (self->m_loop))
213 {
214 gst_element_seek(self->m_play,
215 1.0,
216@@ -110,7 +113,7 @@
217 GST_SEEK_TYPE_NONE,
218 (gint64)GST_CLOCK_TIME_NONE);
219 }
220- else if ((GST_MESSAGE_TYPE(msg) == GST_MESSAGE_STREAM_START) && (self->m_loop))
221+ else if (message_type == GST_MESSAGE_STREAM_START)
222 {
223 /* Set the media role if audio sink is pulsesink */
224 GstElement *audio_sink = nullptr;
225@@ -121,10 +124,11 @@
226 feature = GST_PLUGIN_FEATURE_CAST(GST_ELEMENT_GET_CLASS(audio_sink)->elementfactory);
227 if (feature && g_strcmp0(gst_plugin_feature_get_name(feature), "pulsesink") == 0)
228 {
229- std::string role_str("props,media.role=alarm");
230- GstStructure *props = gst_structure_from_string(role_str.c_str(), nullptr);
231+ auto role_str = g_strdup_printf("props,media.role=%s", self->m_role.c_str());
232+ GstStructure *props = gst_structure_from_string(role_str, nullptr);
233 g_object_set(audio_sink, "stream-properties", props, nullptr);
234 gst_structure_free(props);
235+ g_free(role_str);
236 }
237 gst_object_unref(audio_sink);
238 }
239@@ -137,6 +141,7 @@
240 ****
241 ***/
242
243+ const std::string m_role;
244 const std::string m_uri;
245 const unsigned int m_volume;
246 const bool m_loop;
247@@ -144,8 +149,8 @@
248 GstElement* m_play = nullptr;
249 };
250
251-Sound::Sound(const std::string& uri, unsigned int volume, bool loop):
252- impl (new Impl(uri, volume, loop))
253+Sound::Sound(const std::string& role, const std::string& uri, unsigned int volume, bool loop):
254+ impl (new Impl(role, uri, volume, loop))
255 {
256 }
257
258
259=== modified file 'tests/manual'
260--- tests/manual 2015-01-15 19:45:58 +0000
261+++ tests/manual 2015-01-22 04:37:48 +0000
262@@ -124,6 +124,19 @@
263 <dd>The newly-selected sound should play, rather than the previous sound.</dd>
264 </dl>
265
266+Test-case indicator-datetime/silent-mode
267+<dl>
268+ <dt>Set an alarm and wait for it to arrive.</dt>
269+ <dt>From the sound indicator, turn on silent mode.</dt>
270+ <dd>Alarm should go off at the specified time and play its sound regardless of silent mode.</dd>
271+ <dt>From the sound indicator, turn on silent mode.</dt>
272+ <dt>Create a calendar event from the calendar app and wait for it to arrive.</dt>
273+ <dd>The calendar event notification should be silent.</dd>
274+ <dt>From the sound indicator, turn off silent mode.</dt>
275+ <dt>Create a calendar event from the calendar app and wait for it to arrive.</dt>
276+ <dd>The calendar event notification should play a sound.</dd>
277+</dl>
278+
279 <strong>
280 If all actions produce the expected results listed, please <a href="results#add_result">submit</a> a 'passed' result.
281 If an action fails, or produces an unexpected result, please <a href="results#add_result">submit</a> a 'failed' result and <a href="../../buginstructions">file a bug</a>. Please be sure to include the bug number when you <a href="results#add_result">submit</a> your result</strong>.

Subscribers

People subscribed via source and target branches