Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix offline map download resumption #273

Merged
merged 6 commits into from
Mar 13, 2019

Conversation

ekigamba
Copy link
Contributor

@ekigamba ekigamba commented Mar 4, 2019

Fixes #194

To test, my guess is turning off the internet(Wifi in case it's the source) and turning it back on after 5 mins and 11-13 mins to see if the download continues. It should resume within a few seconds of connecting back.

You should also monitor the log for error-tag logs from Mapbox. The only error-tag logs from Mapbox should be after disconnecting the internet. Error-tag logs while the internet is off indicates that Mapbox is still retrying the connection

More notes are on the issue

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 4, 2019

BLOCKED waiting for the other PRs to be merged

@coveralls
Copy link

coveralls commented Mar 4, 2019

Pull Request Test Coverage Report for Build 1257

  • 12 of 35 (34.29%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.1%) to 32.972%

Changes Missing Coverage Covered Lines Changed/Added Lines %
library/src/main/java/io/ona/kujaku/downloaders/MapBoxOfflineResourcesDownloader.java 12 16 75.0%
library/src/main/java/io/ona/kujaku/services/MapboxOfflineDownloaderService.java 0 4 0.0%
sample/src/main/java/io/ona/kujaku/sample/activities/OfflineRegionsActivity.java 0 15 0.0%
Files with Coverage Reduction New Missed Lines %
library/src/main/java/io/ona/kujaku/services/MapboxOfflineDownloaderService.java 1 56.9%
sample/src/main/java/io/ona/kujaku/sample/activities/OfflineRegionsActivity.java 1 0.0%
Totals Coverage Status
Change from base Build 1240: 0.1%
Covered Lines: 1643
Relevant Lines: 4983

💛 - Coveralls

@ekigamba ekigamba removed the blocked label Mar 5, 2019
The Android MapboxSDK OfflineManager does not directly enable the com.mapbox.mapboxsdk.net.ConnectivityReceiver when offline region download resumes. It is only enabled when an offline region is being created and deactivated immediately after. However, the OfflineManager and it's components do use the com.mapbox.mapboxsdk.net.ConnectivityReceiver(which is a singleton) if it is activated.

This fix activates the com.mapbox.mapboxsdk.net.ConnectivityReceiver before an OfflineRegion status is activated and deactivates after the OfflineRegion is done downloading, encounters a tile count limit exception or the Service is shutdown
@ekigamba ekigamba force-pushed the issue/194-offline-map-download-resumption branch from 8034f30 to dbfe605 Compare March 5, 2019 13:41
@vincent-karuri
Copy link
Contributor

screenshot_20190306-175100

Getting very many error notification messages when wifi is turned off while downloading map.

Samsung J5 physical phone.

@vincent-karuri
Copy link
Contributor

screenshot_20190306-175410

Looks like there are duplicated offline region details when you first open the activity after a period of online and offline wifi resumption.

Opening the offline downloads activity again will then show the correct information.

@vincent-karuri
Copy link
Contributor

screenshot_20190306-180507

Not sure if this is an issue but the notification shows 42.29mb downloaded at 100% while the activity above shows 44.34mb?

- Replace offline download error notifications
- Show inconsistent download size
- Fix duplicated offline map download status information
@vincent-karuri
Copy link
Contributor

For some reason, I can still download maps even when wifi is off, after downloading a previous map with wifi available.

@vincent-karuri
Copy link
Contributor

When you then turn on the wifi and turn it off, it shows the error notification.

However, on wifi resumption, it takes a while for the app to resume downloading.

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 7, 2019

For some reason, I can still download maps even when wifi is off, after downloading a previous map with wifi available.

Mapbox shares resources if the two or more maps use the same resources. If you download two areas in the same area, or covering some shared area, Mapbox does not redownload such tiles, fonts or other kind of resources

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 7, 2019

When you then turn on the wifi and turn it off, it shows the error notification.

However, on wifi resumption, it takes a while for the app to resume downloading.

If it takes less than a minute or around there off, then it is OK. The download progress should be picked up as soon as the device itself establishes that it has a connection/internet connection(If I am not wrong). Longer than that would be an issue

Is it clear after how long the download resumes and does the device connect immediately and internet connectivity resume immediately

@vincent-karuri
Copy link
Contributor

vincent-karuri commented Mar 7, 2019

When you then turn on the wifi and turn it off, it shows the error notification.

However, on wifi resumption, it takes a while for the app to resume downloading.

If it takes less than a minute or around there off, then it is OK. The download progress should be picked up as soon as the device itself establishes that it has a connection/internet connection(If I am not wrong). Longer than that would be an issue

Is it clear after how long the download resumes and does the device connect immediately and internet connectivity resume immediately

Took about 4 min in this scenario. Internet resumes immediately.

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 7, 2019

When you then turn on the wifi and turn it off, it shows the error notification.

However, on wifi resumption, it takes a while for the app to resume downloading.

If it takes less than a minute or around there off, then it is OK. The download progress should be picked up as soon as the device itself establishes that it has a connection/internet connection(If I am not wrong). Longer than that would be an issue
Is it clear after how long the download resumes and does the device connect immediately and internet connectivity resume immediately

Took about 4 min in this scenario. Internet resumes immediately.

Would it be possible to provide the specific time after which you resumed internet(So that I can test using the same time on my side)? And probably the logs if possible

@vincent-karuri
Copy link
Contributor

When you then turn on the wifi and turn it off, it shows the error notification.

However, on wifi resumption, it takes a while for the app to resume downloading.

If it takes less than a minute or around there off, then it is OK. The download progress should be picked up as soon as the device itself establishes that it has a connection/internet connection(If I am not wrong). Longer than that would be an issue
Is it clear after how long the download resumes and does the device connect immediately and internet connectivity resume immediately

Took about 4 min in this scenario. Internet resumes immediately.

Would it be possible to provide the specific time after which you resumed internet(So that I can test using the same time on my side)? And probably the logs if possible

Try 5 min.

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 7, 2019

screenshot_20190307-192828
screenshot_20190307-192852
screenshot_20190307-192856
screenshot_20190307-192911

Are there any special permissions that are not provided on your phone? This was my result after turning off the WIFI at 7:22/23 pm and resuming at 7:28. It started almost immediately. Took around 10 seconds which is justifiable

You can see that the error notification showed up at 7:22 pm and in the second screenshot, my WIFI is on and the download progress had changed

@vincent-karuri
Copy link
Contributor

The test flow I mentioned happened after this #273 (comment).

Is that what you followed?

If it's still unclear, let's take do this in-person to save on time.

@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 8, 2019

The test flow I mentioned happened after this #273 (comment).

Is that what you followed?

If it's still unclear, let's take do this in-person to save on time.

Cool

Keep track of ConnectivityReceiver activation and only deactivate the ConnectivityReceiver onDestroy if the counter is greater than 0
@ekigamba
Copy link
Contributor Author

ekigamba commented Mar 8, 2019

Fixed the issue, the ConnectivityReceiver count was going -1 so on reactivating it would be 0 and not 1 which is the point at which the broadcast receiver is enabled

Copy link
Contributor

@vincent-karuri vincent-karuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix failing tests.

@ekigamba ekigamba merged commit 9a762b2 into master Mar 13, 2019
@ekigamba ekigamba deleted the issue/194-offline-map-download-resumption branch March 13, 2019 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants