search

criticalmaps / criticalmaps-android

🤖 Critical Maps Android App
http://criticalmaps.net
Apache License 2.0
136 stars 27 forks source link

Lib updates cleanup and deprecations #221

Closed cbalster closed 6 years ago

cbalster commented 6 years ago

Can someone with an android O device please check the behavior of the notification channel? According to docs IMPORTANCE_DEFAULT should not make a sound, but answer 3 in this SO post claims otherwise and recommends using _LOW: https://stackoverflow.com/questions/45919392/disable-sound-from-notificationchannel Ideally test it the way it is and with importance set to _DEFAULT.

stephanlindauer commented 6 years ago

Regarding preperations for Oreo, i actually got a heads-up from the developer of go-crew:

hi Stephan,

Ich bin der entwickler der hat Go Crew gemacht. https://go-crew.com

Ich wolte nur dir sagen das Android Oreo hat mein app ganz schlim impaktiert .

 2 grosse sachen sind:

1 - background services sind sehr limitiert. Ich glaube ihr nutzt das auch fur standort updates ( jedes 11 sec) 

2 - notifcations mussen ein channel habben, 

Vor ihr die Target Api an 27 macht, bitte auf die 2 sachen aufpassen.

Mehr info:
https://blog.klinkerapps.com/android-o-background-services/

I haven't looked into those issues yet. I guess we'll cross that bridge once we see that stuff doesn't work as expected.

cbalster commented 6 years ago

The background service issue has actually already been addressed in: https://github.com/criticalmaps/criticalmaps-android/commit/5bde79c2865428f6a9138fa994d1733a004b8f55 We don't really have a problem there since we always show a notification and are therefore considered to be in the foreground. All that had to be changed was the method to start the service with which we basically promise to call setForeground within a specific time window (which is several seconds after service start, so we are fine here).

Where you able to test the notification channel stuff? Unfortunately I don't have an O device and the Genymotion Oreo image doesn't work on my machine. Since I have an AMD CPU I can't use the Google Emulator...

stephanlindauer commented 6 years ago

I just tested this in my emulator on a Pixel 2 with Oreo and notifications worked fine. Are there any specific things to test for?

cbalster commented 6 years ago

Great, thanks! Mainly whether the notification makes a sound (it shouldn't), if it's visible on lockscreen (it should) and whether the name an description of the notification channel in the notification settings are okay.

stephanlindauer commented 6 years ago

tested again. all of that works as expected. :+1:

cbalster commented 6 years ago

Nice, thank you! Then I'll just fix the notification description placeholder text and then it should be good to go. :)