Add headings and correlated lat/lon to verbose matrix output #5072
Conversation
| @@ -242,7 +242,7 @@ bool CostMatrix::SourceToTarget(Api& request, | |||
|
|
|||
| // resize/reserve all properties of Matrix on first pass only | |||
| valhalla::Matrix& matrix = *request.mutable_matrix(); | |||
| reserve_pbf_arrays(matrix, best_connection_.size(), costing_->pass()); | |||
| reserve_pbf_arrays(matrix, best_connection_.size(), request.options().verbose(), costing_->pass()); | |||
There was a problem hiding this comment.
only reserve space for the new information when we know we will need it
| const auto shape = RecostFormPath(graphreader, best_connection, source_location_list[source_idx], | ||
| target_location_list[target_idx], source_idx, target_idx, | ||
| time_infos[source_idx], invariant, shape_format); | ||
| std::string shape; |
There was a problem hiding this comment.
small change, large diff: I pulled the path construction out of RecostFormPath (now that I spell it out, maybe this should be renamed to something like RecostFormShape), because we may need it for the headings and correlated lat/lon's. I figured this was better than the alternative, which would require passing a lot more args to the method, and making it do things beyond its original scope (i.e. forming part of the response).
There was a problem hiding this comment.
what if we kept this stuff down in FormPath but maybe we rename it to somethign else? Im actually ok with it still being named FormPath really, its the stuff you have to do after you do the expansion to make use of the paths that were found. in the case of other routing algorithms we just return the path itself (edgeids) but for matrix we are pretty much done so doing a bit of "serialization" is ok too. seems like we can just send the request API object to this method and modify it there to save on arguments to the method.
| const auto& target_edge = find_correlated_edge(target, path_edges.back()); | ||
| float source_pct = static_cast<float>(source_edge.percent_along()); | ||
| float target_pct = static_cast<float>(target_edge.percent_along()); | ||
| const ShapeFormat shape_format, |
There was a problem hiding this comment.
simplified signature
| @@ -1297,6 +1330,7 @@ std::string CostMatrix::RecostFormPath(GraphReader& graphreader, | |||
| auto is_first_edge = i == 0; | |||
| auto is_last_edge = i == (path_edges.size() - 1); | |||
|
|
|||
| graph_tile_ptr tile; | |||
There was a problem hiding this comment.
previously this reused the tile pointer used for forming the path.
Issue
Matrix results are frequently used as input for optimizing software, and detecting whether subsequent paths require u turns is currently not possible with the information provided in the response (unless you decide to get the shape too, which blows up with big matrices).
This PR introduces
begin/end_lat/lonandbegin/end_headingmembers for each connection in the verbose output to address this issue.I added beta labels to these in the docs, because there may be future requests to add more information to the verbose output, at which point it might be wise to look into something like #5063.
Tasklist