From 63820ffd5bc9d51fe55ab4ad48bfc6270c919037 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Sun, 29 Sep 2024 10:56:40 +0200 Subject: [PATCH 1/7] notify-feedback: Send event for new notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The feedback theme should decide what should happen on notifications, not the shell. Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index d45063bb5..be73ba310 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -70,18 +70,21 @@ find_event (const char *category) const char *ret = NULL; if (inactive) { - if (g_strcmp0 (category, "email.arrived") == 0) + if (g_strcmp0 (category, "email.arrived") == 0) { ret = "message-missed-email"; - else if (g_strcmp0 (category, "im.received") == 0) + } else if (g_strcmp0 (category, "im.received") == 0) { ret = "message-missed-instant"; - else + } else { + /* TODO: notification-missed-generic */ ret = "message-missed-notification"; + } } else { if (g_strcmp0 (category, "email.arrived") == 0) ret = "message-new-email"; else if (g_strcmp0 (category, "im.received") == 0) ret = "message-new-instant"; - /* no feedback when not locked as to not distract the user */ + else + ret = "notification-new-generic"; } if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) -- GitLab From 603fe6d11badd09ed812593ff8a6e670b9914d20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Sun, 29 Sep 2024 14:36:02 +0200 Subject: [PATCH 2/7] notify-feedback: Make condition match maybe_trigger_feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The early exit should use the same criteria we use in `maybe_trigger_feedback` later on (as it's only purpose is to shortcut the logic early. Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index be73ba310..f2eeee700 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -238,7 +238,7 @@ on_shell_state_changed (PhoshNotifyFeedback *self, GParamSpec *pspec, PhoshShell if (self->event && lfb_event_get_state (self->event) == LFB_EVENT_STATE_RUNNING) return; - if (!phosh_shell_get_blanked (shell)) + if (!phosh_shell_get_blanked (shell) && !phosh_shell_get_locked (shell)) return; for (guint i = 0; i < g_list_model_get_n_items (G_LIST_MODEL (self->list)); i++) { -- GitLab From acbec93d055860f224e71dac163df5b668ae3b52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Sun, 29 Sep 2024 11:21:37 +0200 Subject: [PATCH 3/7] notify-feedback: Emit proper feedback when device is locked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So far we only picked the `*-missed-*` events and relied on the app to submit proper feedback. That isn't very reasonable though as the app e.g. won't know if the user has disabled notifications. So emit feedback for all notifications with reasonable categories. Closes: https://gitlab.gnome.org/World/Phosh/phosh/-/issues/728 Closes: https://source.puri.sm/Librem5/feedbackd/-/merge_requests/69 Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 153 ++++++++++++++++++++-------- 1 file changed, 113 insertions(+), 40 deletions(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index f2eeee700..c5d476878 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -61,33 +61,55 @@ end_notify_feedback (PhoshNotifyFeedback *self) lfb_event_end_feedback_async (self->event, NULL, NULL, NULL); } - +/** + * find_event_inactive: + * @category: The category to look up the event for + * + * Look up an event when for a notification category when the device + * is not in active use. + * + * Returns:(nullable): The event name + */ static const char * -find_event (const char *category) +find_event_inactive (const char *category) { - PhoshShell *shell = phosh_shell_get_default (); - gboolean inactive = phosh_shell_get_blanked (shell) || phosh_shell_get_locked (shell); const char *ret = NULL; - if (inactive) { - if (g_strcmp0 (category, "email.arrived") == 0) { - ret = "message-missed-email"; - } else if (g_strcmp0 (category, "im.received") == 0) { - ret = "message-missed-instant"; - } else { - /* TODO: notification-missed-generic */ - ret = "message-missed-notification"; - } + if (g_strcmp0 (category, "email.arrived") == 0) { + ret = "message-missed-email"; + } else if (g_strcmp0 (category, "im.received") == 0) { + ret = "message-missed-instant"; + } else if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) { + ret = "phone-missed-call"; + } else if (g_strcmp0 (category, "call.unanswered") == 0) { + ret = "phone-missed-call"; } else { - if (g_strcmp0 (category, "email.arrived") == 0) - ret = "message-new-email"; - else if (g_strcmp0 (category, "im.received") == 0) - ret = "message-new-instant"; - else - ret = "notification-new-generic"; + /* TODO: notification-missed-generic */ + ret = "message-missed-notification"; } - if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) + return ret; +} + +/** + * find_event_active: + * @category: The category to look up the event for + * + * Look up an event when for a notification category when the device + * is in active use. + * + * Returns:(nullable): The event name + */ +static const char * +find_event_active (const char *category) +{ + const char *ret = NULL; + + if (g_strcmp0 (category, "email.arrived") == 0) + ret = "message-new-email"; + else if (g_strcmp0 (category, "im.received") == 0) + ret = "message-new-instant"; + else if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) ret = "phone-missed-call"; else if (g_strcmp0 (category, "call.ended") == 0) ret = "phone-hangup"; @@ -95,6 +117,8 @@ find_event (const char *category) ret = "phone-incoming-call"; else if (g_strcmp0 (category, "call.unanswered") == 0) ret = "phone-missed-call"; + else + ret = "notification-new-generic"; return ret; } @@ -124,25 +148,41 @@ maybe_wakeup_screen (PhoshNotifyFeedback *self, PhoshNotificationSource *source, } } - +/** + * maybe_trigger_feedback: + * @self: The notification feedback handler + * @source: The notification source model + * @position: Position where notifications were added to the model + * @num: How many notifications were added to the model + * @inactive_only: Whether to only look for events used when the device + * is considered inactive + * + * Look up events matching the categories of newly added notifications + * and trigger feedback therefore. + * + * Returns: `TRUE` when any feedback was triggered + */ static gboolean -maybe_trigger_feedback (PhoshNotifyFeedback *self, PhoshNotificationSource *source, guint position, guint num) +maybe_trigger_feedback (PhoshNotifyFeedback *self, + PhoshNotificationSource *source, + guint position, + guint num, + gboolean inactive_only) { + PhoshShell *shell = phosh_shell_get_default (); + gboolean inactive = phosh_shell_get_blanked (shell) || phosh_shell_get_locked (shell); + + /* Get us the first notification that triggers meaningful feedback */ for (int i = 0; i < num; i++) { g_autoptr (PhoshNotification) noti = g_list_model_get_item (G_LIST_MODEL (source), position + i); - g_autoptr (LfbEvent) event = NULL; - const char *category, *event_name, *profile; + const char *category, *profile; g_autofree char *app_id = NULL; GAppInfo *info = NULL; + gboolean ret = FALSE; g_return_val_if_fail (PHOSH_IS_NOTIFICATION (noti), FALSE); g_return_val_if_fail (lfb_is_initted (), FALSE); - category = phosh_notification_get_category (noti); - event_name = find_event (category); - if (event_name == NULL) - continue; - info = phosh_notification_get_app_info (noti); if (info) app_id = phosh_strip_suffix_from_app_id (g_app_info_get_id (info)); @@ -151,17 +191,47 @@ maybe_trigger_feedback (PhoshNotifyFeedback *self, PhoshNotificationSource *sour if (g_strcmp0 (profile, "none") == 0) continue; - g_debug ("Emitting event %s for %s, profile: %s", - event_name, app_id ?: "unknown", profile); - event = lfb_event_new (event_name); - lfb_event_set_feedback_profile (event, profile); - g_set_object (&self->event, event); + category = phosh_notification_get_category (noti); + + /* The default event */ + if (!inactive_only) { + const char *name; - if (app_id) - lfb_event_set_app_id (event, app_id); + name = find_event_active (category); + if (name) { + g_autoptr (LfbEvent) event = event = lfb_event_new (name); - lfb_event_trigger_feedback_async (self->event, NULL, NULL, NULL); - return TRUE; + lfb_event_set_feedback_profile (event, profile); + if (app_id) + lfb_event_set_app_id (event, app_id); + g_debug ("Emitting event %s for %s, profile: %s", name, app_id ?: "unknown", profile); + lfb_event_trigger_feedback_async (event, NULL, NULL, NULL); + ret = TRUE; + } + } + + /* The event when the device is not in active use (usually long running LED feedback) */ + if (inactive) { + const char *name; + + name = find_event_inactive (category); + if (name) { + g_autoptr (LfbEvent) event = event = lfb_event_new (name); + + lfb_event_set_feedback_profile (event, profile); + if (app_id) + lfb_event_set_app_id (event, app_id); + g_debug ("Emitting event %s for %s, profile: %s", name, app_id ?: "unknown", profile); + lfb_event_trigger_feedback_async (event, NULL, NULL, NULL); + + /* TODO: we should better track that on the notification */ + g_set_object (&self->event, event); + ret = TRUE; + } + } + + if (ret) + return ret; } return FALSE; @@ -184,7 +254,7 @@ on_notification_source_items_changed (PhoshNotifyFeedback *self, if (self->event && lfb_event_get_state (self->event) == LFB_EVENT_STATE_RUNNING) return; - maybe_trigger_feedback (self, PHOSH_NOTIFICATION_SOURCE (list), position, added); + maybe_trigger_feedback (self, PHOSH_NOTIFICATION_SOURCE (list), position, added, FALSE); } @@ -241,10 +311,13 @@ on_shell_state_changed (PhoshNotifyFeedback *self, GParamSpec *pspec, PhoshShell if (!phosh_shell_get_blanked (shell) && !phosh_shell_get_locked (shell)) return; + /* Ensure we have visual feedback when the device blanks with open notifications */ for (guint i = 0; i < g_list_model_get_n_items (G_LIST_MODEL (self->list)); i++) { + guint n_items; g_autoptr (PhoshNotificationSource) source = g_list_model_get_item (G_LIST_MODEL (self->list), i); - if (maybe_trigger_feedback (self, source, 0, g_list_model_get_n_items (G_LIST_MODEL (source)))) + n_items = g_list_model_get_n_items (G_LIST_MODEL (source)); + if (maybe_trigger_feedback (self, source, 0, n_items, TRUE)) break; } } -- GitLab From 980fd291b68a9d859bfc4f99eb5d5d921ea3f7f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Tue, 1 Oct 2024 13:57:27 +0200 Subject: [PATCH 4/7] notify-feedback: Allow apps to special case SMS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is useful for phones Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index c5d476878..0fc1899e4 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -79,6 +79,8 @@ find_event_inactive (const char *category) ret = "message-missed-email"; } else if (g_strcmp0 (category, "im.received") == 0) { ret = "message-missed-instant"; + } else if (g_strcmp0 (category, "x-phosh.sms.received") == 0) { + ret = "message-missed-sms"; } else if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) { ret = "phone-missed-call"; } else if (g_strcmp0 (category, "call.unanswered") == 0) { @@ -109,6 +111,8 @@ find_event_active (const char *category) ret = "message-new-email"; else if (g_strcmp0 (category, "im.received") == 0) ret = "message-new-instant"; + else if (g_strcmp0 (category, "x-phosh.sms.received") == 0) + ret = "message-new-sms"; else if (g_strcmp0 (category, "x-gnome.call.unanswered") == 0) ret = "phone-missed-call"; else if (g_strcmp0 (category, "call.ended") == 0) -- GitLab From 057a47d5fe3582442e4c9c076646959590004f1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Wed, 2 Oct 2024 20:37:22 +0200 Subject: [PATCH 5/7] notify-feedback: Clarify event purpose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We only track the events for the inactive case Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index 0fc1899e4..de72f702d 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -39,7 +39,7 @@ static GParamSpec *props[PROP_LAST_PROP]; struct _PhoshNotifyFeedback { GObject parent; - LfbEvent *event; + LfbEvent *inactive_event; PhoshNotificationList *list; GSettings *settings; @@ -54,11 +54,11 @@ end_notify_feedback (PhoshNotifyFeedback *self) { g_return_if_fail (lfb_is_initted ()); - if (self->event == NULL) + if (self->inactive_event == NULL) return; - if (lfb_event_get_state (self->event) == LFB_EVENT_STATE_RUNNING) - lfb_event_end_feedback_async (self->event, NULL, NULL, NULL); + if (lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) + lfb_event_end_feedback_async (self->inactive_event, NULL, NULL, NULL); } /** @@ -229,7 +229,7 @@ maybe_trigger_feedback (PhoshNotifyFeedback *self, lfb_event_trigger_feedback_async (event, NULL, NULL, NULL); /* TODO: we should better track that on the notification */ - g_set_object (&self->event, event); + g_set_object (&self->inactive_event, event); ret = TRUE; } } @@ -255,7 +255,7 @@ on_notification_source_items_changed (PhoshNotifyFeedback *self, maybe_wakeup_screen (self, PHOSH_NOTIFICATION_SOURCE (list), position, added); /* TODO: add pending events to queue instead of just skipping them. */ - if (self->event && lfb_event_get_state (self->event) == LFB_EVENT_STATE_RUNNING) + if (self->inactive_event && lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) return; maybe_trigger_feedback (self, PHOSH_NOTIFICATION_SOURCE (list), position, added, FALSE); @@ -309,7 +309,7 @@ on_shell_state_changed (PhoshNotifyFeedback *self, GParamSpec *pspec, PhoshShell g_return_if_fail (PHOSH_IS_NOTIFY_FEEDBACK (self)); /* Feedback ongoing, nothing to do */ - if (self->event && lfb_event_get_state (self->event) == LFB_EVENT_STATE_RUNNING) + if (self->inactive_event && lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) return; if (!phosh_shell_get_blanked (shell) && !phosh_shell_get_locked (shell)) @@ -392,9 +392,9 @@ phosh_notify_feedback_dispose (GObject *object) g_clear_object (&self->settings); - if (self->event) { + if (self->inactive_event) { end_notify_feedback (self); - g_clear_object (&self->event); + g_clear_object (&self->inactive_event); } g_signal_handlers_disconnect_by_data (self->list, self); -- GitLab From 082414a4e212c5d6c3d9dfc9c373e90039755f37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Wed, 2 Oct 2024 20:41:35 +0200 Subject: [PATCH 6/7] notify-feedback: Always trigger feedback when events are added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we handle inactive and active now we shouldn't silently ignore new notifications. Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index de72f702d..a8112dff0 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -254,10 +254,6 @@ on_notification_source_items_changed (PhoshNotifyFeedback *self, maybe_wakeup_screen (self, PHOSH_NOTIFICATION_SOURCE (list), position, added); - /* TODO: add pending events to queue instead of just skipping them. */ - if (self->inactive_event && lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) - return; - maybe_trigger_feedback (self, PHOSH_NOTIFICATION_SOURCE (list), position, added, FALSE); } -- GitLab From 3b5cf8934f4dc7a62ba263d21594a581c29972b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Wed, 2 Oct 2024 20:43:31 +0200 Subject: [PATCH 7/7] notify-feedback: Handle feedback for active case separately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures we don't forget to end feedback for long running feedback when the device is active (e.g. `call-unanswered`). Signed-off-by: Guido Günther Part-of: --- src/notifications/notify-feedback.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/notifications/notify-feedback.c b/src/notifications/notify-feedback.c index a8112dff0..29d4cc468 100644 --- a/src/notifications/notify-feedback.c +++ b/src/notifications/notify-feedback.c @@ -39,6 +39,7 @@ static GParamSpec *props[PROP_LAST_PROP]; struct _PhoshNotifyFeedback { GObject parent; + LfbEvent *active_event; LfbEvent *inactive_event; PhoshNotificationList *list; @@ -54,10 +55,10 @@ end_notify_feedback (PhoshNotifyFeedback *self) { g_return_if_fail (lfb_is_initted ()); - if (self->inactive_event == NULL) - return; + if (self->active_event && lfb_event_get_state (self->active_event) == LFB_EVENT_STATE_RUNNING) + lfb_event_end_feedback_async (self->active_event, NULL, NULL, NULL); - if (lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) + if (self->inactive_event && lfb_event_get_state (self->inactive_event) == LFB_EVENT_STATE_RUNNING) lfb_event_end_feedback_async (self->inactive_event, NULL, NULL, NULL); } @@ -210,6 +211,8 @@ maybe_trigger_feedback (PhoshNotifyFeedback *self, lfb_event_set_app_id (event, app_id); g_debug ("Emitting event %s for %s, profile: %s", name, app_id ?: "unknown", profile); lfb_event_trigger_feedback_async (event, NULL, NULL, NULL); + /* TODO: we should better track that on the notification */ + g_set_object (&self->active_event, event); ret = TRUE; } } @@ -227,7 +230,6 @@ maybe_trigger_feedback (PhoshNotifyFeedback *self, lfb_event_set_app_id (event, app_id); g_debug ("Emitting event %s for %s, profile: %s", name, app_id ?: "unknown", profile); lfb_event_trigger_feedback_async (event, NULL, NULL, NULL); - /* TODO: we should better track that on the notification */ g_set_object (&self->inactive_event, event); ret = TRUE; @@ -388,8 +390,9 @@ phosh_notify_feedback_dispose (GObject *object) g_clear_object (&self->settings); - if (self->inactive_event) { + if (self->inactive_event && self->active_event) { end_notify_feedback (self); + g_clear_object (&self->active_event); g_clear_object (&self->inactive_event); } -- GitLab