Skip to content

Optional OpenLR Encoded Path Edges in API Response#2424

Merged
mookerji merged 2 commits intomasterfrom
mookerji-openlr-shape
Jul 6, 2020
Merged

Optional OpenLR Encoded Path Edges in API Response#2424
mookerji merged 2 commits intomasterfrom
mookerji-openlr-shape

Conversation

@mookerji
Copy link
Copy Markdown
Contributor

@mookerji mookerji commented Jun 17, 2020 •
edited
Loading

Adds support for a new request option 'linear_references=true|false', which adds an array of OpenLR references to the response body for route and trace_route Valhalla HTTP endpoints.

@mookerji mookerji requested a review from kevinkreiser June 17, 2020 02:25
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from 583aa26 to 35ce208 Compare June 17, 2020 20:20
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 18, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from a30e753 to 4026677 Compare June 23, 2020 00:50
@mookerji
Copy link
Copy Markdown
Contributor Author

mookerji commented Jun 23, 2020 •
edited
Loading

Per off-Github discussion, moved this from the geometry field to a new field in the map matching response that returns a single OpenLR descriptor per edge. A few more things:

  • Only a small subset of clients need such descriptors, so I've also added an optional request flag; otherwise, the default response size will increase quite a bit.
  • I've added support for both OpenLR per edge as well as OpenLR per leg, largely so we can do a size comparison on responses. I'm assuming that we want a descriptor per-edge in lieu of some kind of canonical edge ID.
  • Following up to add some new test coverage.

Example output:

$ curl --data '{"shape":[{"lat":"37.7826603","lon":"-122.4708554","type":"break","radius":15,"search_cutoff":21000000,"preferred_side":"either"},{"lat":"37.7826734","lon":"-122.4706716","type":"break","radius":15,"search_cutoff":21000000,"preferred_side":"either"},{"lat":"37.7826703","lon":"-122.4705267","type":"break","radius":15,"search_cutoff":21000000,"preferred_side":"either"}],"costing":"auto","date_time":{"type":0},"shape_match":"map_snap","format":"osrm","shape_format":"geojson","filters":{},"include_linear_reference": true, "trace_options":{"interpolation_distance":0},"force_traffic":false,"directions_type":"instructions","units":"miles"}' localhost:8004/trace_route
{
  "matchings": [
    {
      "weight_name": "Valhalla default",
      "weight": 2.608,
      "legs": [
      ....        
      ],
      "duration": 2.608,
      "distance": 28.979,
      "geometry": {
        "coordinates": [
          [
            -122.470856,
            37.782661
          ],
          [
            -122.470673,
            37.782669
          ],
          [
            -122.470528,
            37.782677
          ]
        ],
        "type": "LineString"
      },
      "linear_reference": [
        "C6jo3BreID/nAAASAAE/Fw==",
        "C6jo3BreID/gAAAAAAA/EA==",
        "C6jo5RreID/nAAANAAI/Fw==",
        "C6jo5RreID/gAP//AAE/EA=="
      ],
      "confidence": 1
    }
  ],
  "tracepoints": [
    {
      "matchings_index": 0,
      "name": "",
      "waypoint_index": 0,
      "alternatives_count": 0,
      "distance": 0,
      "location": [
        -122.470856,
        37.782661
      ]
    },
    {
      "matchings_index": 0,
      "name": "",
      "waypoint_index": 1,
      "alternatives_count": 0,
      "distance": 0.425,
      "location": [
        -122.470673,
        37.782669
      ]
    },
    {
      "matchings_index": 0,
      "name": "",
      "waypoint_index": 2,
      "alternatives_count": 0,
      "distance": 0.85,
      "location": [
        -122.470528,
        37.782677
      ]
    }
  ],
  "code": "Ok"
}

@mookerji mookerji changed the title OpenLR encoding for route geometries. Optional OpenLR encoding for trace route edge IDs. Jun 23, 2020
@mookerji mookerji force-pushed the mookerji-openlr-shape branch 2 times, most recently from 6b8d367 to 48f2b26 Compare June 24, 2020 05:20
@mookerji mookerji requested a review from kevinkreiser June 24, 2020 05:27
@mookerji mookerji self-assigned this Jun 24, 2020
@mookerji mookerji marked this pull request as ready for review June 24, 2020 05:28
@mookerji mookerji force-pushed the mookerji-openlr-shape branch 4 times, most recently from 187ffce to 5e49014 Compare June 26, 2020 02:58
mookerji
mookerji commented Jun 26, 2020
View reviewed changes
Comment thread test/util_midgard.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread docs/api/map-matching/api-reference.md Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread proto/options.proto Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread src/midgard/util.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread src/tyr/route_serializer_osrm.cc Outdated
kevinkreiser
kevinkreiser reviewed Jun 26, 2020
View reviewed changes
Comment thread test/mapmatch.cc Outdated
@mookerji mookerji force-pushed the mookerji-openlr-shape branch 2 times, most recently from a19d85f to 49f557d Compare June 30, 2020 01:52
@mookerji mookerji requested a review from kevinkreiser June 30, 2020 02:41
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from 6725892 to 730185c Compare June 30, 2020 22:20
@mookerji
Copy link
Copy Markdown
Contributor Author

