nightscout / AndroidAPS

Opensource automated insulin delivery system (closed loop)
https://wiki.aaps.app
GNU Affero General Public License v3.0
685 stars 1.67k forks source link

Wear : Data layer foreground notification #3143

Closed robertrub closed 7 months ago

robertrub commented 8 months ago

Latest Dev on phone and wear.

Screenshot_20231225_183747_AAPS.jpg

I keep getting the Data layer foreground notification over and over. Screenshot_20231227_180411_sysui.png

Can't erase it.

Screenshot_20231227_180447_sysui.png

The only option is open the notification settings and stop the notifications but it then asks me if I want to stop all notifications from AAPS...

Screenshot_20231227_180419_sysui.png

Was working great before. I don't know why this commit was added but it made things worst.

Logs.

AndroidAPS_LOG_1703697301388.log.zip

robertrub commented 8 months ago

Turning OFF the notification turned OFF ALL notifications on watch from AAPS.

https://github.com/nightscout/AndroidAPS/commit/d75c4651f6e41b44486c8b13500d6b5972ae406a Is not working as it should.

robertrub commented 8 months ago

Updated to the latest Dev on phone and watch tonight and the problem is still there.

Screenshot_20231227_203227_AAPS.jpg

vanelsberg commented 8 months ago

Confirming: See also https://github.com/nightscout/AndroidAPS/issues/3137 (AAPS Wear: Ongoing bolus notification that can not be cleared)

This commit may be fixing a problem for a minority of users not running a 3th party watch face, but is apparently causing problems for most users running AAPS-wear the way it was designed for?

Proposed solution: Rollback changes and review for better solution, or alternatively: enable "Wear: Foreground Service" only as a user option through watch face settings (default = OFF)

MilosKozak commented 8 months ago

on android 10+ ongoing notification must be in place if you want to have service running on background

robertrub commented 8 months ago

Till now, the notifications were working great . Now we're stuck with a notification that we can't erase.

The fix made things worst (at least for some (or many ?) users).

Used on Android 13 and 14.

MilosKozak commented 8 months ago

in 3.2.0.3 service was suspended without active watchface (on background) and complications were not working. if you add notification android allows service running on background. the price for it is permanent notification

vanelsberg commented 8 months ago

in 3.2.0.3 service was suspended without active watchface (on background) and complications were not working. if you add notification android allows service running on background. the price for it is permanent notification

@MilosKozak @robertrub How about making this notification feature optional (read: configurable on/off)? I could do a PR for this...

robertrub commented 8 months ago

Totally vote for it being an option.

MilosKozak commented 8 months ago

totally against more options :)

robertrub commented 8 months ago

You're the boss and I totally agree on there are too many options syndrome (ie xDrip) ;)

How many people use the AAPS watchface and how many use others ?

I still believe that more will be bothered with the change than happy to gave it.

Of course, you can ignore the stuck notification on the watch but I'll be the one to tell them to ignore it when they'll complain about it on Discord... :(

vanelsberg commented 8 months ago

totally against more options :)

Agree. 'need optional option to make options optional :-)

TebbeUbben commented 7 months ago

@MilosKozak As far as I know, using an ongoing notification is not required, although the system might enforce that depending on the Android version. It might also be worth an attempt to use PRIORITY_MIN. These notifications did not appear on my watch at all and I got a notification by the system instead that AndroidAPS was running in the background (which can be dismissed).

Anyways, on my Pixel Watch 2 I'm able to disable the notification channel in the system settings. Perhaps that setting is hidden on watches from other manufacturers. Manually launching the intent might work (see https://stackoverflow.com/questions/32366649/any-way-to-link-to-the-android-notification-settings-for-my-app). There's probably also a way to manage notifications via ADB.

vanelsberg commented 7 months ago

...] Perhaps that setting is hidden on watches from other manufacturers [...

With current dev-builds, on the Samsung Galaxy Watch 4, I can hide the ongoing notification through developer menu settings on Watch:

App Notifications -> All watch apps -> AAPS -> Data layer Foreground Service Channel: OFF

robertrub commented 7 months ago

No, I don't have that option. Samsung Android 14. Both phone and watch are set to developer mode.

Am I searching in the wrong place ?

Screenshot_20240114_090427_Settings.jpg

vanelsberg commented 7 months ago

No, I don't have that option...

Wrong place. Check developer options menu on Watch:

  1. Scroll all to way down to "App Notifications"
  2. Select: All watch apps -> AAPS -> Data layer Foreground Service Channel
  3. Switch it to OFF

(btw: No need to set the Phone to developer mode)

robertrub commented 7 months ago

OK, thanks. Found it and deactivated it with succes.

Not the best option/solution for an "average" user though.

Robby

olorinmaia commented 7 months ago

@MilosKozak Here are my thoughts on the matter. I don't think we want to solve a problem for a few users, and create a new problem that will concern all. As it stands now the data layer foreground notification is an annoyance that most users will have issues with. It vibrates often without any reason, and if you enact anything from the tiles it also comes up first, before what you usually see. This is tested with a Samsung Galaxy Watch 4 with latest dev and updates from google on watch.

I've been trying to alter the code to make it less annoying, it seems that the following helps, keeping it from blocking wanted notifications when enacting from tiles and vibrating alot less. It still isn't possible to remove tho, and I find that troubling, as it very often adds a dot to the watch screen making it look like you have a missed notification. Which is usually the datalayer foreground notification :/ image image

I have tested a few days without commit 041d0f0b to pursue @TebbeUbben claim that a notification channel is not necessary, and so far, using stock samsung galaxy watch watchface I'm still able to enact from all commands from every tile. So maybe a persistent channel isn't necessary? Would be nice if more could test this, so we don't add stuff that isn't really necessary and just is in the way of notifications you really want to see. Edit: Having issues when using AAPS watchface. So this is probably not a solution to directly revert that commit.

vanelsberg commented 7 months ago

I have tested a few days without commit https://github.com/nightscout/AndroidAPS/commit/041d0f0b7bd43042dbf201f442c1a48433eb22cd

Originally had the idea to disable the changes based on a watch config option setting.

However, when I disable this commit in current dev by commenting out the changes (see code from the first screenshot) somehow this leaves the wear app unstable in that what looks like the watchface restarting about every minute or so? Did not get to load this into the debugger (need to figure out how to do that ... ) to see what it is doing...

olorinmaia commented 7 months ago

I have tested a few days without commit 041d0f0

Originally had the idea disable the change based on a watch config option setting.

However, when I disable this commit in current dev by commenting out the changes (see code from the first screenshot) somehow this leaves the wear app unstable in that what looks like the watchface restarting about every minute or so? Did not get to load this into the debugger (need to figure out how to do that ... ) to see what it is doing...

Edit: Experiencing same problem as @vanelsberg using AAPS watchface when reverting commit https://github.com/nightscout/AndroidAPS/commit/041d0f0b7bd43042dbf201f442c1a48433eb22cd I had tested without AAPS watchface, and not enough with AAPS watchface.. :)

MilosKozak commented 7 months ago

the reason why I added notification channel is that in logs were a message that communication service was killed because it didn't call startforeground. That disappeared after mentioned change. My thoughts is that complications cannot work properly without this service. The change what started this is targeting API 29 on watch in last master version

vanelsberg commented 7 months ago

the reason why I added notification channel ...

Thanks for clarifying, which would explain. Seems solving this is not as straightforward as I anticipated.

olorinmaia commented 7 months ago

Please test latest dev 8cfa312 and see if it's better now. At least for me it works great. (Samsung Galaxy watch 4)