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

[Charles Kerr]

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

Sibling rtm-14.09 MP: https://code.launchpad.net/~charlesk/indicator-datetime/lp-1410874-alarm-sounds-vs-calendar-sounds/+merge/247226

== 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

[Antti Kaijanmäki (kaijanmaki)]

LGTM.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:398
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/291/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-amd64-ci/5/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/5/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-datetime-ci/291/rebuild

review: Needs Fixing (continuous-integration)

[lp:~charlesk/indicator-datetime/lp-1410874-alarm-sounds-vs-calendar-sounds]

399. By Charles Kerr on 2015-01-22

tweak an overly-verbose variable name

400. By Charles Kerr on 2015-01-22

require an explicit role for the sound so that we can differentiate between eg 'alarm' and 'alert'

401. 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

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:399
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/292/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/6
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/6/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-datetime-ci/292/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:401
http://jenkins.qa.ubuntu.com/job/indicator-datetime-ci/293/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/7/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-datetime-ci/293/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

http://jenkins.qa.ubuntu.com/job/indicator-datetime-vivid-armhf-ci/7/artifact/work/output/*zip*/output.zip tested & WfM on mako vivid r73

[Antti Kaijanmäki (kaijanmaki)]

LGTM.

review: Approve

=== modified file 'include/notifications/sound.h'
--- include/notifications/sound.h	2014-07-27 04:35:38 +0000
+++ include/notifications/sound.h	2015-01-22 01:59:13 +0000
@@ -41,7 +41,7 @@
 class Sound
 {
 public:
-    Sound(const std::string& uri, unsigned int volume, bool loop);
+    Sound(const std::string& role, const std::string& uri, unsigned int volume, bool loop);
     ~Sound();
 
 private:

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2014-09-02 16:15:58 +0000
+++ src/CMakeLists.txt	2015-01-22 01:59:13 +0000
@@ -47,6 +47,9 @@
 add_gdbus_codegen(SERVICE_GENERATED_SOURCES dbus-alarm-properties
                   com.canonical.indicator
                   ${CMAKE_SOURCE_DIR}/data/com.canonical.indicator.datetime.AlarmProperties.xml)
+add_gdbus_codegen(SERVICE_GENERATED_SOURCES dbus-accounts-sound
+                  com.ubuntu.touch
+                  ${CMAKE_SOURCE_DIR}/src/com.ubuntu.touch.AccountsService.Sound.xml)
 
 # add warnings/coverage info on handwritten files
 # but not the autogenerated ones...

=== added file 'src/com.ubuntu.touch.AccountsService.Sound.xml'
--- src/com.ubuntu.touch.AccountsService.Sound.xml	1970-01-01 00:00:00 +0000
+++ src/com.ubuntu.touch.AccountsService.Sound.xml	2015-01-22 01:59:13 +0000
@@ -0,0 +1,42 @@
+<node>
+    <interface name="com.ubuntu.touch.AccountsService.Sound">
+
+        <annotation name="org.freedesktop.Accounts.VendorExtension" value="true"/>
+
+        <!-- Muted is all sound, SilentMode is only non-user-initiated sounds -->
+        <property name="SilentMode" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="false"/>
+        </property>
+
+        <property name="IncomingCallSound" type="s" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue.String" 
+                value="/usr/share/sounds/ubuntu/ringtones/Ubuntu.ogg"/>
+        </property>
+
+        <property name="IncomingMessageSound" type="s" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue.String" 
+                value="/usr/share/sounds/ubuntu/notifications/Xylo.ogg"/>
+        </property>
+
+        <property name="IncomingCallVibrate" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
+        </property>
+
+        <property name="IncomingCallVibrateSilentMode" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
+        </property>
+
+        <property name="IncomingMessageVibrate" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
+        </property>
+
+        <property name="IncomingMessageVibrateSilentMode" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
+        </property>
+
+        <property name="DialpadSoundsEnabled" type="b" access="readwrite">
+            <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
+        </property>
+
+    </interface>
+</node>

=== modified file 'src/snap.cpp'
--- src/snap.cpp	2014-12-08 02:48:13 +0000
+++ src/snap.cpp	2015-01-22 01:59:13 +0000
@@ -17,6 +17,8 @@
  *   Charles Kerr <[e-mail]>
  */
 
+#include "dbus-accounts-sound.h"
+
 #include <datetime/snap.h>
 #include <datetime/utils.h> // is_locale_12h()
 
@@ -32,6 +34,9 @@
 #include <set>
 #include <string>
 
+#include <unistd.h> // getuid()
+#include <sys/types.h> // getuid()
+
 namespace uin = unity::indicator::notifications;
 
 namespace unity {
@@ -49,12 +54,26 @@
     Impl(const std::shared_ptr<unity::indicator::notifications::Engine>& engine,
          const std::shared_ptr<const Settings>& settings):
       m_engine(engine),
-      m_settings(settings)
+      m_settings(settings),
+      m_cancellable(g_cancellable_new())
     {
+        auto object_path = g_strdup_printf("/org/freedesktop/Accounts/User%lu", (gulong)getuid());
+        accounts_service_sound_proxy_new_for_bus(G_BUS_TYPE_SYSTEM,
+                                                 G_DBUS_PROXY_FLAGS_GET_INVALIDATED_PROPERTIES,
+                                                 "org.freedesktop.Accounts",
+                                                 object_path,
+                                                 m_cancellable,
+                                                 on_sound_proxy_ready,
+                                                 this);
+        g_free(object_path);
     }
 
     ~Impl()
     {
+        g_cancellable_cancel(m_cancellable);
+        g_clear_object(&m_cancellable);
+        g_clear_object(&m_accounts_service_sound_proxy);
+
         for (const auto& key : m_notifications)
             m_engine->close (key);
     }
@@ -72,11 +91,16 @@
         // force the system to stay awake
         auto awake = std::make_shared<uin::Awake>(m_engine->app_name());
 
-        // create the sound...
-        const auto uri = get_alarm_uri(appointment, m_settings);
-        const auto volume = m_settings->alarm_volume.get();
-        const bool loop = interactive;
-        auto sound = std::make_shared<uin::Sound>(uri, volume, loop);
+        // calendar events are muted in silent mode; alarm clocks never are
+        std::shared_ptr<uin::Sound> sound;
+        if (appointment.is_ubuntu_alarm() || !silent_mode()) {
+            // create the sound.
+            const auto role = appointment.is_ubuntu_alarm() ? "alarm" : "alert";
+            const auto uri = get_alarm_uri(appointment, m_settings);
+            const auto volume = m_settings->alarm_volume.get();
+            const bool loop = interactive;
+            sound = std::make_shared<uin::Sound>(role, uri, volume, loop);
+        }
 
         // create the haptic feedback...
         const auto haptic_mode = m_settings->alarm_haptic.get();
@@ -131,6 +155,31 @@
 
 private:
 
+    static void on_sound_proxy_ready(GObject* /*source_object*/, GAsyncResult* res, gpointer gself)
+    {
+        GError * error;
+
+        error = nullptr;
+        auto proxy = accounts_service_sound_proxy_new_for_bus_finish (res, &error);
+        if (error != nullptr)
+        {
+            if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+                g_warning("%s Couldn't find accounts service sound proxy: %s", G_STRLOC, error->message);
+
+            g_clear_error(&error);
+        }
+        else
+        {
+            static_cast<Impl*>(gself)->m_accounts_service_sound_proxy = proxy;
+        }
+    }
+
+    bool silent_mode() const
+    {
+        return (m_accounts_service_sound_proxy != nullptr)
+            && (accounts_service_sound_get_silent_mode(m_accounts_service_sound_proxy));
+    }
+
     std::string get_alarm_uri(const Appointment& appointment,
                               const std::shared_ptr<const Settings>& settings) const
     {
@@ -167,6 +216,8 @@
     const std::shared_ptr<unity::indicator::notifications::Engine> m_engine;
     const std::shared_ptr<const Settings> m_settings;
     std::set<int> m_notifications;
+    GCancellable * m_cancellable {nullptr};
+    AccountsServiceSound * m_accounts_service_sound_proxy {nullptr};
 };
 
 /***

=== modified file 'src/sound.cpp'
--- src/sound.cpp	2014-09-17 16:51:51 +0000
+++ src/sound.cpp	2015-01-22 01:59:13 +0000
@@ -38,9 +38,11 @@
 {
 public:
 
-    Impl(const std::string& uri,
+    Impl(const std::string& role,
+         const std::string& uri,
          unsigned int volume,
          bool loop):
+        m_role(role),
         m_uri(uri),
         m_volume(volume),
         m_loop(loop)
@@ -98,8 +100,9 @@
     static gboolean bus_callback(GstBus*, GstMessage* msg, gpointer gself)
     {
         auto self = static_cast<Impl*>(gself);
+        const auto message_type = GST_MESSAGE_TYPE(msg);
 
-        if ((GST_MESSAGE_TYPE(msg) == GST_MESSAGE_EOS) && (self->m_loop))
+        if ((message_type == GST_MESSAGE_EOS) && (self->m_loop))
         {
             gst_element_seek(self->m_play,
                              1.0,
@@ -110,7 +113,7 @@
                              GST_SEEK_TYPE_NONE,
                              (gint64)GST_CLOCK_TIME_NONE);
         }
-        else if ((GST_MESSAGE_TYPE(msg) == GST_MESSAGE_STREAM_START) && (self->m_loop))
+        else if (message_type == GST_MESSAGE_STREAM_START)
         {
             /* Set the media role if audio sink is pulsesink */
             GstElement *audio_sink = nullptr;
@@ -121,10 +124,11 @@
                 feature = GST_PLUGIN_FEATURE_CAST(GST_ELEMENT_GET_CLASS(audio_sink)->elementfactory);
                 if (feature && g_strcmp0(gst_plugin_feature_get_name(feature), "pulsesink") == 0)
                 {
-                    std::string role_str("props,media.role=alarm");
-                    GstStructure *props = gst_structure_from_string(role_str.c_str(), nullptr);
+                    auto role_str = g_strdup_printf("props,media.role=%s", self->m_role.c_str());
+                    GstStructure *props = gst_structure_from_string(role_str, nullptr);
                     g_object_set(audio_sink, "stream-properties", props, nullptr);
                     gst_structure_free(props);
+                    g_free(role_str);
                 }
                 gst_object_unref(audio_sink);
             }
@@ -137,6 +141,7 @@
     ****
     ***/
 
+    const std::string m_role;
     const std::string m_uri;
     const unsigned int m_volume;
     const bool m_loop;
@@ -144,8 +149,8 @@
     GstElement* m_play = nullptr;
 };
 