mookerji commented Jun 30, 2020

This isochrone unit test failure is deterministic, but seems out-of-scope of the current change. Checking this out right now, but any other feedback you have @kevinkreiser would be great.

kevinkreiser
kevinkreiser reviewed Jul 1, 2020
View reviewed changes
Comment thread src/midgard/openlr.cc Outdated
kevinkreiser
kevinkreiser reviewed Jul 1, 2020
View reviewed changes
Comment thread test/predictedspeeds.cc Outdated
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from 730185c to d8544be Compare July 1, 2020 20:30
@mookerji mookerji mentioned this pull request Jul 1, 2020
Merged
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from d8544be to 9c17220 Compare July 2, 2020 00:46
@mookerji
Copy link
Copy Markdown
Contributor Author

mookerji commented Jul 2, 2020

Factored out the base64 stuff into: https://github.com/valhalla/valhalla/pull/2452/files. Once we get that merged, we can rebase this OpenLR PR on master and review again.

@kevinkreiser kevinkreiser changed the title Optional OpenLR encoding for trace route edge IDs. Optional OpenLR Encoded Path Edges in API Response Jul 6, 2020
mookerji pushed a commit that referenced this pull request Jul 6, 2020
Fix: Base64 encoding/decoding
0a2fa52 
- Moves base64 decoding into proper header file, adds unit test coverage.
- Fix bug in base64 decoder that failed to remove padding bytes.
- Updates test fixtures impacted by improper base64 decoding.

Split out from:
- #2424

/cc @kevinkreiser
mookerji pushed a commit that referenced this pull request Jul 6, 2020
Fix: Base64 encoding/decoding (#2452)
4d1ccc6 
* Fix: Base64 encoding/decoding

- Moves base64 decoding into proper header file, adds unit test coverage.
- Fix bug in base64 decoder that failed to remove padding bytes.
- Updates test fixtures impacted by improper base64 decoding.

Split out from:
- #2424

/cc @kevinkreiser
@mookerji mookerji force-pushed the mookerji-openlr-shape branch from 9c17220 to 0700ff1 Compare July 6, 2020 18:55
@mookerji mookerji requested a review from kevinkreiser July 6, 2020 18:58
@mookerji mookerji merged commit d530889 into master Jul 6, 2020
@mookerji mookerji mentioned this pull request Jul 6, 2020
Merged
mookerji pushed a commit that referenced this pull request Jul 9, 2020
openlr: Explicitly check for linear reference option for Valhalla ser…
74964ca 
…ialization.

In a prior PR (link below) we added OpenLR encoding for trace_route edges but apparently did not
check for the API option, defaulting to adding OpenLR linear references to the response. This fixes
this issue and adds the options checking to the library function filling out references.

#2424

/cc @kevinkreiser
mookerji pushed a commit that referenced this pull request Jul 9, 2020
openlr: Explicitly check for linear reference option for Valhalla ser…
ac08a25 
…ialization.

In a prior PR (link below) we added OpenLR encoding for trace_route edges but apparently did not
check for the API option, defaulting to adding OpenLR linear references to the response. This fixes
this issue and adds the options checking to the library function filling out references.

#2424

/cc @kevinkreiser
mookerji pushed a commit that referenced this pull request Jul 10, 2020
openlr: Explicitly check for linear reference option for Valhalla ser…
75cb099 
…ialization (#2458)

* openlr: Explicitly check for linear reference option for Valhalla serialization.

In a prior PR (link below) we added OpenLR encoding for trace_route edges but apparently did not
check for the API option, defaulting to adding OpenLR linear references to the response. This fixes
this issue and adds the options checking to the library function filling out references.

Follow up to: 
#2424

/cc @kevinkreiser
@nilsnolde nilsnolde deleted the mookerji-openlr-shape branch February 24, 2024 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kevinkreiser kevinkreiser kevinkreiser approved these changes

Assignees

@mookerji mookerji

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mookerji @kevinkreiser