Add road-following routing to the maps API - #5480
Open
shai-almog wants to merge 17 commits into
Open
Conversation
MapContainer.addPath and MapSurface.addPolyline join the coordinates they are given with straight segments; they know nothing about roads. Getting a line that follows the road network needs a routing service to work out the geometry first, and the maps API had none. Add com.codename1.maps.routing: * Routing - the entry point. showRoute(map, origin, destination) fetches the route, draws it and frames the camera; findRoute(...) hands back the result for styling and UI. * RouteRequest / TravelMode - waypoints, driving/walking/cycling, alternatives and a steps toggle. * Route / RouteLeg / RouteStep - drawable geometry (toPolyline()), bounds, total distance and duration, and the turn-by-turn breakdown. * RouteService / RouteCallback - the SPI. Routing.setService(...) swaps in Google Directions, Mapbox, GraphHopper or an in-house server without app code changing. * OsrmRouteService - the keyless default routing over OpenStreetMap data, mirroring how OpenFreeMap is the keyless tile default. Its default endpoint is OSRM's public demo server, so the docs are explicit that production apps should point it at their own instance. Also add PolylineCodec and Polyline.fromEncoded(...) for the encoded-polyline format (precision 5 and polyline6), which every directions API returns, so a geometry fetched by hand is one call from being drawable. Polyline's class doc now says outright that it draws straight segments and points at routing. The developer guide gains a "Routes that follow the road" section with include-backed snippets covering the zero-config call, the callback form and pointing the service at a self-hosted endpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 434533f1b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds first-class road-following routing support to the modern com.codename1.maps API by introducing a new com.codename1.maps.routing package with a default, keyless OSRM-backed implementation, plus an encoded-polyline codec to turn typical directions geometries into drawable Polylines. It also updates the developer guide with usage examples and adds unit tests for the routing facade, OSRM URL/response handling, and polyline encoding/decoding.
Changes:
- Introduces
Routingfacade + routing model/SPI (Route*,RouteRequest,TravelMode,RouteService,RouteCallback) with a defaultOsrmRouteService. - Adds
PolylineCodecandPolyline.fromEncoded(...)to support encoded polyline (precision 5 and 6) geometries. - Adds developer guide documentation and generated snippets, plus unit tests and SpotBugs exclusions.
Reviewed changes
Copilot reviewed 16 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| maven/core-unittests/src/test/java/com/codename1/maps/routing/MapsRoutingTest.java | Unit tests for routing model, OSRM URL/response parsing, and Routing facade behavior. |
| maven/core-unittests/src/test/java/com/codename1/maps/PolylineCodecTest.java | Unit tests for encoded polyline encode/decode (precision 5/6) and Polyline.fromEncoded. |
| maven/core-unittests/spotbugs-exclude.xml | Adds SpotBugs exclusion for ConnectionRequest subclasses used by routing. |
| docs/developer-guide/Maps.asciidoc | Documents “Routes that follow the road” and shows how to use/configure routing. |
| docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava005Snippet.java | Generated snippet: simplest Routing.showRoute(...) usage. |
| docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava006Snippet.java | Generated snippet: Routing.findRoute(...) with callback and custom styling/UI updates. |
| docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/MapsJava007Snippet.java | Generated snippet: configuring OsrmRouteService endpoint. |
| CodenameOne/src/com/codename1/maps/routing/TravelMode.java | Adds travel modes and wire IDs for routing backends. |
| CodenameOne/src/com/codename1/maps/routing/Routing.java | Adds routing facade methods (findRoute, showRoute) and default service management. |
| CodenameOne/src/com/codename1/maps/routing/RouteStep.java | Adds immutable step model (instruction, road name, distance/duration, geometry). |
| CodenameOne/src/com/codename1/maps/routing/RouteService.java | Adds routing service SPI contract. |
| CodenameOne/src/com/codename1/maps/routing/RouteRequest.java | Adds request model (origin/destination, waypoints, travel mode, steps, alternatives). |
| CodenameOne/src/com/codename1/maps/routing/RouteLeg.java | Adds leg model containing steps and aggregate metrics. |
| CodenameOne/src/com/codename1/maps/routing/RouteCallback.java | Adds callback interface for async route results/failures. |
| CodenameOne/src/com/codename1/maps/routing/Route.java | Adds route model (geometry, bounds, distance/duration, legs) + toPolyline(). |
| CodenameOne/src/com/codename1/maps/routing/package-info.java | Package-level documentation for routing API and default OSRM behavior. |
| CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java | Implements OSRM backend: URL construction, response parsing, and async networking. |
| CodenameOne/src/com/codename1/maps/PolylineCodec.java | Implements encoded polyline codec (precision 5/6) with truncation tolerance. |
| CodenameOne/src/com/codename1/maps/Polyline.java | Adds Polyline.fromEncoded(...) factory methods and clarifies straight-line semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Cloudflare Preview
|
Codex and Copilot both caught a real hang, plus three smaller issues: * Deliver the callback on transport failures. The request was marked failSilently, and NetworkManager swallows IOException/RuntimeException outright for silent requests -- handleException() never ran, so a client that lost DNS or TLS waited forever for a callback that never came, breaking the exactly-once contract RouteService advertises. The flag is gone (every failure hook here is overridden, so nothing reaches the default retry dialog anyway), and the request now also watches its own completion event as a backstop, covering a killed request or an app-wide error listener that consumes the event first. * Reject an encoded value cut in half. Exhausting the string mid-value returned the partial accumulator as if it were a complete delta, so dropping the final byte of the Google sample still emitted three points with the last longitude at -121.21 instead of -126.453 -- a spurious segment and skewed route bounds. readValue now reports whether it saw a terminating chunk and the point is dropped when it did not. * Return an unmodifiable view from RouteRequest.getWaypoints(), so a caller cannot slip a null or non-LatLng entry past addWaypoint() and blow up later inside buildUrl(). * Fix the showRoute() javadoc, which claimed the callback could restyle the drawn route. It cannot: that polyline is not exposed and toPolyline() returns a fresh instance each call. The doc now says so and points at findRoute() for styling. Also apply the complete Codename One license header to the six new files that carried a short or missing one, and rework the guide prose the Vale and LanguageTool gates flagged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad93d433fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit paired the failSilently fix with a per-request PROGRESS_TYPE_COMPLETED listener, meant to cover the one case the fix leaves open: an app-wide error listener that consumes the event, so NetworkManager never reaches req.handleIOException(). That backstop is racy and has to go. NetworkManager fires COMPLETED from its finally block for every attempt, including a redirect hop that is about to be retried (ConnectionRequest sets redirecting = true, calls retry() and returns false). Guarding on isRedirecting() does not help: progress events reach the listener through callSerially, so the check runs on the EDT after the network thread may already have started the retried attempt and reset the flag at the top of performOperationComplete. A redirect would then deliver a spurious failure and latch `delivered`, swallowing the real route that followed. Removing failSilently is the actual fix and stands on its own -- it restores delivery on every path NetworkManager exposes to the request. The remaining gap is ordinary ConnectionRequest behavior shared by the whole framework, so it is documented on RouteConnection rather than worked around with a racy listener. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b4d0966f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… one
parseResponse() is public and documents IOException for a malformed body,
but a non-object entry in routes/legs/steps escaped through the unchecked
casts, so a caller feeding it a response from its own transport crashed
past the documented catch:
{"code":"Ok","routes":[null]} -> NullPointerException
{"code":"Ok","routes":[7]} -> ClassCastException
Every object cast now goes through requireObject(value, what), which
raises IOException("Malformed routing response: <what> is not an object")
for route, leg and step entries; parseRoute and parseLeg propagate it. The
nested optional fields were already guarded -- maneuver behind
instanceof Map, location behind instanceof List, numerics through
number(), which falls back rather than throwing.
A malformed entry fails the parse rather than being skipped. Silently
dropping a leg or a step from a public parser hides the problem, and a
hard failure is what the documented contract promises. The javadoc now
states that malformed input never escapes as an unchecked exception, so
catching IOException is enough.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
Six review findings, all valid:
* The OSRM travel-mode documentation overclaimed. A stock osrm-routed
process serves the single dataset it was prepared with and ignores the
profile path component, so "a self-hosted instance with the matching
profiles honors them properly" was wrong -- one endpoint cannot serve
all three modes. The mode stays in the URL, since OSRM-compatible hosted
services do select on it, but the docs now say one service speaks to one
endpoint and show the per-mode instance pattern. TravelMode no longer
implies the backend honors whatever is asked.
* Narrow the SpotBugs exclusion to the literal
OsrmRouteService$RouteConnection. The ($.*)? form was copied from the
HttpTileSource entry, where the wildcard covers anonymous subclasses;
here it only risked hiding future findings on the service itself.
* Catch Exception rather than Throwable around the parse. An
OutOfMemoryError is a VM problem, not a routing failure, and disguising
it as one buries it.
* Build the failure message from getMessage() with a readable fallback
instead of concatenating the exception, which leaked class names into
text a caller may show a user. The throwable still reaches the callback.
* Always pass the throwable from the parse path. Nulling it for every
IOException conflated "OSRM declined to route" with "the body was
malformed" and discarded the detail worth logging for the second.
RouteCallback documents when error is null rather than that special case.
* Reject a precision outside 1 to 10 in PolylineCodec. Zero or negative
left the scale at 1 and inflated coordinates; a huge one overflowed the
scale and collapsed them to zero, both silently. Validated before the
empty-input short-circuits so the argument is checked regardless of
payload. A range rather than {5, 6} because precision 7 is in real use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
Collaborator
Author
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
The quality-report gate rejects UnnecessaryImport, and Routing's reference to Polyline became documentation-only once showRoute stopped naming the type. PMD does not read markdown doc comments, so the import counted as unused. Link the type by its fully qualified name in the doc instead. Worth noting for future changes: `mvn pmd:check` passes on this, which is why it slipped through locally. The gate that fails is .github/scripts/generate-quality-report.py, which parses target/pmd.xml against its own forbidden-rule list. Running it locally over the generated reports reproduces CI exactly, and now reports no findings at all in the new maps sources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 19 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java:420
- The
RouteServicecontract requires exactly one callback invocation per request, butRouteConnection.handleException()can be skipped if an app-wideNetworkManagererror listener consumes the exception event. In that caseNetworkManagerdoesn’t call the request’shandleIOException()/handleRuntimeException(), so this request never delivers a callback, which can leave callers waiting indefinitely.
A robust fix is to register a per-request NetworkManager error/progress listener (filtered to this request) that calls failLater(...) on error and unregisters itself on completion, so the routing callback is delivered even when the global error listener consumes the event.
/// One framework-level caveat remains, and it is the same for every
/// `ConnectionRequest`: an app-wide error listener registered through
/// [com.codename1.io.NetworkManager#addErrorListener] that *consumes* the
/// event has taken over failure reporting, and this request never hears
/// about it.
* OSRM reports maneuver.exit and the parser threw it away, so every multi-exit roundabout produced "Enter the roundabout and exit" -- useless to a driver. Instructions now read "At the roundabout take the 3rd exit onto Elm Street", with the teens handled, falling back to the old wording when no exit is reported rather than saying "0th". * Make Routing's OnceOnly wrapper actually enforce what findRoute documents. The latch is claimed under a lock, since a service that reports from a background thread or reports concurrently with throwing could otherwise race a plain field and deliver twice. Delivery is marshalled to the EDT rather than assuming the service got there, and always queued rather than run inline: a service answering synchronously would otherwise re-enter the caller before findRoute returned, and the timing would differ between services for no visible reason. * Fall back to "unknown" when a service reports a null or blank id, so an unavailable-service message does not quote null at the user. * Correct the MAX_PRECISION comment: 1e10 is not the largest power of ten a double represents exactly, it is simply well inside the exact range. * Move the StubProvider javadoc back onto StubProvider. Inserting tests above it had left it attached to the following test method. * Use WebMercator.MAX_LATITUDE in the test rather than repeating the literal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94692484e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
* OSRM's "roundabout turn" -- a small roundabout taken as an ordinary turn -- fell through to the generic wording, producing "Roundabout turn onto Main Street" and discarding the modifier that says which way to go. It is now treated as a turn, and "exit roundabout"/"exit rotary" get wording of their own instead of the same fallback. * legs and steps present as something other than a list were read as absent, so a bad response produced a half-populated route rather than the IOException parseResponse promises. Absent stays legitimate, since a steps=false request carries none. * Align the two fitBounds implementations on degenerate input. NativeMap returned NaN for zero-extent bounds -- keep the zoom, just recentre -- while the vector engine drove zoom to getMaxZoom(), so MapSurface# fitBounds still meant two different things for a single-point bounds. The vector engine now leaves the zoom alone in that case, matching native. * Treat padding that swallows the viewport as "nothing to fit" in both. Forcing the usable size up to one pixel solved for an absurd zoom instead of falling back to recentring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3f865e3b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 24 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CodenameOne/src/com/codename1/maps/vector/VectorMapEngine.java:282
- This changes degenerate-axis handling from the previous
getMaxZoom()cap toDouble.MAX_VALUE. IfsetZoom()doesn’t clamp internally, a bounds with zero span on one axis can now causefitBounds()to set a zoom higher than the engine supports (where previously it would cap at max zoom). Recommendation (moderate): explicitly clamp the computed zoom to the engine’s supported zoom range here (or revert the degenerate-axis cap togetMaxZoom()), to preserve bounded behavior and keep vector and native fit behavior consistent.
boolean roomToFit = viewWidth - 2 * padding > 0 && viewHeight - 2 * padding > 0;
if (roomToFit && (worldW > 0 || worldH > 0)) {
double usableW = (viewWidth - 2 * padding) / pixelRatio;
double usableH = (viewHeight - 2 * padding) / pixelRatio;
double zx = worldW <= 0 ? Double.MAX_VALUE : log2(usableW / worldW);
double zy = worldH <= 0 ? Double.MAX_VALUE : log2(usableH / worldH);
setZoom(Math.min(zx, zy));
}
…hopping * AppleMapProvider defined zoom as 360/2^zoom degrees, independent of the viewport, while MapSurface documents zoom as the standard slippy scale where a level spans 256 points and the visible degrees follow from the view's width. The two only agree on a 256-point-wide map. Fitting 10 degrees of longitude into a 512-point square therefore asked MapKit for a 5 degree span and clipped half the route -- and the same mismatch has always applied to setCamera and getZoom, so a zoom meant one thing on iOS and another everywhere else. zoomToSpan/spanToZoom now take the view width, read on the main thread where MapKit requires it, and fall back to 256 points when the view has not been laid out yet so the old behavior is preserved in that case. This is native code that cannot be exercised from a unit test or the simulator; it needs a device check on an iOS build. * Claim the delivery in failLater before queueing the EDT hop rather than inside it. Nothing observed a wrong outcome -- callSerially is FIFO and the failure is queued before the completion event -- but leaving the claim until the hop ran made correctness depend on that queue ordering. Claiming up front makes it local, and lets the backstop skip allocating a runnable when the delivery is already spoken for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The copyright gate requires the complete Codename One GPLv2 + Classpath header on every file a change touches; the provider template predates the gate and so had only its descriptive comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 25 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java:343
parseLocation()usesnumber(...), which returns0on non-numeric/unparseable values. That can silently turn a malformed OSRM location into a real(0,0)coordinate (null island), which is misleading and can skew UI logic that relies onRouteStep.getStart(). Consider validating that both entries are numeric (or parse cleanly) and returningnull(or throwingIOExceptionif you want to treat this as a malformed response) when either value can’t be decoded.
private static LatLng parseLocation(Object location) {
if (!(location instanceof List) || ((List) location).size() < 2) {
return null;
}
List pair = (List) location;
return new LatLng(number(pair.get(1)), number(pair.get(0)));
}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ffa69e6fac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 25 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
CodenameOne/src/com/codename1/maps/routing/Routing.java:258
routes.get(0)is cast toRoutewithout validating the element type. If a third-partyRouteServiceviolates the SPI contract and returns a list containing a non-Routeelement (ornull), this will throw on the EDT and skip invoking the user callback (and potentially break the “exactly once” guarantee from the caller’s perspective). Suggested fix: checkroutes.get(0) instanceof Route(and non-null) and callrouteFailed(\"The routing service returned an invalid route\", ...)when invalid, similar to the existing empty-list guard.
if (routes == null || routes.isEmpty()) {
// A service that breaks the contract shouldn't surface as
// an IndexOutOfBoundsException in the middle of the EDT.
routeFailed("The routing service returned no route", null);
return;
}
Route best = (Route) routes.get(0);
map.addPolyline(best.toPolyline());
if (best.getBounds() != null) {
map.fitBounds(best.getBounds(), CN.convertToPixels(4));
}
showRoute guarded null and empty route lists but still cast the first element blind, so a service handing back [null] or a non-Route threw on the EDT with nothing reported to the application. It now fails the same way the empty case does; two tests cover both halves and fail without the guard. The Apple provider read MKMapView state (bounds, region, centerCoordinate, coordinate conversions) on whatever thread called in. Reads now go through a cn1RunOnMain helper that runs inline when already on the main thread -- the iOS EDT is the main thread, so dispatch_sync would deadlock -- and marshals otherwise. getZoom also takes the span and the width in one hop, so a resize between them cannot convert one region against another's width. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 25 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
CodenameOne/src/com/codename1/maps/routing/OsrmRouteService.java:648
RouteCallback's contract sayserrorshould benullwhen the failure is just a service-level status (e.g. OSRMcodelikeNoRoute/NoSegment). HereparseResponse()throws anIOExceptionfor those statuses, and this catch block forwards that exception object to the app, which makes service-level failures look like thrown errors and contradicts the callback docs.
} catch (Exception e) {
// Deliberately not Throwable: an OutOfMemoryError or
// StackOverflowError is a VM problem, not a routing failure,
// and disguising it as one only makes it harder to find.
routes = null;
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ac451c0c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!claim()) { | ||
| return; | ||
| } | ||
| detach(); |
There was a problem hiding this comment.
When this request owns the last global progress listener, a successful postResponse() can execute on the EDT and call detach() here while the network thread is entering NetworkManager.runCurrentRequest()'s finally block. That block checks progressListeners != null at NetworkManager.java:1133 but dereferences the field again at line 1134; if this removal clears it between those operations, the resulting NullPointerException escapes NetworkThread.run() and terminates the default single network worker, leaving subsequent networking stuck. Defer this removal until after the completion dispatch, or make that framework dispatch use a stable local dispatcher reference.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5478.
The report
MapContainer.addPathdraws a straight line between the coordinates instead of the route a driver would take.That is what a polyline does —
addPath/MapSurface.addPolylinejoins the vertices it is given with straight segments and has no knowledge of roads. Turning two endpoints into road geometry is a separate job, done by a routing service, and the maps API had none. (MapContaineris the deprecatedcodenameone-google-mapscn1lib; the guide already points at the modernMapView/NativeMap, so the fix lands there.)What this adds
A new
com.codename1.maps.routingpackage:Routing— the entry point.Routing.showRoute(map, origin, destination)fetches the route, draws it and frames the camera.findRoute(...)hands back the result when you want to style the line or show distance/ETA.RouteRequest/TravelMode— waypoints,DRIVING/WALKING/CYCLING, alternatives, and a steps toggle.Route/RouteLeg/RouteStep— drawable geometry (toPolyline()),getBounds()forfitBounds, total distance and duration, and the turn-by-turn breakdown with generated instruction text.RouteService/RouteCallback— the SPI.Routing.setService(...)swaps in Google Directions, Mapbox, GraphHopper or an in-house server without app code changing.OsrmRouteService— the keyless default, routing over OpenStreetMap data. This mirrors how OpenFreeMap is the keyless tile default: a road route works in the simulator and on device with no API key and no signup.Plus
PolylineCodecandPolyline.fromEncoded(...)for the encoded-polyline format (precision 5 andpolyline6) that every directions API returns, so a geometry you fetched yourself is one call from being drawable.Polyline's class doc now states outright that it draws straight segments, and points at routing.The reporter's case becomes:
On the default endpoint
OsrmRouteServicedefaults to OSRM's public demo server: no SLA, rate limited, and it hosts the car profile so walking/cycling requests route over driving data there. The class docs, the package docs and the developer guide all say this explicitly and show the one-line switch to a self-hosted (or any OSRM-compatible) endpoint:Docs
New "Routes that follow the road" section in
docs/developer-guide/Maps.asciidoc, with three include-backed snippets: the zero-config call, the callback form, and pointing the service at your own endpoint.Verification
PolylineCodecTest,MapsRoutingTest) covering the codec against the Google specification vector, truncated-input tolerance, OSRM URL construction (lon/lat ordering and fixed-decimal formatting that keeps scientific notation out of the URL), response parsing, service-level error codes, and the facade.total_bugs=0) and PMD clean. OneEQ_DOESNT_OVERRIDE_EQUALSexclusion added for the one-shotConnectionRequestsubclass, mirroring the existingHttpTileSourceentry.🤖 Generated with Claude Code