-Sound::Sound(const std::string& uri, unsigned int volume, bool loop):
-  impl (new Impl(uri, volume, loop))
+Sound::Sound(const std::string& role, const std::string& uri, unsigned int volume, bool loop):
+  impl (new Impl(role, uri, volume, loop))
 {
 }
 

=== modified file 'tests/manual'
--- tests/manual	2014-12-08 02:52:50 +0000
+++ tests/manual	2015-01-22 01:59:13 +0000
@@ -118,6 +118,19 @@
 		<dd>The newly-selected sound should play, rather than the previous sound.</dd>
 </dl>
 
+Test-case indicator-datetime/silent-mode
+<dl>
+	<dt>Set an alarm and wait for it to arrive.</dt>
+	<dt>From the sound indicator, turn on silent mode.</dt>
+		<dd>Alarm should go off at the specified time and play its sound regardless of silent mode.</dd>
+	<dt>From the sound indicator, turn on silent mode.</dt>
+	<dt>Create a calendar event from the calendar app and wait for it to arrive.</dt>
+		<dd>The calendar event notification should be silent.</dd>
+	<dt>From the sound indicator, turn off silent mode.</dt>
+	<dt>Create a calendar event from the calendar app and wait for it to arrive.</dt>
+		<dd>The calendar event notification should play a sound.</dd>
+</dl>
+
 <strong>
     If all actions produce the expected results listed, please <a href="results#add_result">submit</a> a 'passed' result.
     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>.