From 59f8e7241084ac7d74e3e6d5139d8d937bd8ae0d Mon Sep 17 00:00:00 2001 From: Andreas Shimokawa Date: Thu, 15 Nov 2018 15:53:04 +0100 Subject: [PATCH] Notification actions refatoring and fixes - Fixes "Mute, Open, Dismiss" to work again on pebble - Greatly reduces complexity in PebbleProtocol, since all logic for adding specific reply actions to notification have been moved to generic code Fixes the rest of #1336 (the part that says "Additionally, dismissing a notification on the watch no longer dismisses it on the Android device") --- .../externalevents/AlarmClockReceiver.java | 20 ++- .../externalevents/NotificationListener.java | 133 ++++++++++++------ .../externalevents/SMSReceiver.java | 12 +- .../gadgetbridge/model/NotificationSpec.java | 14 +- .../service/DeviceCommunicationService.java | 2 +- .../devices/pebble/PebbleProtocol.java | 83 ++++------- 6 files changed, 159 insertions(+), 105 deletions(-) diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/AlarmClockReceiver.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/AlarmClockReceiver.java index b92566fa3..9d22d3b66 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/AlarmClockReceiver.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/AlarmClockReceiver.java @@ -21,6 +21,8 @@ import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; +import java.util.ArrayList; + import nodomain.freeyourgadget.gadgetbridge.GBApplication; import nodomain.freeyourgadget.gadgetbridge.model.NotificationSpec; import nodomain.freeyourgadget.gadgetbridge.model.NotificationType; @@ -64,13 +66,21 @@ public class AlarmClockReceiver extends BroadcastReceiver { private synchronized void sendAlarm(boolean on) { dismissLastAlarm(); if (on) { - NotificationSpec spec = new NotificationSpec(); + NotificationSpec notificationSpec = new NotificationSpec(); //TODO: can we attach a dismiss action to the notification and not use the notification ID explicitly? - lastId = spec.getId(); - spec.type = NotificationType.GENERIC_ALARM_CLOCK; - spec.sourceName = "ALARMCLOCKRECEIVER"; + lastId = notificationSpec.getId(); + notificationSpec.type = NotificationType.GENERIC_ALARM_CLOCK; + notificationSpec.sourceName = "ALARMCLOCKRECEIVER"; + notificationSpec.attachedActions = new ArrayList<>(); + + // DISMISS ALL action + NotificationSpec.Action dismissAllAction = new NotificationSpec.Action(); + dismissAllAction.title = "Dismiss All"; + dismissAllAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_DISMISS_ALL; + notificationSpec.attachedActions.add(dismissAllAction); + // can we get the alarm title somehow? - GBApplication.deviceService().onNotification(spec); + GBApplication.deviceService().onNotification(notificationSpec); } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/NotificationListener.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/NotificationListener.java index d16b9b269..023541d5a 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/NotificationListener.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/NotificationListener.java @@ -86,7 +86,9 @@ public class NotificationListener extends NotificationListenerService { public static final String ACTION_REPLY = "nodomain.freeyourgadget.gadgetbridge.notificationlistener.action.reply"; - private LimitedQueue mActionLookup = new LimitedQueue(16); + private LimitedQueue mActionLookup = new LimitedQueue(32); + private LimitedQueue mPackageLookup = new LimitedQueue(64); + private LimitedQueue mNotificationHandleLookup = new LimitedQueue(128); private HashMap notificationBurstPrevention = new HashMap<>(); private HashMap notificationOldRepeatPrevention = new HashMap<>(); @@ -96,39 +98,58 @@ public class NotificationListener extends NotificationListenerService { @Override public void onReceive(Context context, Intent intent) { String action = intent.getAction(); + if (action == null) { + LOG.warn("no action"); + return; + } + + int handle = (int) intent.getLongExtra("handle", -1); switch (action) { case GBApplication.ACTION_QUIT: stopSelf(); break; - case ACTION_MUTE: + case ACTION_OPEN: { StatusBarNotification[] sbns = NotificationListener.this.getActiveNotifications(); - int handle = (int) intent.getLongExtra("handle", -1); + Long ts = (Long) mNotificationHandleLookup.lookup(handle); + if (ts == null) { + LOG.info("could not lookup handle for open action"); + break; + } + for (StatusBarNotification sbn : sbns) { - if ((int) sbn.getPostTime() == handle) { - if (action.equals(ACTION_OPEN)) { - try { - PendingIntent pi = sbn.getNotification().contentIntent; - if (pi != null) { - pi.send(); - } - } catch (PendingIntent.CanceledException e) { - e.printStackTrace(); + if (sbn.getPostTime() == ts) { + try { + PendingIntent pi = sbn.getNotification().contentIntent; + if (pi != null) { + pi.send(); } - } else { - // ACTION_MUTE - LOG.info("going to mute " + sbn.getPackageName()); - GBApplication.addAppToNotifBlacklist(sbn.getPackageName()); + } catch (PendingIntent.CanceledException e) { + e.printStackTrace(); } } } break; } + case ACTION_MUTE: + String packageName = (String) mPackageLookup.lookup(handle); + if (packageName == null) { + LOG.info("could not lookup handle for mute action"); + break; + } + + LOG.info("going to mute " + packageName); + GBApplication.addAppToNotifBlacklist(packageName); + break; case ACTION_DISMISS: { StatusBarNotification[] sbns = NotificationListener.this.getActiveNotifications(); - int handle = (int) intent.getLongExtra("handle", -1); + Long ts = (Long) mNotificationHandleLookup.lookup(handle); + if (ts == null) { + LOG.info("could not lookup handle for dismiss action"); + break; + } for (StatusBarNotification sbn : sbns) { - if ((int) sbn.getPostTime() == handle) { + if (sbn.getPostTime() == ts) { if (GBApplication.isRunningLollipopOrLater()) { String key = sbn.getKey(); NotificationListener.this.cancelNotification(key); @@ -146,8 +167,7 @@ public class NotificationListener extends NotificationListenerService { NotificationListener.this.cancelAllNotifications(); break; case ACTION_REPLY: - int id = (int)intent.getLongExtra("handle", -1); - NotificationCompat.Action wearableAction = (NotificationCompat.Action) mActionLookup.lookup(id); + NotificationCompat.Action wearableAction = (NotificationCompat.Action) mActionLookup.lookup(handle); String reply = intent.getStringExtra("reply"); if (wearableAction != null) { PendingIntent actionIntent = wearableAction.getActionIntent(); @@ -162,14 +182,13 @@ public class NotificationListener extends NotificationListenerService { try { LOG.info("will send exec intent to remote application"); actionIntent.send(context, 0, localIntent); - mActionLookup.remove(id); + mActionLookup.remove(handle); } catch (PendingIntent.CanceledException e) { LOG.warn("replyToLastNotification error: " + e.getLocalizedMessage()); } } break; } - } }; @@ -218,6 +237,18 @@ public class NotificationListener extends NotificationListenerService { return; } } + + // Ignore too frequent notifications, according to user preference + long min_timeout = prefs.getInt("notifications_timeout", 0) * 1000; + long cur_time = System.currentTimeMillis(); + if (notificationBurstPrevention.containsKey(source)) { + long last_time = notificationBurstPrevention.get(source); + if (cur_time - last_time < min_timeout) { + LOG.info("Ignoring frequent notification, last one was " + (cur_time - last_time) + "ms ago"); + return; + } + } + NotificationSpec notificationSpec = new NotificationSpec(); // determinate Source App Name ("Label") @@ -269,37 +300,51 @@ public class NotificationListener extends NotificationListenerService { NotificationCompat.WearableExtender wearableExtender = new NotificationCompat.WearableExtender(notification); List actions = wearableExtender.getActions(); + + if (actions.size() == 0 && NotificationCompat.isGroupSummary(notification)) { //this could cause #395 to come back + LOG.info("Not forwarding notification, FLAG_GROUP_SUMMARY is set and no wearable action present. Notification flags: " + notification.flags); + return; + } + notificationSpec.attachedActions = new ArrayList<>(); + + // DISMISS action + NotificationSpec.Action dismissAction = new NotificationSpec.Action(); + dismissAction.title = "Dismiss"; + dismissAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_DISMISS; + notificationSpec.attachedActions.add(dismissAction); + for (NotificationCompat.Action act : actions) { if (act != null) { NotificationSpec.Action wearableAction = new NotificationSpec.Action(); wearableAction.title = act.getTitle().toString(); if(act.getRemoteInputs()!=null) { - wearableAction.isReply = true; + wearableAction.type = NotificationSpec.Action.TYPE_WEARABLE_REPLY; + } else { + wearableAction.type = NotificationSpec.Action.TYPE_WEARABLE_SIMPLE; } - notificationSpec.flags |= NotificationSpec.FLAG_WEARABLE_ACTIONS; + notificationSpec.attachedActions.add(wearableAction); mActionLookup.add((notificationSpec.getId()<<4) + notificationSpec.attachedActions.size(), act); LOG.info("found wearable action: " + notificationSpec.attachedActions.size() + " - "+ act.getTitle() + " " + sbn.getTag()); } } + // OPEN action + NotificationSpec.Action openAction = new NotificationSpec.Action(); + openAction.title = getString(R.string._pebble_watch_open_on_phone); + openAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_OPEN; + notificationSpec.attachedActions.add(openAction); - if ((notificationSpec.flags & NotificationSpec.FLAG_WEARABLE_ACTIONS) == 0 && NotificationCompat.isGroupSummary(notification)) { //this could cause #395 to come back - LOG.info("Not forwarding notification, FLAG_GROUP_SUMMARY is set and no wearable action present. Notification flags: " + notification.flags); - return; - } + // MUTE action + NotificationSpec.Action muteAction = new NotificationSpec.Action(); + muteAction.title = getString(R.string._pebble_watch_mute); + muteAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_MUTE; + notificationSpec.attachedActions.add(muteAction); + + mNotificationHandleLookup.add(notificationSpec.getId(), sbn.getPostTime()); // for both DISMISS and OPEN + mPackageLookup.add(notificationSpec.getId(), sbn.getPackageName()); // for MUTE - // Ignore too frequent notifications, according to user preference - long min_timeout = prefs.getInt("notifications_timeout", 0) * 1000; - long cur_time = System.currentTimeMillis(); - if (notificationBurstPrevention.containsKey(source)) { - long last_time = notificationBurstPrevention.get(source); - if (cur_time - last_time < min_timeout) { - LOG.info("Ignoring frequent notification, last one was " + (cur_time - last_time) + "ms ago"); - return; - } - } notificationBurstPrevention.put(source, cur_time); notificationOldRepeatPrevention.put(source, notification.when); @@ -331,6 +376,9 @@ public class NotificationListener extends NotificationListenerService { private boolean isServiceRunning() { ActivityManager manager = (ActivityManager) getSystemService(ACTIVITY_SERVICE); + if (manager == null) { + return false; + } for (ActivityManager.RunningServiceInfo service : manager.getRunningServices(Integer.MAX_VALUE)) { if (DeviceCommunicationService.class.getName().equals(service.service.getClassName())) { return true; @@ -401,7 +449,9 @@ public class NotificationListener extends NotificationListenerService { @Override public void onNotificationRemoved(StatusBarNotification sbn) { - if (shouldIgnore(sbn) || true) // FIXME: DISABLED for now + // FIXME: DISABLED for now + /* + if (shouldIgnore(sbn)) return; Prefs prefs = GBApplication.getPrefs(); @@ -409,8 +459,10 @@ public class NotificationListener extends NotificationListenerService { LOG.info("notification removed, will ask device to delete it"); GBApplication.deviceService().onDeleteNotification((int) sbn.getPostTime()); } + */ } + /* private void dumpExtras(Bundle bundle) { for (String key : bundle.keySet()) { Object value = bundle.get(key); @@ -420,6 +472,7 @@ public class NotificationListener extends NotificationListenerService { LOG.debug(String.format("Notification extra: %s %s (%s)", key, value.toString(), value.getClass().getName())); } } + */ private boolean shouldIgnore(StatusBarNotification sbn) { /* @@ -492,7 +545,7 @@ public class NotificationListener extends NotificationListenerService { Prefs prefs = GBApplication.getPrefs(); if (!prefs.getBoolean("notifications_generic_whenscreenon", false)) { PowerManager powermanager = (PowerManager) getSystemService(POWER_SERVICE); - if (powermanager.isScreenOn()) { + if (powermanager != null && powermanager.isScreenOn()) { // LOG.info("Not forwarding notification, screen seems to be on and settings do not allow this"); return true; } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/SMSReceiver.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/SMSReceiver.java index 11e203fd3..ada2796f7 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/SMSReceiver.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/externalevents/SMSReceiver.java @@ -45,7 +45,7 @@ public class SMSReceiver extends BroadcastReceiver { } if ("when_screen_off".equals(prefs.getString("notification_mode_sms", "when_screen_off"))) { PowerManager powermanager = (PowerManager) context.getSystemService(Context.POWER_SERVICE); - if (powermanager.isScreenOn()) { + if (powermanager != null && powermanager.isScreenOn()) { return; } } @@ -74,11 +74,19 @@ public class SMSReceiver extends BroadcastReceiver { notificationSpec.body = entry.getValue().toString(); notificationSpec.phoneNumber = originatingAddress; notificationSpec.attachedActions = new ArrayList<>(); + + // REPLY action NotificationSpec.Action replyAction = new NotificationSpec.Action(); replyAction.title = "Reply"; - replyAction.isReply = true; + replyAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_REPLY_PHONENR; notificationSpec.attachedActions.add(replyAction); + // DISMISS ALL action + NotificationSpec.Action dismissAllAction = new NotificationSpec.Action(); + dismissAllAction.title = "Dismiss All"; + dismissAllAction.type = NotificationSpec.Action.TYPE_SYNTECTIC_DISMISS_ALL; + notificationSpec.attachedActions.add(dismissAllAction); + switch (GBApplication.getGrantedInterruptionFilter()) { case NotificationManager.INTERRUPTION_FILTER_ALL: break; diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/NotificationSpec.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/NotificationSpec.java index 24503d42d..8ace1c759 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/NotificationSpec.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/NotificationSpec.java @@ -21,8 +21,6 @@ import java.util.ArrayList; import java.util.concurrent.atomic.AtomicInteger; public class NotificationSpec { - public static final int FLAG_WEARABLE_ACTIONS = 0x00000001; - public int flags; private static final AtomicInteger c = new AtomicInteger((int) (System.currentTimeMillis()/1000)); private int id; @@ -64,7 +62,17 @@ public class NotificationSpec { } public static class Action implements Serializable { - public boolean isReply = false; + static final int TYPE_UNDEFINED = -1; + public static final int TYPE_WEARABLE_SIMPLE = 0; + public static final int TYPE_WEARABLE_REPLY = 1; + public static final int TYPE_SYNTECTIC_REPLY_PHONENR = 2; + public static final int TYPE_SYNTECTIC_DISMISS = 3; + public static final int TYPE_SYNTECTIC_DISMISS_ALL = 4; + public static final int TYPE_SYNTECTIC_MUTE = 5; + public static final int TYPE_SYNTECTIC_OPEN = 6; + + public int type = TYPE_UNDEFINED; + public long handle; public String title; } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/DeviceCommunicationService.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/DeviceCommunicationService.java index 9cb183328..f4b32fabf 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/DeviceCommunicationService.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/DeviceCommunicationService.java @@ -376,7 +376,7 @@ public class DeviceCommunicationService extends Service implements SharedPrefere } //TODO: check if at least one of the attached actions is a reply action instead? - if (((notificationSpec.flags & NotificationSpec.FLAG_WEARABLE_ACTIONS) > 0) + if ((notificationSpec.attachedActions != null && notificationSpec.attachedActions.size() > 0) || (notificationSpec.type == NotificationType.GENERIC_SMS && notificationSpec.phoneNumber != null)) { // NOTE: maybe not where it belongs // I would rather like to save that as an array in SharedPreferences diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleProtocol.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleProtocol.java index cbaa92549..7e8c4f809 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleProtocol.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleProtocol.java @@ -485,7 +485,6 @@ public class PebbleProtocol extends GBDeviceProtocol { @Override public byte[] encodeNotification(NotificationSpec notificationSpec) { - boolean hasHandle = notificationSpec.sourceAppId != null; int id = notificationSpec.getId() != -1 ? notificationSpec.getId() : mRandom.nextInt(); String title; String subtitle = null; @@ -507,7 +506,7 @@ public class PebbleProtocol extends GBDeviceProtocol { if (mFwMajor >= 3 || mForceProtocol || notificationSpec.type != NotificationType.GENERIC_EMAIL) { // 3.x notification return encodeNotification(id, (int) (ts & 0xffffffffL), title, subtitle, notificationSpec.body, - notificationSpec.sourceName, hasHandle, notificationSpec.type, notificationSpec.pebbleColor, + notificationSpec.type, notificationSpec.pebbleColor, notificationSpec.cannedReplies, notificationSpec.attachedActions); } else { // 1.x notification on FW 2.X @@ -794,9 +793,8 @@ public class PebbleProtocol extends GBDeviceProtocol { return encodeBlobdb(uuid, BLOBDB_INSERT, BLOBDB_PIN, buf.array()); } - private byte[] encodeNotification(int id, int timestamp, String title, String subtitle, String body, String sourceName, - boolean hasHandle, NotificationType notificationType, byte backgroundColor, - String[] cannedReplies, ArrayList attachedActions) { + private byte[] encodeNotification(int id, int timestamp, String title, String subtitle, String body, + NotificationType notificationType, byte backgroundColor, String[] cannedReplies, ArrayList attachedActions) { final short NOTIFICATION_PIN_LENGTH = 46; final short ACTION_LENGTH_MIN = 6; @@ -811,25 +809,6 @@ public class PebbleProtocol extends GBDeviceProtocol { // Calculate length first int actions_count = 0; short actions_length = 0; - String dismiss_string; - String open_string = GBApplication.getContext().getString(R.string._pebble_watch_open_on_phone); - String mute_string = GBApplication.getContext().getString(R.string._pebble_watch_mute); - if (sourceName != null) { - mute_string += " " + sourceName; - } - - byte dismiss_action_id; - if (hasHandle && !"ALARMCLOCKRECEIVER".equals(sourceName)) { - actions_count += 3; - dismiss_string = "Dismiss"; - dismiss_action_id = 0x02; - actions_length += (short) (ACTION_LENGTH_MIN * 3 + dismiss_string.getBytes().length + open_string.getBytes().length + mute_string.getBytes().length); - } else { - actions_count += 1; - dismiss_string = "Dismiss all"; - dismiss_action_id = 0x03; - actions_length += (short) (ACTION_LENGTH_MIN + dismiss_string.getBytes().length); - } int replies_length = 0; if (cannedReplies != null && cannedReplies.length > 0) { @@ -846,7 +825,7 @@ public class PebbleProtocol extends GBDeviceProtocol { for (Action act : attachedActions) { actions_count++; actions_length += (short) (ACTION_LENGTH_MIN + act.title.getBytes().length); - if (act.isReply) { + if (act.type == Action.TYPE_WEARABLE_REPLY || act.type == Action.TYPE_SYNTECTIC_REPLY_PHONENR) { actions_length += (short) replies_length + 3; // 3 = attribute id (byte) + length(short) } } @@ -941,46 +920,42 @@ public class PebbleProtocol extends GBDeviceProtocol { buf.put(backgroundColor); } - // dismiss action - buf.put(dismiss_action_id); - buf.put(dismiss_action_type); - buf.put((byte) 0x01); // number attributes - buf.put((byte) 0x01); // attribute id (title) - buf.putShort((short) dismiss_string.getBytes().length); - buf.put(dismiss_string.getBytes()); - - // open and mute actions - if (hasHandle && !"ALARMCLOCKRECEIVER".equals(sourceName)) { - buf.put((byte) 0x01); - buf.put((byte) 0x02); // generic action - buf.put((byte) 0x01); // number attributes - buf.put((byte) 0x01); // attribute id (title) - buf.putShort((short) open_string.getBytes().length); - buf.put(open_string.getBytes()); - - buf.put((byte) 0x04); - buf.put((byte) 0x02); // generic action - buf.put((byte) 0x01); // number attributes - buf.put((byte) 0x01); // attribute id (title) - buf.putShort((short) mute_string.getBytes().length); - buf.put(mute_string.getBytes()); - } - if (attachedActions != null && attachedActions.size() > 0) { for (int ai = 0 ; ai 0) {