Merge lp:~charlesk/indicator-datetime/lp-1358610-honor-other-vibrations-setting into lp:indicator-datetime/15.04

[Charles Kerr]

Honor the 'other vibrations' setting to enable/disable vibrations when alarm notifications are shown.

== Description of the Change

A pretty straightforward patch this time:

1. Don't vibrate if the AccountsService "Other Vibrations" bool property is false

2. In test-snap, add an AccountsService dbus mock to cover the new code.

== Checklist

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

No

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

mako vivid r165

> What manual tests are relevant for this MP?

indicator-datetime/other-vibrations

> 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

[PS Jenkins bot (ps-jenkins)]

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

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

review: Approve (continuous-integration)

[David Barth (dbarth)]

Charles, can you clarify what you need the AccountService for, and the library you use to access it? It seems that this is creating an issue further down the line with apparmor. See https://bugs.launchpad.net/ubuntu/+source/apparmor-easyprof-ubuntu/+bug/1433590 for reference.

[Charles Kerr (charlesk)]

David, sure, indicator-datetime is connecting to the com.ubuntu.touch.AccountsService.Sound interface in order to watch two settings that are published there:

(1) Silent Mode, so that we know whether or not to disable audio on alarms (this has been landed for some time now)

(2) Other Vibrations, so that we know whether or not to vibrate on alarms (this is new in this MP)

[Charles Kerr (charlesk)]

See comment in https://bugs.launchpad.net/ubuntu/+source/apparmor-easyprof-ubuntu/+bug/1433590/comments/7

[dobey (dobey)]

Looks ok to me.

review: Approve

=== modified file 'src/com.ubuntu.touch.AccountsService.Sound.xml'
--- src/com.ubuntu.touch.AccountsService.Sound.xml	2015-01-21 20:25:52 +0000
+++ src/com.ubuntu.touch.AccountsService.Sound.xml	2015-04-09 00:42:59 +0000
@@ -34,6 +34,11 @@
             <annotation name="org.freedesktop.Accounts.DefaultValue" value="true"/>
         </property>
 
+        <!-- "Other vibrations" should cover all vibrations except for those relating to phone calls and messages -->
+        <property name="OtherVibrate" 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>

=== modified file 'src/snap.cpp'
--- src/snap.cpp	2015-04-03 18:21:44 +0000
+++ src/snap.cpp	2015-04-09 00:42:59 +0000
@@ -104,10 +104,12 @@
         }
 
         // create the haptic feedback...
-        const auto haptic_mode = m_settings->alarm_haptic.get();
         std::shared_ptr<uin::Haptic> haptic;
-        if (haptic_mode == "pulse")
-            haptic = std::make_shared<uin::Haptic>(uin::Haptic::MODE_PULSE);
+        if (should_vibrate()) {
+            const auto haptic_mode = m_settings->alarm_haptic.get();
+            if (haptic_mode == "pulse")
+                haptic = std::make_shared<uin::Haptic>(uin::Haptic::MODE_PULSE);
+        }
 
         // show a notification...
         const auto minutes = std::chrono::minutes(m_settings->alarm_duration.get());
@@ -181,6 +183,12 @@
             && (accounts_service_sound_get_silent_mode(m_accounts_service_sound_proxy));
     }
 
