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

Multi download #1901

Merged
merged 66 commits into from
Oct 29, 2020
Merged

Multi download #1901

merged 66 commits into from
Oct 29, 2020

Conversation

westnordost
Copy link
Member

No description provided.

@smichel17
Copy link
Member

Follow-up question: how large is the stored quest data? Does it (or should it) interact with the "map cache size" setting?

@westnordost
Copy link
Member Author

It doesn't. Quests are not considered part of the map. Map cache is only for the background map. Unsolved quests are

@smichel17
Copy link
Member

Unsolved quests are

I think you accidentally

@westnordost
Copy link
Member Author

... deleted automatically after a certain interval

now does not store per quest type, but per download type. There is currently only one download type, "quests"
@westnordost westnordost marked this pull request as ready for review October 28, 2020 23:50
@westnordost
Copy link
Member Author

Hmm, I think I am done.

Anyone want to review the code or give it a go in a test?

I implemented what @smichel17 and @matkoniecz suggested. On my Samsung S10e, download is so fast that after I tapped through the tutorial, all the quests are already there. 5s for initial scan (zoom 16 tile around my location - 360x360m) yielding 140 quests, then another 8 seconds for the download in 600m radius around my location, yielding another 2200 quests.

The download logic works like that: First download the z16 tile which encloses the user's location (around 360x360m), then from that, calculate the quest density. The download strategy when in WiFi targets downloading around 1000 quests around the user, so it calculates the estimated download radius based on the measured quest density and then issues another download. In my case, that was around 600m. The mobile data download strategry aims at having around 500 quests around the user, so downloaded radii will be smaller.

@westnordost
Copy link
Member Author

westnordost commented Oct 29, 2020

Interesting classes:

Copy link
Member

@smichel17 smichel17 left a comment

Choose a reason for hiding this comment

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

I can't comment on the parsing without spending more time learning OSM details than I want, but I found one potential issue.

@westnordost
Copy link
Member Author

Thanks for trying it out, @smichel17 !

@westnordost
Copy link
Member Author

westnordost commented Oct 29, 2020

Comment on lines +60 to +62
/** exclude buildings that intersect with the bounding box because it is not possible to
ascertain for these if there is an address node within the building - it could be outside
the bounding box */
Copy link
Member Author

@westnordost westnordost Oct 29, 2020

Choose a reason for hiding this comment

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

Unfortunately, the problem here is that in small bounding boxes, quite a few buildings will be excluded because of this. If a housenumber quest was downloaded before as part of a big bounding box and then another small download is being executed, unsolved housenumber quests on buildings on the edge will even vanish.

The prior solution was to expand the bounding box to include all the buildings and then download the housenumber nodes based on that enlarged bounding box. With the solution now, this is not possible (or: not expedient) anymore - we have that one bounding box with the data and we should create all the quests based on the downloaded data.

# Conflicts:
#	app/src/main/java/de/westnordost/streetcomplete/quests/QuestModule.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddCyclewayPartSurface.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddFootwayPartSurface.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddPathSurface.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurface.kt
#	app/src/main/java/de/westnordost/streetcomplete/quests/surface/DetailRoadSurface.kt
@westnordost westnordost merged commit 761f286 into master Oct 29, 2020
@westnordost westnordost deleted the multi-download branch October 29, 2020 23:53
@mnalis mnalis mentioned this pull request Nov 9, 2020
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.

4 participants