organicmaps/organicmaps

Fork 18

Add foreground service to indicate about ongoing downloads #10284

Merged

zeac merged 1 commit from github/fork/zeac/resolve/520 into master

2025-02-25 11:14:56 +00:00

zeac commented

2025-02-18 14:33:45 +00:00

(Migrated from github.com)

No description provided.

zeac commented

2025-02-18 14:33:46 +00:00

(Migrated from github.com)

requested review from @ghost

requested review from `@ghost`

zeac commented

2025-02-18 14:33:46 +00:00

(Migrated from github.com)

requested review from @ghost

requested review from `@ghost`

pastk commented

2025-02-22 16:02:35 +00:00

Member

I did a quick test:

set some maps to update and put OM to background

I expected a system notification, but there was none
updates finished successfully (no interruption)!

set some new maps to download and put OM to background

there was a system notification
- the progress line was just "spinning", but no real progress line of "xx out of yy" figures displayed
- there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active)
downloads finished successfully (no interruption)!

I did a quick test: 1. set some maps to update and put OM to background - I expected a system notification, but there was none - updates finished successfully (no interruption)! 2. set some new maps to download and put OM to background - there was a system notification - the progress line was just "spinning", but no real progress line of "xx out of yy" figures displayed - there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active) - downloads finished successfully (no interruption)!

zeac commented

2025-02-24 23:13:07 +00:00

(Migrated from github.com)

I did a quick test:

1. set some maps to update and put OM to background


* I expected a system notification, but there was none

* updates finished successfully (no interruption)!


2. set some new maps to download and put OM to background


* there was a system notification
  
  * the progress line was just "spinning", but no real progress line of "xx out of yy" figures displayed
  * there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active)

* downloads finished successfully (no interruption)!

There are 2 possible reasons why notification wasn't visible:

Lack of POST_NOTIFICATION permission. That permission is requested by the app in some circumstances, but some paths could be missing.
There is a 10 seconds delay before the notification became visible, it's a system feature.

I was unable to get correct progress from MapManager and to keep this PR smaller, I will address that in next ones. Indeterminate progress is better than no notification.

There is one additional feature required: Starting from Android 14 foreground service is not a recommended way to implement such downloads: user-initiated data transfers. As foreground service is still needed for older versions of android it's worth to make it first.

