Open
Conversation
TheMarex
force-pushed
the
patrick/geometry_by_leg
branch
from
dcd81c1 to
310708e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds support for returning geometry on a per-leg basis instead of at the route level by introducing a new overview=by_legs parameter option. This addresses a usability gap where users previously had to aggregate step geometries to get per-leg geometry, which was cumbersome when step-level instructions weren't needed.
Changes:
- Added new
ByLegsenum value toRouteParameters::OverviewTypeto support the new parameter - Updated Route, Match, and Trip APIs to populate leg-level geometry when
overview=by_legsis specified, while omitting route-level geometry - Extended FlatBuffer schema to support geometry fields on the
Legtable - Updated documentation across HTTP API docs, Node.js API docs, and inline code documentation
- Added comprehensive test coverage for both JSON and FlatBuffer formats across Route, Match, and Trip APIs
- Refactored CMakeLists.txt to use a proper custom target for test data generation
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| include/engine/api/route_parameters.hpp | Added ByLegs enum value to OverviewType |
| include/server/api/route_parameters_grammar.hpp | Added "by_legs" to HTTP parameter grammar |
| include/nodejs/node_osrm_support.hpp | Added "by_legs" support to Node.js bindings |
| src/nodejs/node_osrm.cpp | Updated Node.js API documentation to describe by_legs option |
| include/engine/api/route_api.hpp | Core implementation: generates leg geometries when ByLegs is set, skips route geometry |
| src/engine/api/json_factory.cpp | Modified makeRouteLeg to accept optional leg_geometry parameter |
| include/engine/api/json_factory.hpp | Updated function signature declarations (but has critical mismatch issue) |
| include/engine/api/flatbuffers/route.fbs | Added polyline and coordinates fields to Leg table |
| generated/include/engine/api/flatbuffers/route_generated.h | Auto-generated FlatBuffer code for new schema |
| unit_tests/library/route.cpp | Added comprehensive tests for route API with by_legs option |
| unit_tests/library/match.cpp | Added comprehensive tests for match API with by_legs option |
| unit_tests/library/trip.cpp | Added comprehensive tests for trip API with by_legs option |
| unit_tests/CMakeLists.txt | Refactored test data build to use custom target |
| test/nodejs/route.js | Updated error message expectations to include by_legs |
| docs/nodejs/api.md | Updated documentation to describe by_legs option |
| docs/http.md | Updated HTTP API documentation for Route, Match, and Trip endpoints |
| CHANGELOG.md | Added entry for the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
TheMarex
force-pushed
the
patrick/geometry_by_leg
branch
from
d845632 to
44857a6
Compare
…ead of having one for the whole route
- Update HTTP API docs for route, match, and trip services - Update Node.js API docs for route, match, and trip methods - Update test error messages to include by_legs option 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This is mostly relevant for library-tests which expects an extracted osrm dataset.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
TheMarex
force-pushed
the
patrick/geometry_by_leg
branch
from
3742ea7 to
56129ea
Compare
afarber
reviewed
Jan 28, 2026
| } | ||
|
|
||
| util::json::Object makeRouteLeg(guidance::RouteLeg leg, util::json::Array steps) | ||
| util::json::Object makeRouteLeg(guidance::RouteLeg leg, |
Contributor
There was a problem hiding this comment.
makeRouteLeg was removed from the header - should this be static or in an anonymous namespace now?
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
2 participants
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.
Issue
Right now it is kind of a pain to get the route geometry by leg. You can request the steps and accumulate it, but that is annoying if you are not interested in the instructions. With hindsight that wasn't the greatest API decision, alas better fix it late then never. In general it doesn't make sense to have it by route and by leg at the same time, so it seems appropriate to extend the
overviewoption for triggering the new behavior.Implementation
overview=by_legsparameter that will add the (unsimplified) geometry by leg.Tasklist