do not crash demo for GraphHopper rerouting by karussell · Pull Request #201 · maplibre/maplibre-navigation-android

karussell · 2025-12-09T11:25:59Z

This change avoids the GraphHopper demo app crash when rerouting, i.e. fixes #168.

However, a potential different request is used for the GET request compared to the initial POST request, which is a general problem also for ValhallaNavigationActivity.

Note that the baseUrl (and so also graphhopper_url) has to end with '/'. Maybe this should be added in the comment of the developer xml config?

boldtrn · 2025-12-12T09:25:03Z

Note that the baseUrl (and so also graphhopper_url) has to end with '/'. Maybe this should be added in the comment of the developer xml config?

Yes, I think it would be good to mention that.

The change itself looks good to me.

However, a potential different request is used for the GET request compared to the initial POST request, which is a general problem also for ValhallaNavigationActivity

It's possible to work around this, but this would probably be too much for a small sample. We should allow to make the rerouting behavior easier to customize.

boldtrn · 2025-12-12T09:33:59Z

gradle/developer-config.gradle

+                "    <!-- Instead of valhalla you can use GraphHopper for the path finding. -->\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" +


I think you can't have these commented out, or the build will fail (as it is right now)

boldtrn

The build is failing with error:

e: file:///Users/runner/work/maplibre-navigation-android/maplibre-navigation-android/app/src/main/java/org/maplibre/navigation/android/example/GraphHopperNavigationActivity.kt:223:78 Unresolved reference 'graphhopper_key'.

Please double check if ./gradlew test succeeds locally.

karussell · 2025-12-12T10:21:53Z

I've done the requested changes.

Please note that the ValhallaNavigationActivity is still crashing for the same reason when rerouting.

Also note that not only the graphhopper_url but also the base_url and valhalla_url has to end with '/'. This seems to be a requirement of the SDK for when doing the rerouting:

java.lang.IllegalArgumentException: baseUrl must end in /: https://graphhopper.com/api/1/navigate
    at retrofit2.Retrofit$Builder.baseUrl(Retrofit.java:549)
    at retrofit2.Retrofit$Builder.baseUrl(Retrofit.java:492)
    at com.mapbox.core.MapboxService.getService(MapboxService.java:132)
    at com.mapbox.api.directions.v5.MapboxDirections.get(MapboxDirections.java:87)
    at com.mapbox.api.directions.v5.MapboxDirections.callForUrlLength(MapboxDirections.java:79)
    at com.mapbox.api.directions.v5.MapboxDirections.initializeCall(MapboxDirections.java:68)
    at com.mapbox.core.MapboxService.getCall(MapboxService.java:69)
    at com.mapbox.api.directions.v5.MapboxDirections.enqueueCall(MapboxDirections.java:181)
    at org.maplibre.navigation.android.navigation.ui.v5.route.NavigationRoute.getRoute(NavigationRoute.java:85)
    at org.maplibre.navigation.android.navigation.ui.v5.route.MapLibreRouteFetcher.findRouteWith(MapLibreRouteFetcher.java:164)
    at org.maplibre.navigation.android.navigation.ui.v5.NavigationViewRouter.findOnlineRouteWith(NavigationViewRouter.java:132)
    at org.maplibre.navigation.android.navigation.ui.v5.NavigationViewRouter.findRouteFrom(NavigationViewRouter.java:78)
    at org.maplibre.navigation.android.navigation.ui.v5.NavigationViewModel.handleOffRouteEvent(NavigationViewModel.java:418)
    at org.maplibre.navigation.android.navigation.ui.v5.NavigationViewModel.-$$Nest$mhandleOffRouteEvent(Unknown Source:0)
    at org.maplibre.navigation.android.navigation.ui.v5.NavigationViewModel$1.userOffRoute(NavigationViewModel.java:322)
    at org.maplibre.navigation.core.navigation.NavigationEventDispatcher.onUserOffRoute(NavigationEventDispatcher.kt:128)

boldtrn

Thanks for fixing this 👍

Copilot

Pull request overview

This PR aims to prevent the GraphHopper demo app from crashing during rerouting (issue #168) by ensuring the demo’s GraphHopper configuration provides the fields required by the reroute pipeline and by splitting the GraphHopper URL/key configuration.

Changes:

Update the generated developer-config.xml template to separate graphhopper_url and a new graphhopper_key.
Update GraphHopperNavigationActivity to append the GraphHopper key to the request URL and to populate RouteOptions so rerouting doesn’t immediately crash.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
gradle/developer-config.gradle	Adjusts generated developer config strings for GraphHopper (separate URL + key, trailing slash note).
app/src/main/java/org/maplibre/navigation/android/example/GraphHopperNavigationActivity.kt	Uses new `graphhopper_key` and updates `RouteOptions` intended to prevent reroute crashes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-09T20:12:28Z

app/src/main/java/org/maplibre/navigation/android/example/GraphHopperNavigationActivity.kt

+                                    baseUrl = getString(R.string.graphhopper_url),
+                                    profile = "car",
+                                    user = "gh",
+                                    accessToken = "pk." + getString(R.string.graphhopper_key),


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 · 2026-02-09T20:12:29Z

gradle/developer-config.gradle

+                "    <!-- 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" +


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 · 2026-02-09T20:12:29Z

app/src/main/java/org/maplibre/navigation/android/example/GraphHopperNavigationActivity.kt

            .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()))


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.

adapt rerouting GET request to avoid app crash

0d738f8

karussell mentioned this pull request Dec 9, 2025

Going Offroute with the GraphHopper sample crashes the app #168

Open

boldtrn reviewed Dec 12, 2025

View reviewed changes

boldtrn requested changes Dec 12, 2025

View reviewed changes

fix developer-config.gradle

d0c0260

boldtrn approved these changes Feb 9, 2026

View reviewed changes

boldtrn requested a review from Copilot February 9, 2026 20:06

Copilot started reviewing on behalf of boldtrn February 9, 2026 20:06 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

do not crash demo for GraphHopper rerouting#201

do not crash demo for GraphHopper rerouting#201
karussell wants to merge 2 commits intomaplibre:mainfrom
karussell:fix_168

karussell commented Dec 9, 2025

Uh oh!

boldtrn commented Dec 12, 2025

Uh oh!

boldtrn Dec 12, 2025

Uh oh!

boldtrn left a comment

Uh oh!

karussell commented Dec 12, 2025 •

edited

Loading

Uh oh!

boldtrn left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Copilot AI Feb 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-                                    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",

Uh oh!

Comments

Conversation

karussell commented Dec 9, 2025

Uh oh!

boldtrn commented Dec 12, 2025

Uh oh!

boldtrn Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

boldtrn left a comment

Choose a reason for hiding this comment

Uh oh!

karussell commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

boldtrn left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

karussell commented Dec 12, 2025 •

edited

Loading