> I did a quick test: > > 1. set some maps to update and put OM to background > > > * I expected a system notification, but there was none > > * updates finished successfully (no interruption)! > > > 2. set some new maps to download and put OM to background > > > * there was a system notification > > * the progress line was just "spinning", but no real progress line of "xx out of yy" figures displayed > * there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active) > > * downloads finished successfully (no interruption)! There are 2 possible reasons why notification wasn't visible: 1. Lack of POST_NOTIFICATION permission. That permission is requested by the app in some circumstances, but some paths could be missing. 2. There is a 10 seconds delay before the notification became visible, it's a system [feature](https://developer.android.com/develop/background-work/background-tasks#is_it_a_short_critical_task). I was unable to get correct progress from MapManager and to keep this PR smaller, I will address that in next ones. Indeterminate progress is better than no notification. There is one additional feature required: Starting from Android 14 foreground service is not a recommended way to implement such downloads: [user-initiated data transfers](https://developer.android.com/develop/background-work/background-tasks/uidt). As foreground service is still needed for older versions of android it's worth to make it first.

pastk commented

2025-02-25 07:36:56 +00:00

Member

@organicmaps/android anyone else to test it and review please?

`@organicmaps/android` anyone else to test it and review please?

rtsisyk commented

2025-02-25 11:03:14 +00:00

Owner

Review: Approved

LGTM. Thanks for contributing!!

**Review:** Approved LGTM. Thanks for contributing!!

rtsisyk commented

2025-02-25 11:03:14 +00:00

Owner

approved this merge request

rtsisyk closed this pull request

2025-02-25 11:14:56 +00:00

rtsisyk merged commit into master

2025-02-25 11:14:56 +00:00

rtsisyk commented

2025-02-25 11:15:40 +00:00

Owner

Merging this PR now. Any follow-ups can go in separate PRs.

pastk commented

2025-02-26 11:13:27 +00:00

Member

the string should be localized

pastk commented

2025-02-26 11:23:37 +00:00

Member

nativeUpdate() calls should be changed similarly so that fg service is started

`nativeUpdate()` calls should be changed similarly so that fg service is started

pastk commented

2025-02-26 11:28:28 +00:00

Member

looks like there is an "auto-retry" feature in the core

  /**
   * Returns {@code true} if the core will NOT do attempts to download failed maps anymore.
   */
  public static native boolean nativeIsAutoretryFailed();

When its triggered then the fg service is not started as the client part doesn't know there is a re-try happening in the core?

looks like there is an "auto-retry" feature in the core ``` /** * Returns {@code true} if the core will NOT do attempts to download failed maps anymore. */ public static native boolean nativeIsAutoretryFailed(); ``` When its triggered then the fg service is not started as the client part doesn't know there is a re-try happening in the core?

pastk commented

2025-02-26 11:30:18 +00:00

Member

hmm what was its role and why its no longer needed?

pastk commented

2025-02-26 11:35:52 +00:00

Member

There are 2 possible reasons why notification wasn't visible:
1. Lack of POST_NOTIFICATION permission. That permission is requested by the app in some circumstances, but some paths could be missing.

I'm on Android 10, looking at the code it was introduced in tiramisu recently.

2. There is a 10 seconds delay before the notification became visible, it's a system [feature](https://developer.android.com/develop/background-work/background-tasks#is_it_a_short_critical_task).

I've waited for longer.
Also when I start a new country download then a notification appears immediately.

Looking at the code you forgot to wrap nativeUpdate() calls, so the fg service is not started.
Also there is no notification if there was an auto-retry. See my code comments.

I was unable to get correct progress from MapManager and to keep this PR smaller, I will address that in next ones. Indeterminate progress is better than no notification.

Its OK.
Also when tapped on the notification it opens OM's map screen. It should open the downloader screen instead (and ideally auto-scroll the list to the country being downloaded).

> There are 2 possible reasons why notification wasn't visible: > > 1. Lack of POST_NOTIFICATION permission. That permission is requested by the app in some circumstances, but some paths could be missing. I'm on Android 10, looking at the code it was introduced in tiramisu recently. > > 2. There is a 10 seconds delay before the notification became visible, it's a system [feature](https://developer.android.com/develop/background-work/background-tasks#is_it_a_short_critical_task). > I've waited for longer. Also when I start a new country download then a notification appears immediately. Looking at the code you forgot to wrap `nativeUpdate()` calls, so the fg service is not started. Also there is no notification if there was an auto-retry. See my code comments. > I was unable to get correct progress from MapManager and to keep this PR smaller, I will address that in next ones. Indeterminate progress is better than no notification. Its OK. Also when tapped on the notification it opens OM's map screen. It should open the downloader screen instead (and ideally auto-scroll the list to the country being downloaded).

pastk commented

2025-02-26 11:40:19 +00:00

Member

But first let's focus on this:

there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active)

The repeated beeping continues whether in fg or bg, sometimes more rare, sometimes more often. Maybe it beeps every time a progress update is posted? Also I've noticed a small "bell" icon on the notification box appearing for a split second and then disappearing again.

This beeping is really annoying.
I think its a release blocker.
Could anyone reproduce it?

But first let's focus on this: > there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active) The repeated beeping continues whether in fg or bg, sometimes more rare, sometimes more often. Maybe it beeps every time a progress update is posted? Also I've noticed a small "bell" icon on the notification box appearing for a split second and then disappearing again. This beeping is really annoying. I think its a release blocker. Could anyone reproduce it?

zeac commented

2025-02-27 13:47:58 +00:00

(Migrated from github.com)

RetryFailedDownloadConfirmationListener was needed to remove "Download failed" notification. It's no longer needed as the new progress notification will replace it.

zeac commented

2025-02-27 15:20:23 +00:00

(Migrated from github.com)

But first let's focus on this:

there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active)

The repeated beeping continues whether in fg or bg, sometimes more rare, sometimes more often. Maybe it beeps every time a progress update is posted? Also I've noticed a small "bell" icon on the notification box appearing for a split second and then disappearing again.