+    bool should_vibrate() const
+    {
+        return (m_accounts_service_sound_proxy != nullptr)
+            && (accounts_service_sound_get_other_vibrate(m_accounts_service_sound_proxy));
+    }
+
     std::string get_alarm_uri(const Alarm& alarm,
                               const std::shared_ptr<const Settings>& settings) const
     {

=== modified file 'tests/test-snap.cpp'
--- tests/test-snap.cpp	2015-04-05 22:27:52 +0000
+++ tests/test-snap.cpp	2015-04-09 00:42:59 +0000
@@ -29,6 +29,9 @@
 
 #include <glib.h>
 
+#include <unistd.h> // getuid()
+#include <sys/types.h> // getuid()
+
 using namespace unity::indicator::datetime;
 
 #include "glib-fixture.h"
@@ -83,14 +86,21 @@
 
   static constexpr char const * HINT_TIMEOUT {"x-canonical-snap-decisions-timeout"};
 
+  static constexpr char const * AS_BUSNAME            {"org.freedesktop.Accounts"};
+  static constexpr char const * AS_INTERFACE          {"com.ubuntu.touch.AccountsService.Sound"};
+  static constexpr char const * PROP_OTHER_VIBRATIONS {"OtherVibrate"};
+  static constexpr char const * PROP_SILENT_MODE      {"SilentMode"};
+
   Appointment appt;
   GDBusConnection * system_bus = nullptr;
   GDBusConnection * session_bus = nullptr;
   DbusTestService * service = nullptr;
+  DbusTestDbusMock * as_mock = nullptr;
   DbusTestDbusMock * notify_mock = nullptr;
   DbusTestDbusMock * powerd_mock = nullptr;
   DbusTestDbusMock * screen_mock = nullptr;
   DbusTestDbusMock * haptic_mock = nullptr;
+  DbusTestDbusMockObject * as_obj = nullptr;
   DbusTestDbusMockObject * notify_obj = nullptr;
   DbusTestDbusMockObject * powerd_obj = nullptr;
   DbusTestDbusMockObject * screen_obj = nullptr;
@@ -116,6 +126,38 @@
     service = dbus_test_service_new(nullptr);
 
     ///
+    ///  Add the AccountsService mock
+    ///
+
+    as_mock = dbus_test_dbus_mock_new(AS_BUSNAME);
+    auto as_path = g_strdup_printf("/org/freedesktop/Accounts/User%lu", (gulong)getuid());
+    as_obj = dbus_test_dbus_mock_get_object(as_mock,
+                                            as_path,
+                                            AS_INTERFACE,
+                                            &error);
+    g_free(as_path);
+    g_assert_no_error(error);
+
+    // PROP_SILENT_MODE
+    dbus_test_dbus_mock_object_add_property(as_mock,
+                                            as_obj,
+                                            PROP_SILENT_MODE,
+                                            G_VARIANT_TYPE_BOOLEAN,
+                                            g_variant_new_boolean(false),
+                                            &error);
+    g_assert_no_error(error);
+
+    // PROP_OTHER_VIBRATIONS
+    dbus_test_dbus_mock_object_add_property(as_mock,
+                                            as_obj,
+                                            PROP_OTHER_VIBRATIONS,
+                                            G_VARIANT_TYPE_BOOLEAN,
+                                            g_variant_new_boolean(true),
+                                            &error);
+    g_assert_no_error(error);
+    dbus_test_service_add_task(service, DBUS_TEST_TASK(as_mock));
+
+    ///
     ///  Add the Notifications mock
     ///
 
@@ -283,6 +325,7 @@
     g_clear_object(&screen_mock);
     g_clear_object(&powerd_mock);
     g_clear_object(&notify_mock);
+    g_clear_object(&as_mock);
     g_clear_object(&service);
     g_object_unref(session_bus);
     g_object_unref(system_bus);
@@ -480,40 +523,50 @@
 ****
 ***/
 
-TEST_F(SnapFixture, HapticModes)
+TEST_F(SnapFixture,Vibrate)
 {
   auto settings = std::make_shared<Settings>();
   auto ne = std::make_shared<unity::indicator::notifications::Engine>(APP_NAME);
   auto func = [this](const Appointment&, const Alarm&){g_idle_add(quit_idle, loop);};
   GError * error = nullptr;
 
-  // invoke a snap decision while haptic feedback is set to "pulse",
-  // confirm that VibratePattern got called
-  settings->alarm_haptic.set("pulse");
-  auto snap = new Snap (ne, settings);
-  (*snap)(appt, appt.alarms.front(), func, func);
-  wait_msec(100);
-  EXPECT_TRUE (dbus_test_dbus_mock_object_check_method_call (haptic_mock,
-                                                             haptic_obj,
-                                                             HAPTIC_METHOD_VIBRATE_PATTERN,
-                                                             nullptr,
-                                                             &error));
-  delete snap;
-
-  // invoke a snap decision while haptic feedback is set to "none",
-  // confirm that VibratePattern =didn't= get called
-  wait_msec(100);
-  dbus_test_dbus_mock_object_clear_method_calls (haptic_mock, haptic_obj, &error);
-  settings->alarm_haptic.set("none");
-  snap = new Snap (ne, settings);
-  (*snap)(appt, appt.alarms.front(), func, func);
-  wait_msec(100);
-  EXPECT_FALSE (dbus_test_dbus_mock_object_check_method_call (haptic_mock,
-                                                              haptic_obj,
-                                                              HAPTIC_METHOD_VIBRATE_PATTERN,
-                                                              nullptr,
-                                                              &error));
-  delete snap;
-
-  g_assert_no_error (error);
+  struct {
+      bool other_vibrations; // the com.ubuntu.touch.AccountsService.Sound "other vibrations" setting
+      const char* haptic_mode;  // supported values: "none", "pulse"
+      bool expected_vibrate_called; // do we expect the phone to vibrate?
+  } test_cases[] = {
+      { false, "none",  false },
+      { true,  "none",  false },
+      { false, "pulse", false },
+      { true,  "pulse", true  }
+  };
+
+  auto snap = std::make_shared<Snap>(ne, settings);
+
+  for(const auto& test_case : test_cases)
+  {
+    // clear out any previous iterations' noise
+    dbus_test_dbus_mock_object_clear_method_calls(haptic_mock, haptic_obj, &error);
+
+    // set the properties to match the test case
+    settings->alarm_haptic.set(test_case.haptic_mode);
+    dbus_test_dbus_mock_object_update_property(as_mock,
+                                               as_obj,
+                                               PROP_OTHER_VIBRATIONS,
+                                               g_variant_new_boolean(test_case.other_vibrations),
+                                               &error);
+    g_assert_no_error(error);
+    wait_msec(100);
+
+    // run the test
+    (*snap)(appt, appt.alarms.front(), func, func);
+    wait_msec(100);
+    const bool vibrate_called = dbus_test_dbus_mock_object_check_method_call(haptic_mock,
+                                                                             haptic_obj,
+                                                                             HAPTIC_METHOD_VIBRATE_PATTERN,
+                                                                             nullptr,
+                                                                             &error);
+    g_assert_no_error(error);
+    EXPECT_EQ(test_case.expected_vibrate_called, vibrate_called);
+  }
 }