Skip to content

do not crash demo for GraphHopper rerouting #201

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.

Sign up for GitHub

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

Open
karussell wants to merge 2 commits into
base: main
Choose a base branch
from
+10 −8
Open

do not crash demo for GraphHopper rerouting #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d738f8
adapt rerouting GET request to avoid app crash
karussell Dec 9, 2025
d0c0260
fix developer-config.gradle
karussell Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions app/src/main/java/org/maplibre/navigation/android/example/GraphHopperNavigationActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ class GraphHopperNavigationActivity :
// Create request object. Requires graphhopper_url to be set in developer-config.xml
val request = Request.Builder()
.header("User-Agent", "MapLibre Android Navigation SDK Demo App")
.url(getString(R.string.graphhopper_url))
.url(getString(R.string.graphhopper_url) + "?key=" + getString(R.string.graphhopper_key))
.post(requestBodyJson.toRequestBody("application/json; charset=utf-8".toMediaType()))
Comment on lines 189 to 191
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Building the GraphHopper request URL via string concatenation is brittle: it will produce an invalid URL if graphhopper_url already contains a query string, and it doesn't URL-encode the key. Prefer constructing the URL with OkHttp's HttpUrl (parse + newBuilder + addQueryParameter) so key is appended correctly regardless of trailing slashes or existing parameters.

Copilot uses AI. Check for mistakes.
.build()

Expand Down Expand Up @@ -215,11 +215,12 @@ class GraphHopperNavigationActivity :
.first()
.copy(
routeOptions = RouteOptions(
// See ValhallaNavigationActivity why these dummy route options are necessary
baseUrl = "https://valhalla.routing",
profile = "valhalla",
user = "valhalla",
accessToken = "valhalla",
// Used for rerouting. See #201.
// TODO: problematic as not the original POST request is used, see #168
baseUrl = getString(R.string.graphhopper_url),
profile = "car",
user = "gh",
accessToken = "pk." + getString(R.string.graphhopper_key),
Comment on lines +220 to +223
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

These RouteOptions are used by the reroute pipeline, which is implemented via NavigationRoute/MapboxDirections (see MapLibreRouteFetcher). Setting baseUrl to the GraphHopper /navigate/ endpoint and using user="gh" / profile="car" will cause reroute requests to be formed as /directions/v5/gh/car/..., which is not a valid endpoint for either Mapbox or GraphHopper and effectively guarantees rerouting will fail. If the intent is only to avoid crashes, consider using Mapbox-compatible user/profile values and a base URL intended for MapboxDirections (ending with /), while keeping the dummy accessToken to satisfy token-format validation.

Suggested change
baseUrl = getString(R.string.graphhopper_url),
profile = "car",
user = "gh",
accessToken = "pk." + getString(R.string.graphhopper_key),
// Use Mapbox-compatible settings so NavigationRoute/MapboxDirections
// constructs syntactically valid reroute URLs, with a dummy token.
baseUrl = "https://api.mapbox.com/",
profile = "driving",
user = "mapbox",
accessToken = "pk.00000000000000000000000000000000",

Copilot uses AI. Check for mistakes.
voiceInstructions = true,
bannerInstructions = true,
language = language,
Expand Down
5 changes: 3 additions & 2 deletions gradle/developer-config.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ task accessToken {
" <!-- Your valhalla url (example: https://valhalla1.openstreetmap.de/route) -->\n" +
" <!-- Don't use the following server in production, it is for demonstration purposes only: -->\n" +
" <string name=\"valhalla_url\" translatable=\"false\">https://valhalla1.openstreetmap.de/route</string>\n" +
" <!-- Instead of valhalla you can use GraphHopper for the path finding. Example: https://graphhopper.com/api/1/navigate?key=YOUR_API_KEY or a local server -->\n" +
" <!-- Instead of valhalla you can use GraphHopper for the path finding. Note that the graphhopper_url has to end with '/'. (#201) -->\n" +
" <!-- Don't use the following API key in production, it is for demonstration purposes only: -->\n" +
" <string name=\"graphhopper_url\" translatable=\"false\">https://graphhopper.com/api/1/navigate?key=7088b84f-4cee-4059-96de-fd0cbda2fdff</string>\n" +
" <string name=\"graphhopper_key\" translatable=\"false\">7088b84f-4cee-4059-96de-fd0cbda2fdff</string>\n" +
" <string name=\"graphhopper_url\" translatable=\"false\">https://graphhopper.com/api/1/navigate/</string>\n" +
Comment on lines +20 to +23
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

Introducing a new required string resource (graphhopper_key) will break builds for anyone who already has an existing untracked developer-config.xml generated by older versions, because this Gradle task only writes the file when it does not exist. Consider updating the task to detect/migrate an existing file (e.g., add graphhopper_key and normalize graphhopper_url), or fail early with a clear, actionable message telling the developer to delete/regenerate the file.

Copilot uses AI. Check for mistakes.
" <!-- Your Mapbox access token (example: pk.abc...) -->\n" +
" <string name=\"mapbox_access_token\" translatable=\"false\">" + mapboxAccessToken + "</string>\n" +
" <!-- Map tile provider for light design (example: https://api.maptiler.com/maps/basic-v2/style.json?key=...) -->\n" +
Expand Down