From 6f1c3f7f25052c1f4c001e30bc8368359531af87 Mon Sep 17 00:00:00 2001 From: Ratchanan Srirattanamet Date: Thu, 7 Dec 2023 02:20:08 +0700 Subject: Bring back Lomiri-specific notification hints As discussed in [1], Lomiri has a custom logic in notification timeout and require the custom hint for timeout to work correctly. Restore the hints from commit 0a88a8d7 ("Remove osd-notify remnants and use native notification timeout"), while keeping the standard timeout in place. Lomiri will ignore standard timeout, while other DE's will ignore the extra hint. [1] https://gitlab.com/ubports/development/core/content-hub/-/merge_requests/32#note_1552217874 --- src/notifications.cpp | 6 ++++++ tests/notification-fixture.h | 2 ++ tests/test-sound.cpp | 10 ++++++++++ 3 files changed, 18 insertions(+) diff --git a/src/notifications.cpp b/src/notifications.cpp index b962e68..a2a1fc3 100644 --- a/src/notifications.cpp +++ b/src/notifications.cpp @@ -249,6 +249,10 @@ public: const auto& d= info.m_duration; auto ms = std::chrono::duration_cast(d); notify_notification_set_timeout (nn.get (), ms.count ()); + // Lomiri has its own logic regarding timeout. + notify_notification_set_hint (nn.get(), + HINT_LOMIRI_TIMEOUT, + g_variant_new_int32(ms.count())); } for (const auto& hint : info.m_string_hints) @@ -509,6 +513,8 @@ private: // server capabilities. // as the name indicates, don't use this directly: use server_caps() instead mutable std::set m_lazy_caps; + + static constexpr char const * HINT_LOMIRI_TIMEOUT {"x-lomiri-snap-decisions-timeout"}; }; /*** diff --git a/tests/notification-fixture.h b/tests/notification-fixture.h index 33bcd7e..7dcc8bf 100644 --- a/tests/notification-fixture.h +++ b/tests/notification-fixture.h @@ -77,6 +77,8 @@ protected: static constexpr char const * SIGNAL_CLOSED {"NotificationClosed"}; + static constexpr char const * HINT_LOMIRI_TIMEOUT {"x-lomiri-snap-decisions-timeout"}; + static constexpr char const * AS_BUSNAME {"org.freedesktop.Accounts"}; static constexpr char const * AS_INTERFACE {"com.lomiri.touch.AccountsService.Sound"}; static constexpr char const * PROP_OTHER_VIBRATIONS {"OtherVibrate"}; diff --git a/tests/test-sound.cpp b/tests/test-sound.cpp index 19dffe3..db1402a 100644 --- a/tests/test-sound.cpp +++ b/tests/test-sound.cpp @@ -100,6 +100,16 @@ TEST_F(NotificationFixture, InteractiveDuration) g_variant_get_child (params, 7, "i", &i32); const auto duration = std::chrono::minutes(duration_minutes); EXPECT_EQ(std::chrono::duration_cast(duration).count(), i32); + + // Due to custom logic in Lomiri, also make sure custom timeout hint is set. + bool b; + auto hints = g_variant_get_child_value (params, 6); + i32 = 0; + b = g_variant_lookup (hints, HINT_LOMIRI_TIMEOUT, "i", &i32); + EXPECT_TRUE(b); + EXPECT_EQ(std::chrono::duration_cast(duration).count(), i32); + g_variant_unref(hints); + ne.reset(); } -- cgit v1.2.3 From 30b2de458752ad0855b508eb2f8ffeee85628cea Mon Sep 17 00:00:00 2001 From: Ratchanan Srirattanamet Date: Thu, 21 Dec 2023 19:16:35 +0700 Subject: src, tests: enable notification code path without Lomiri deps With the recent change, the notification code path is no longer Lomiri (or Ubuntu Touch) specific. Theres still some if-def's in the code to avoid adding dependendies in non-Lomiri case. --- src/main.cpp | 6 ---- src/notifications.cpp | 21 ++++++++++--- src/snap.cpp | 19 ++++++++++-- tests/CMakeLists.txt | 22 +++++--------- tests/notification-fixture.h | 21 ++++++++++--- tests/test-notification-response.cpp | 2 +- tests/test-sound.cpp | 59 ++++++++++++++++++++++++++---------- 7 files changed, 103 insertions(+), 47 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 4590e84..29e4472 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -32,9 +32,7 @@ #include #include #include -#ifdef LOMIRI_FEATURES_ENABLED #include -#endif #include #include #include @@ -95,7 +93,6 @@ namespace return state; } -#ifdef LOMIRI_FEATURES_ENABLED std::shared_ptr create_simple_alarm_queue(const std::shared_ptr& clock, const std::shared_ptr& snooze_planner, const std::shared_ptr& engine, @@ -119,7 +116,6 @@ namespace auto wakeup_timer = std::make_shared(clock); return std::make_shared(clock, planner, wakeup_timer); } -#endif } int @@ -149,7 +145,6 @@ main(int /*argc*/, char** /*argv*/) auto actions = std::make_shared(state); MenuFactory factory(actions, state); -#ifdef LOMIRI_FEATURES_ENABLED // set up the snap decisions auto snooze_planner = std::make_shared(state->settings, state->clock); auto notification_engine = std::make_shared("ayatana-indicator-datetime-service"); @@ -173,7 +168,6 @@ main(int /*argc*/, char** /*argv*/) engine->disable_alarm(appointment); }; alarm_queue->alarm_reached().connect(on_alarm_reached); -#endif // create the menus std::vector> menus; diff --git a/src/notifications.cpp b/src/notifications.cpp index a2a1fc3..4049851 100644 --- a/src/notifications.cpp +++ b/src/notifications.cpp @@ -199,6 +199,13 @@ public: return server_caps().count("actions") != 0; } +#ifdef LOMIRI_FEATURES_ENABLED + bool requires_hint_lomiri_timeout() const + { + return server_caps().count(HINT_LOMIRI_TIMEOUT) != 0; + } +#endif + void close_all () { // call close() on all our keys @@ -249,10 +256,14 @@ public: const auto& d= info.m_duration; auto ms = std::chrono::duration_cast(d); notify_notification_set_timeout (nn.get (), ms.count ()); - // Lomiri has its own logic regarding timeout. - notify_notification_set_hint (nn.get(), - HINT_LOMIRI_TIMEOUT, - g_variant_new_int32(ms.count())); +#ifdef LOMIRI_FEATURES_ENABLED + if (requires_hint_lomiri_timeout()) { + // Lomiri has its own logic regarding timeout. + notify_notification_set_hint (nn.get(), + HINT_LOMIRI_TIMEOUT, + g_variant_new_int32(ms.count())); + } +#endif } for (const auto& hint : info.m_string_hints) @@ -514,7 +525,9 @@ private: // as the name indicates, don't use this directly: use server_caps() instead mutable std::set m_lazy_caps; +#ifdef LOMIRI_FEATURES_ENABLED static constexpr char const * HINT_LOMIRI_TIMEOUT {"x-lomiri-snap-decisions-timeout"}; +#endif }; /*** diff --git a/src/snap.cpp b/src/snap.cpp index 46f1d7b..c18f955 100644 --- a/src/snap.cpp +++ b/src/snap.cpp @@ -21,6 +21,7 @@ #ifdef LOMIRI_FEATURES_ENABLED #include "dbus-accounts-sound.h" +#endif #include #include // is_locale_12h() @@ -64,6 +65,7 @@ public: m_cancellable(g_cancellable_new()), m_system_bus{G_DBUS_CONNECTION(g_object_ref(system_bus))} { + #ifdef LOMIRI_FEATURES_ENABLED auto object_path = g_strdup_printf("/org/freedesktop/Accounts/User%lu", (gulong)getuid()); @@ -75,13 +77,16 @@ public: on_sound_proxy_ready, this); g_free(object_path); + #endif } ~Impl() { g_cancellable_cancel(m_cancellable); g_clear_object(&m_cancellable); + #ifdef LOMIRI_FEATURES_ENABLED g_clear_object(&m_accounts_service_sound_proxy); + #endif g_clear_object(&m_system_bus); for (const auto& key : m_notifications) @@ -235,6 +240,7 @@ private: return m_settings->vibrate_silent_mode.get(); } +#ifdef LOMIRI_FEATURES_ENABLED static void on_sound_proxy_ready(GObject* /*source_object*/, GAsyncResult* res, gpointer gself) { GError * error; @@ -253,17 +259,26 @@ private: static_cast(gself)->m_accounts_service_sound_proxy = proxy; } } +#endif bool silent_mode() const { +#ifdef LOMIRI_FEATURES_ENABLED return (m_accounts_service_sound_proxy != nullptr) && (accounts_service_sound_get_silent_mode(m_accounts_service_sound_proxy)); +#else + return false; +#endif } bool should_vibrate() const { +#ifdef LOMIRI_FEATURES_ENABLED return (m_accounts_service_sound_proxy != nullptr) && (accounts_service_sound_get_other_vibrate(m_accounts_service_sound_proxy)); +#else + return true; +#endif } std::string get_alarm_uri(const Appointment& appointment, @@ -306,7 +321,9 @@ private: const std::shared_ptr m_settings; std::set m_notifications; GCancellable * m_cancellable {nullptr}; +#ifdef LOMIRI_FEATURES_ENABLED AccountsServiceSound * m_accounts_service_sound_proxy {nullptr}; +#endif GDBusConnection * m_system_bus {nullptr}; static constexpr char const * ACTION_NONE {"none"}; @@ -345,5 +362,3 @@ Snap::operator()(const Appointment& appointment, } // namespace datetime } // namespace indicator } // namespace ayatana - -#endif diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a0687f4..60e7864 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -42,13 +42,9 @@ function(add_test_by_name name) target_link_libraries (${TEST_NAME} indicatordatetimeservice ${DBUSTEST_LIBRARIES} ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES}) endfunction() add_test_by_name(test-datetime) - -if (ENABLE_LOMIRI_FEATURES) - add_test_by_name(test-sound) - add_test_by_name(test-notification) - add_test_by_name(test-notification-response) -endif() - +add_test_by_name(test-sound) +add_test_by_name(test-notification) +add_test_by_name(test-notification-response) add_test_by_name(test-actions) add_test_by_name(test-alarm-queue) add_test(NAME dear-reader-the-next-test-takes-60-seconds COMMAND true) @@ -64,13 +60,11 @@ add_test_by_name(test-settings) add_test_by_name(test-timezone-timedated) add_test_by_name(test-utils) -if (ENABLE_LOMIRI_FEATURES) - set (TEST_NAME manual-test-snap) - set (COVERAGE_TEST_TARGETS ${COVERAGE_TEST_TARGETS} ${TEST_NAME}) - add_executable (${TEST_NAME} ${TEST_NAME}.cpp) - target_link_options(${TEST_NAME} PRIVATE -no-pie) - target_link_libraries (${TEST_NAME} indicatordatetimeservice ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES}) -endif() +set (TEST_NAME manual-test-snap) +set (COVERAGE_TEST_TARGETS ${COVERAGE_TEST_TARGETS} ${TEST_NAME}) +add_executable (${TEST_NAME} ${TEST_NAME}.cpp) +target_link_options(${TEST_NAME} PRIVATE -no-pie) +target_link_libraries (${TEST_NAME} indicatordatetimeservice ${SERVICE_DEPS_LIBRARIES} ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES}) ## ## EDS Tests diff --git a/tests/notification-fixture.h b/tests/notification-fixture.h index 7dcc8bf..5aedadc 100644 --- a/tests/notification-fixture.h +++ b/tests/notification-fixture.h @@ -77,7 +77,9 @@ protected: static constexpr char const * SIGNAL_CLOSED {"NotificationClosed"}; +#ifdef LOMIRI_FEATURES_ENABLED static constexpr char const * HINT_LOMIRI_TIMEOUT {"x-lomiri-snap-decisions-timeout"}; +#endif static constexpr char const * AS_BUSNAME {"org.freedesktop.Accounts"}; static constexpr char const * AS_INTERFACE {"com.lomiri.touch.AccountsService.Sound"}; @@ -312,20 +314,31 @@ protected: super::TearDown(); } - void make_interactive() + void mock_capabilities(bool mock_lomiri_caps = false) { // GetCapabilities returns an array containing 'actions', // so our notifications will be interactive. - // For this test, it means we should get an expire_timeout - // that matches duration_minutes + + #ifndef LOMIRI_FEATURES_ENABLED + g_assert_false(mock_lomiri_caps); + #endif + + std::string python_code = + std::string("ret = ['actions', 'body'") + + #ifdef LOMIRI_FEATURES_ENABLED + (mock_lomiri_caps ? std::string(", '") + HINT_LOMIRI_TIMEOUT + "'" : "") + + #endif + "]"; + GError * error = nullptr; dbus_test_dbus_mock_object_add_method(notify_mock, notify_obj, METHOD_GET_CAPS, nullptr, G_VARIANT_TYPE_STRING_ARRAY, - "ret = ['actions', 'body']", + python_code.c_str(), &error); + g_assert_no_error (error); } diff --git a/tests/test-notification-response.cpp b/tests/test-notification-response.cpp index fd40ed8..d3e9c00 100644 --- a/tests/test-notification-response.cpp +++ b/tests/test-notification-response.cpp @@ -47,7 +47,7 @@ namespace TEST_F(NotificationFixture,Response) { // create the world - make_interactive(); + mock_capabilities(); auto ne = std::make_shared(APP_NAME); auto sb = std::make_shared(); auto settings = std::make_shared(); diff --git a/tests/test-sound.cpp b/tests/test-sound.cpp index db1402a..eaf76ac 100644 --- a/tests/test-sound.cpp +++ b/tests/test-sound.cpp @@ -46,13 +46,22 @@ namespace g_main_loop_quit(static_cast(gloop)); return G_SOURCE_REMOVE; }; + + class SoundNotificationFixture : public NotificationFixture, + public testing::WithParamInterface + { + public: + bool IsLomiri() { + return GetParam(); + } + }; } /*** **** ***/ -TEST_F(NotificationFixture, InteractiveDuration) +TEST_P(SoundNotificationFixture, InteractiveDuration) { static constexpr int duration_minutes = 120; auto settings = std::make_shared(); @@ -67,7 +76,7 @@ TEST_F(NotificationFixture, InteractiveDuration) settings->cal_notification_bubbles.set(true); settings->cal_notification_list.set(true); - make_interactive(); + mock_capabilities(IsLomiri()); // call the Snap Decision auto func = [this](const Appointment&, const Alarm&, const Snap::Response&){g_idle_add(quit_idle, loop);}; @@ -101,18 +110,31 @@ TEST_F(NotificationFixture, InteractiveDuration) const auto duration = std::chrono::minutes(duration_minutes); EXPECT_EQ(std::chrono::duration_cast(duration).count(), i32); - // Due to custom logic in Lomiri, also make sure custom timeout hint is set. - bool b; - auto hints = g_variant_get_child_value (params, 6); - i32 = 0; - b = g_variant_lookup (hints, HINT_LOMIRI_TIMEOUT, "i", &i32); - EXPECT_TRUE(b); - EXPECT_EQ(std::chrono::duration_cast(duration).count(), i32); - g_variant_unref(hints); +#ifdef LOMIRI_FEATURES_ENABLED + if (IsLomiri()) { + // Due to custom logic in Lomiri, also make sure custom timeout hint is set. + bool b; + auto hints = g_variant_get_child_value (params, 6); + i32 = 0; + b = g_variant_lookup (hints, HINT_LOMIRI_TIMEOUT, "i", &i32); + EXPECT_TRUE(b); + EXPECT_EQ(std::chrono::duration_cast(duration).count(), i32); + g_variant_unref(hints); + } +#endif ne.reset(); } +INSTANTIATE_TEST_SUITE_P(SoundNotificationTest, + SoundNotificationFixture, + testing::Values( +#ifdef LOMIRI_FEATURES_ENABLED + true, +#endif + false + )); + /*** **** ***/ @@ -147,12 +169,17 @@ private: uin::DefaultSoundBuilder m_impl; }; -std::string path_to_uri(const std::string& path) +std::string path_to_uri_if_exists(const std::string& path) { + std::string uri; auto file = g_file_new_for_path(path.c_str()); - auto uri_cstr = g_file_get_uri(file); - std::string uri = uri_cstr; - g_free(uri_cstr); + + if (g_file_query_exists(file, /* cancellable */ nullptr)) { + auto uri_cstr = g_file_get_uri(file); + uri = std::string(uri_cstr); + g_free(uri_cstr); + } + g_clear_pointer(&file, g_object_unref); return uri; } @@ -175,9 +202,9 @@ TEST_F(NotificationFixture,DefaultSounds) std::string expected_role; std::string expected_uri; } test_cases[] = { - { ualarm, "alarm", path_to_uri(ALARM_DEFAULT_SOUND) } + { ualarm, "alarm", path_to_uri_if_exists(ALARM_DEFAULT_SOUND) } // No sound for appointments - // { appt, "alert", path_to_uri(CALENDAR_DEFAULT_SOUND) } + // { appt, "alert", path_to_uri_if_exists(CALENDAR_DEFAULT_SOUND) } }; auto snap = create_snap(ne, sb, settings); -- cgit v1.2.3