Add ability to generate dismiss notifications while reminder is active

[rryk]

I was not able to properly test this on my phone because for some reason when I deploy and try to debug the app, the ReminderNotificationListenerService instance is never created. The other day everything worked as expected, but I've made some refactoring changes and couldn't test them anymore. Do you know what might be causing Dagger not to create an instance of the ReminderNotificationListenerService class?

Also I can't add Japanese and Bulgarian translations for the new strings becase I don't speak these languages. If you'd like me to use Google Translate, please let me know and I'll be happy to add them.

[httpdispatch]

Wow, very nice feature. I'll try to review and merge it this week.

Regarding debugger - i've also had problems to attach debugger to the NotificationListenerService. Also i've noticed once NLS is crashed at least once it will not run again until device is rebooted.

Also please feel free to use google translate to add Japanese and Bulgarian translation.

[rryk]

I've actually tried rebooting the device several times, but alas no luck. I've managed to somewhat test this on an emulator by wiping the device clean after it stops creating ReminderNotificationListenerService, but I am not sure how to simulate new notifications from other apps when I need them. On my physical device I mostly use Hangouts to send myself messages :-).

I also don't think it's about attaching to the ReminderNotificationListenerService specifically. I am able to attach to the app, e.g. step through the CustomApplication.onCreate() method, but the class ReminderNotificationListenerService is not created, which is also confirmed by the lack of log entries in Android Monitor. I've also realized that it's not Dagger that creates the class instance, but Android itself when the user enables access to the notifications. Seems like a bug in Android to me...

[httpdispatch]

Do you have Android 8 on your device?

[httpdispatch]

If so i had a couple of crash reports from Android 8 devices: NPE in the NLS service creation. I have a theory that Application.onCreate is called after the NLS.onCreate at some Android 8 ROMs so the dagger injector instance is null there which causes NPE. The moving application initialization to the constructor instead of onCreate may help. But this is a theory - i can't test it, such as on Android 8 emulator it works fine.

[httpdispatch]

Do you see any crash logs in the LogCat?

[rryk]

Yes, I do have Android 8.0 running on Nexus 5X, but not at my home computer atm, so can't look up any logs... What is NPE?

[rryk]

Also I've had a breakpoint on the first line of the onCreate constructor of the ReminderNotificationListenerService, so unless Dagger does injection before that, it should still have triggered the breakpoint, shouldn't it?

[httpdispatch]

NPE is NullPointerException

Also I've had a breakpoint on the first line of the onCreate constructor of the ReminderNotificationListenerService, so unless Dagger does injection before that, it should still have triggered the breakpoint, shouldn't it?

I never had a success in debugging NotificationListenerService itself - always used logging for that.

[rryk]

One potential issue that I've observed while testing on physical device still worked: Hangouts may not create a new notification when the user posts another message, but instead update an existing one. This updated notification will have the same key, therefore the app will continue to ignore it, whereas IMHO it should start reminding again. One solution we can try to use is to add notification.getPostTime() or notification.getNotification().when to the key, but I am not sure whether either of them is actually updated in this case (and can't test it either...).

[rryk]

Ah, thanks for explaining about NPE. I've seen some exceptions (maybe even NPE) when listing apps in the settings activity. Looks like the activity is not always able to decode images for each app and throws an exception. The apps still show up, but without icons. This is probably irrelevant here though...

[httpdispatch]

One potential issue that I've observed while testing on physical device still worked: Hangouts may not create a new notification when the user posts another message, but instead updates an existing one. This updated notification will have the same key, therefore the app will continue to ignore it, whereas IMHO it should start reminding again. One solution we can try to use is to add notification.getPostTime() or notification.getNotification().when to the key, but I am not sure whether either of them is actually updated in this case.

I can see one solution for the described issue. If onNotificationPosted is called with the notification which has the same id it should be removed from the mIgnoredNotificationKeys structure.

[rryk]

That's a good idea. Once I can get testing on my physical device work again, I'd love to test this approach.

[httpdispatch]

To fix you physical device try to disable NLS service, reboot device and enable NLS service again

[httpdispatch]

https://stackoverflow.com/a/40825389/527759

[httpdispatch]

Try to increment version code to avoid this caching issue

[httpdispatch]

https://github.com/httpdispatch/MissedNotificationsReminder/blob/master/app/build.gradle#L5

[rryk]

Awesome, looks like it might just work. Not sure if I can get to it during the week, but I'll definitely have a look on the weekend.

[rryk]

Tried changing versionBuild, app_name, rebooting phone, uninstalling manually, several combinations thereof... still the service is not created by Android. In fact v14Debug works better on my phone now...

[rryk]

Finally, removing manually, rebooting, changing app_name together worked... :-)

[httpdispatch]

That is very weird V14Debug is not accurate because of there are no correct ways to detect notification removal

[rryk]

Implemented and tested your suggestion. Unfortunately, only works on v18 because on v14 the AccessibilityEvent.TYPE_NOTIFICATION_STATE_CHANGED event is not sent when notification changes, therefore we have not way of detecting that. I think this is now ready for your final review. I'll continue testing the version of the app with these changes over the next couple of days and let you know if I'll find any other issues.

[rryk]

V14Debug is not accurate because of there are no correct ways to detect notification removal

It works better in the sense that it creates services and reminds me about notificaitons. v18 did not start the service so I actually almost missed a meeting yersterday due to a missing reminder. But now that I've found a way to make v18 create the service, it's not an issue anymore.

Again steps I've used to reliably restart the app are:

• remove the app from the phone manually
• reboot the phone
• install again by running in Debug mode

[httpdispatch]

I think this is now ready for your final review.

I hope to dedicate a time to merge and release this awesome feature on the weekend. Thank you very much for contribution.

[httpdispatch]

Merged and released with some minor fixes applied. Thank you very much for the contribution

[rryk]

Thanks a lot for the fixes especially with regards to NPE on some devices.