This beeping is really annoying. I think its a release blocker. Could anyone reproduce it?

This will help: #10359

> But first let's focus on this: > > > there were repeated annoying notification sounds (doesn't happen when e.g. track recording is active) > > The repeated beeping continues whether in fg or bg, sometimes more rare, sometimes more often. Maybe it beeps every time a progress update is posted? Also I've noticed a small "bell" icon on the notification box appearing for a split second and then disappearing again. > > This beeping is really annoying. I think its a release blocker. Could anyone reproduce it? This will help: https://git.omaps.dev/organicmaps/organicmaps/pulls/10359

zeac commented

2025-02-27 15:51:09 +00:00

(Migrated from github.com)

Looking at the code you forgot to wrap nativeUpdate() calls, so the fg service is not started. Also there is no notification if there was an auto-retry. See my code comments.

Yes, you're right about nativeUpdate: #10360
I'm not sure about auto-retry. My understanding is that

a region is still in the queue during retries and so the service will be kept running.
there is no auto retries after the region is in the failed state.

> Looking at the code you forgot to wrap `nativeUpdate()` calls, so the fg service is not started. Also there is no notification if there was an auto-retry. See my code comments. Yes, you're right about nativeUpdate: https://git.omaps.dev/organicmaps/organicmaps/pulls/10360 I'm not sure about auto-retry. My understanding is that 1. a region is still in the queue during retries and so the service will be kept running. 2. there is no auto retries after the region is in the failed state.

pastk commented

2025-02-28 04:35:43 +00:00

Member

Yes, you're right about nativeUpdate: #10360 I'm not sure about auto-retry. My understanding is that
1. a region is still in the queue during retries and so the service will be kept running.

2. there is no auto retries after the region is in the failed state.

To reproduce:

enable airplane mode
tap on the map to update
map's icon will turn red
put OM into bg
enable internet
switch back to OM
download would resume automatically
no download notification shown

Reproducible when downloading a new map also, but not every time - sometimes a dialog "download failed, retry internet connection" is displayed immediately (after that there is no auto-retry), sometimes not.

> > Yes, you're right about nativeUpdate: #10360 I'm not sure about auto-retry. My understanding is that > > 1. a region is still in the queue during retries and so the service will be kept running. > > 2. there is no auto retries after the region is in the failed state. To reproduce: 1. enable airplane mode 2. tap on the map to update 3. map's icon will turn red 4. put OM into bg 5. enable internet 6. switch back to OM 7. download would resume automatically 8. no download notification shown Reproducible when downloading a new map also, but not every time - sometimes a dialog "download failed, retry internet connection" is displayed immediately (after that there is no auto-retry), sometimes not.

pastk commented

2025-02-28 04:49:50 +00:00

Member

Another bug that appeared on a slow cellular connection.

Started a region update, notification appeared
Put OM into bg and switched the screen off for quite some time
Turned the phone on, switched to OM
The download was "stuck" (progress not changing) for some time
Then it turned into the red error icon

So when the FG service is running then are there auto-retries on errors and on wake up?
I expected it to either continue download reliably when the phone is off (but might depend on phone's settings? and I don't have anything forbidden for that) or at least continue upon wake up.

Then the same as prev post:
6. Put OM into bg
7. Repeated steps 6-8 from prev post - there was auto-resume with no notification.

Another bug that appeared on a slow cellular connection. 1. Started a region update, notification appeared 2. Put OM into bg and switched the screen off for quite some time 3. Turned the phone on, switched to OM 4. The download was "stuck" (progress not changing) for some time 5. Then it turned into the red error icon So when the FG service is running then are there auto-retries on errors and on wake up? I expected it to either continue download reliably when the phone is off (but might depend on phone's settings? and I don't have anything forbidden for that) or at least continue upon wake up. Then the same as prev post: 6. Put OM into bg 7. Repeated steps 6-8 from prev post - there was auto-resume with no notification.

pastk commented

2025-03-03 02:45:20 +00:00

Member

mentioned in issue #10369

rtsisyk approved these changes 2025-03-22 17:31:22 +00:00