-
Notifications
You must be signed in to change notification settings - Fork 56
Upgrade osmdroid #52
base: master
Are you sure you want to change the base?
Upgrade osmdroid #52
Conversation
} catch (PackageManager.NameNotFoundException e) { | ||
e.printStackTrace(); | ||
} | ||
Configuration.getInstance().setUserAgentValue(context.getPackageName() + "/" + versionCode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as in MapActivity.java
e.printStackTrace(); | ||
} | ||
Configuration.getInstance().setUserAgentValue(context.getPackageName() + "/" + versionCode); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we really need the versionCode in the UserAgent + I'd use BuildConfig.APPLICATION_ID
which identifies the app, instead of context.getPackageName()
(the package name can be different from the application ID).
So I'd replace the whole block with:
Configuration.getInstance().setUserAgentValue(BuildConfig.APPLICATION_ID)
(as suggested in https://github.com/osmdroid/osmdroid/wiki/Important-notes-on-using-osmdroid-in-your-app#set-the-http-user-agent-variable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, yeah it's true that the osmdroid does not officially recommend to go as far as this. Actually, as stated in my commit message I relied on the osmdroid github thread that pointed the issue, and more precisely a comment of an OSM sysadmin called Firefishy.
My understanding of the whole bug is that, at some point of the time when there was a single user-agent for every osmdroid client, it has been banned by the OSM Foundation because of overloading problems in which it was involved. Now the new osmdroid requires the customization of the user-agent so OSM could ban only the ending client that overloads its servers. In this context using the APPLICATION_ID would be enough. That said, the recommendation of Firefishy was to use a different user-agent for each app version to prevent the whole package to be banned forever if a specific version behaves badly for some reason.
@@ -93,7 +106,7 @@ protected void onCreate(Bundle savedInstanceState) { | |||
map.setTileSource(TileSourceFactory.MAPNIK); | |||
break; | |||
case MAP_LAYER_CYCLEMAP: | |||
map.setTileSource(TileSourceFactory.CYCLEMAP); | |||
map.setTileSource(new ThunderforestTileSource(context, ThunderforestTileSource.CYCLE)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this source require an API key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I just made this change because the old code was not compiling, but it would be better to use a legit source instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have remove it. I can re-push with the removing unless you think about another source to add...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally re-pushed with another source (HikeBikeMap). Looking for your review. Regards.
This commit aims to get Mapnik working for newer Android versions, which is done by upgrading osmdroid to latest. This update implies some porting work : - use Double instead of int argument when calling setMinZoomLevel. - enhance osmdroid configuration : call 'load' to ensure config (github.com/osmdroid/osmdroid/wiki/How-to-use-the-osmdroid-library) and customize user-agent to access tiles following recommendations (github.com/osmdroid/osmdroid/wiki/Important-notes-on-using-osmdroid- in-your-app#set-the-http-user-agent-variable). - replace the discarded OpenCycleMap tile source by HikeBikeMap.
7a999f4
to
e1af67f
Compare
Hello ! Please find a new PR including your suggestions and the new osmdroid updated in the meantime. Regards. |
then close this PR ( in favor of which? ) |
Hi everyone. Sorry I missworded, actually I overwrited with a new commit, not a new PR 😛 Please let me know of any feedback about this. |
Hello.
It seems that Mapnik tiles is suffering download errors on recent Android versions (due to limitations raised by the OpenStreetMap administrators).
For example an Android 10 device face this issue (whereas Android 5 don't have it).
This PR do the following :
Changes have been tested on Android 10 : working ; Android 5 is still working.