fix(openlr): Fixes km to meter conversion in OpenLr serializer (valha…

…lla#2688) * fix(openlr): Fixes km to meter conversion in OpenLr serializer Also renames TripLeg.Edge.length to TripLeg.Edge.length_km * Updates changelog
CVTS · Nov 17, 2020 · 852f54c · 852f54c
1 parent 387577e
commit 852f54c
Show file tree

Hide file tree

Showing 14 changed files with 35 additions and 34 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -97,6 +97,7 @@
  * FIXED: Overestimated reach caused be reenquing transition nodes without checking that they had been already expanded [#2670](https://github.com/valhalla/valhalla/pull/2670)
  * FIXED: Build with C++17 standard. Deprecated function calls are substituted with new ones. [#2669](https://github.com/valhalla/valhalla/pull/2669)
  * FIXED: Improve German post_transition_verbal instruction [#2677](https://github.com/valhalla/valhalla/pull/2677)
+ * FIXED: Fixes OpenLr serialization [#2688](https://github.com/valhalla/valhalla/pull/2688)
 
 * **Enhancement**
  * ADDED: Add ability to provide custom implementation for candidate collection in CandidateQuery. [#2328](https://github.com/valhalla/valhalla/pull/2328)

diff --git a/proto/trip.proto b/proto/trip.proto
@@ -186,7 +186,7 @@ message TripLeg {
 
  message Edge {
  repeated StreetName name = 1; // street names
- optional float length = 2;  // km
+ optional float length_km = 2; // km
  optional float speed = 3; // km/h
  optional RoadClass road_class = 4;
  optional uint32 begin_heading = 5; // 0-359

diff --git a/src/odin/directionsbuilder.cc b/src/odin/directionsbuilder.cc
@@ -127,19 +127,19 @@ void DirectionsBuilder::UpdateHeading(EnhancedTripLeg* etp) {
  min_edge_length = kMinPedestrianBicycleEdgeLength;
  }
 
- if (curr_edge && (curr_edge->length() < min_edge_length)) {
+ if (curr_edge && (curr_edge->length_km() < min_edge_length)) {
 
  // Set the current begin heading
- if (prev_edge && (prev_edge->length() >= min_edge_length)) {
+ if (prev_edge && (prev_edge->length_km() >= min_edge_length)) {
  curr_edge->set_begin_heading(prev_edge->end_heading());
- } else if (next_edge && (next_edge->length() >= min_edge_length)) {
+ } else if (next_edge && (next_edge->length_km() >= min_edge_length)) {
  curr_edge->set_begin_heading(next_edge->begin_heading());
  }
 
  // Set the current end heading
- if (next_edge && (next_edge->length() >= min_edge_length)) {
+ if (next_edge && (next_edge->length_km() >= min_edge_length)) {
  curr_edge->set_end_heading(next_edge->begin_heading());
- } else if (prev_edge && (prev_edge->length() >= min_edge_length)) {
+ } else if (prev_edge && (prev_edge->length_km() >= min_edge_length)) {
  curr_edge->set_end_heading(prev_edge->end_heading());
  }
  }

diff --git a/src/odin/enhancedtrippath.cc b/src/odin/enhancedtrippath.cc
@@ -276,7 +276,7 @@ float EnhancedTripLeg::GetLength(const Options::Units& units) {
  float length = 0.0f;
  for (const auto& n : node()) {
  if (n.has_edge()) {
- length += n.edge().length();
+ length += n.edge().length_km();
  }
  }
  if (units == Options::miles) {
@@ -470,9 +470,9 @@ std::vector<std::pair<std::string, bool>> EnhancedTripLeg_Edge::GetNameList() co
 
 float EnhancedTripLeg_Edge::GetLength(const Options::Units& units) {
  if (units == Options::miles) {
- return (length() * kMilePerKm);
+ return (length_km() * kMilePerKm);
  }
- return length();
+ return length_km();
 }
 
 bool EnhancedTripLeg_Edge::HasActiveTurnLane() const {
@@ -653,7 +653,7 @@ std::string EnhancedTripLeg_Edge::ToString() const {
  }
 
  str += " | length=";
- str += std::to_string(length());
+ str += std::to_string(length_km());
 
  str += " | speed=";
  str += std::to_string(speed());
@@ -994,7 +994,7 @@ std::string EnhancedTripLeg_Edge::ToParameterString() const {
  str += StreetNamesToParameterString(this->name());
 
  str += delim;
- str += std::to_string(length());
+ str += std::to_string(length_km());
 
  str += delim;
  str += std::to_string(speed());

diff --git a/src/odin/maneuversbuilder.cc b/src/odin/maneuversbuilder.cc
@@ -1031,13 +1031,13 @@ void ManeuversBuilder::UpdateManeuver(Maneuver& maneuver, int node_index) {
  UpdateInternalTurnCount(maneuver, node_index);
 
  // Distance in kilometers
- maneuver.set_length(maneuver.length() + prev_edge->length());
+ maneuver.set_length(maneuver.length() + prev_edge->length_km());
 
  // Basic time (len/speed on each edge with no stop impact) in seconds
  // TODO: update GetTime and GetSpeed to double precision
  maneuver.set_basic_time(
  maneuver.basic_time() +
- GetTime(prev_edge->length(), GetSpeed(maneuver.travel_mode(), prev_edge->default_speed())));
+ GetTime(prev_edge->length_km(), GetSpeed(maneuver.travel_mode(), prev_edge->default_speed())));
 
  // Portions Toll
  if (prev_edge->toll()) {
@@ -2119,7 +2119,7 @@ bool ManeuversBuilder::IsLeftPencilPointUturn(int node_index,
  // and oneway edges
  if (curr_edge->drive_on_right() &&
  (((turn_degree > 179) && (turn_degree < 211)) ||
- (((prev_edge->length() < 50) || (curr_edge->length() < 50)) && (turn_degree > 179) &&
+ (((prev_edge->length_km() < 50) || (curr_edge->length_km() < 50)) && (turn_degree > 179) &&
  (turn_degree < 226))) &&
  prev_edge->IsOneway() && curr_edge->IsOneway()) {
  // If the above criteria is met then check the following criteria...
@@ -2162,7 +2162,7 @@ bool ManeuversBuilder::IsRightPencilPointUturn(int node_index,
  // and oneway edges
  if (curr_edge->drive_on_right() &&
  (((turn_degree > 149) && (turn_degree < 181)) ||
- (((prev_edge->length() < 50) || (curr_edge->length() < 50)) && (turn_degree > 134) &&
+ (((prev_edge->length_km() < 50) || (curr_edge->length_km() < 50)) && (turn_degree > 134) &&
  (turn_degree < 181))) &&
  prev_edge->IsOneway() && curr_edge->IsOneway()) {
  // If the above criteria is met then check the following criteria...
@@ -2857,7 +2857,7 @@ void ManeuversBuilder::ProcessTurnLanes(std::list<Maneuver>& maneuvers) {
  float remaining_step_distance = 0.f;
  // Add the prev_edge length as we skip it in the loop below
  if (prev_edge) {
- remaining_step_distance += prev_edge->length();
+ remaining_step_distance += prev_edge->length_km();
  }
 
  // Assign turn lanes within step, walking backwards from end to begin node
@@ -2899,7 +2899,7 @@ void ManeuversBuilder::ProcessTurnLanes(std::list<Maneuver>& maneuvers) {
  }
 
  // Update the remaining step distance
- remaining_step_distance += prev_edge->length();
+ remaining_step_distance += prev_edge->length_km();
  }
  }
  }
@@ -3104,12 +3104,12 @@ void ManeuversBuilder::MoveInternalEdgeToPreviousManeuver(Maneuver& prev_maneuve
  // Update the previous maneuver
 
  // Increase distance in kilometers
- prev_maneuver.set_length(prev_maneuver.length() + edge->length());
+ prev_maneuver.set_length(prev_maneuver.length() + edge->length_km());
 
  // Increase basic time (len/speed on each edge with no stop impact) in seconds
  prev_maneuver.set_basic_time(
  prev_maneuver.basic_time() +
- GetTime(edge->length(), GetSpeed(prev_maneuver.travel_mode(), edge->default_speed())));
+ GetTime(edge->length_km(), GetSpeed(prev_maneuver.travel_mode(), edge->default_speed())));
 
  // Set the end node index
  prev_maneuver.set_end_node_index(new_node_index);
@@ -3127,12 +3127,12 @@ void ManeuversBuilder::MoveInternalEdgeToPreviousManeuver(Maneuver& prev_maneuve
  // Update the maneuver
 
  // Decrease distance in kilometers
- maneuver.set_length(maneuver.length() - edge->length());
+ maneuver.set_length(maneuver.length() - edge->length_km());
 
  // Decrease basic time (len/speed on each edge with no stop impact) in seconds
  maneuver.set_basic_time(
  maneuver.basic_time() -
- GetTime(edge->length(), GetSpeed(maneuver.travel_mode(), edge->default_speed())));
+ GetTime(edge->length_km(), GetSpeed(maneuver.travel_mode(), edge->default_speed())));
 
  // Set the begin node index
  maneuver.set_begin_node_index(new_node_index);

diff --git a/src/thor/triplegbuilder.cc b/src/thor/triplegbuilder.cc
@@ -1285,7 +1285,7 @@ void TripLegBuilder::Build(
  if (controller.attributes.at(kEdgeLength)) {
  float km =
  std::max(directededge->length() * kKmPerMeter * (trim_end_pct - trim_start_pct), 0.001f);
- trip_edge->set_length(km);
+ trip_edge->set_length_km(km);
  }
 
  // How long on this edge?

diff --git a/src/tyr/serializers.cc b/src/tyr/serializers.cc
@@ -76,8 +76,8 @@ std::vector<std::string> openlr_edges(const TripLeg& leg) {
  float reverse_heading = midgard::tangent_angle(edge.end_shape_index(), end, shape, 20.f, false);
 
  std::vector<baldr::OpenLR::LocationReferencePoint> lrps;
- lrps.emplace_back(start.lng(), start.lat(), forward_heading, frc, fow, nullptr, edge.length(),
- frc);
+ lrps.emplace_back(start.lng(), start.lat(), forward_heading, frc, fow, nullptr,
+ edge.length_km() * valhalla::midgard::kMetersPerKm, frc);
  lrps.emplace_back(end.lng(), end.lat(), reverse_heading, frc, fow, &lrps.back());
  openlrs.emplace_back(baldr::OpenLR::OpenLr{lrps, 0, 0}.toBase64());
  }

diff --git a/src/tyr/trace_serializer.cc b/src/tyr/trace_serializer.cc
@@ -187,8 +187,8 @@ json::ArrayPtr serialize_edges(const AttributesController& controller,
  if (edge.has_speed()) {
  edge_map->emplace("speed", static_cast<uint64_t>(std::round(edge.speed() * scale)));
  }
- if (edge.has_length()) {
- edge_map->emplace("length", json::fp_t{edge.length() * scale, 3});
+ if (edge.has_length_km()) {
+ edge_map->emplace("length", json::fp_t{edge.length_km() * scale, 3});
  }
  // TODO: do we want to output 'is_route_number'?
  if (edge.name_size() > 0) {

diff --git a/test/gurka/gurka.cc b/test/gurka/gurka.cc
@@ -1078,7 +1078,7 @@ void expect_path_length(const valhalla::Api& result,
  for (const auto& leg : result.trip().routes(0).legs()) {
  for (const auto& node : leg.node()) {
  if (node.has_edge())
- length_km += node.edge().length();
+ length_km += node.edge().length_km();
  }
  }
 

diff --git a/test/gurka/test_recost.cc b/test/gurka/test_recost.cc
@@ -240,7 +240,7 @@ TEST(recosting, all_algorithms) {
  uint32_t pred = baldr::kInvalidLabel;
  sif::LabelCallback label_cb = [&elapsed_itr, &length, &pred,
  reverse](const sif::EdgeLabel& label) -> void {
- length += elapsed_itr->edge().length() * 1000.0;
+ length += elapsed_itr->edge().length_km() * 1000.0;
  EXPECT_EQ(elapsed_itr->edge().id(), label.edgeid());
  EXPECT_EQ(pred++, label.predecessor());
  EXPECT_EQ(static_cast<uint8_t>(elapsed_itr->edge().travel_mode()),

diff --git a/test/gurka/test_route.cc b/test/gurka/test_route.cc
@@ -312,7 +312,7 @@ uint32_t speed_from_edge(const valhalla::Api& api) {
  const auto& node = nodes.Get(i);
  if (!node.has_edge())
  break;
- auto km = node.edge().length();
+ auto km = node.edge().length_km();
  auto h = (nodes.Get(i + 1).cost().elapsed_cost().seconds() -
  node.cost().elapsed_cost().seconds() - node.cost().transition_cost().seconds()) /
  3600.0;

diff --git a/test/maneuversbuilder.cc b/test/maneuversbuilder.cc
@@ -381,7 +381,7 @@ void PopulateEdge(TripLeg_Edge* edge,
  edge_name->set_value(name.first);
  edge_name->set_is_route_number(name.second);
  }
- edge->set_length(length);
+ edge->set_length_km(length);
  edge->set_speed(speed);
  edge->set_road_class(road_class);
  edge->set_begin_heading(begin_heading);

diff --git a/test/mapmatch.cc b/test/mapmatch.cc
@@ -1428,7 +1428,7 @@ TEST(Mapmatch, openlr_parameter_true_osrm_api) {
  "CwOduyULYiKJAAAV//0iGw==",
  "CwOdxCULYCKJAAAN//4iGw==",
  "CwOdySULXyKJAAAf//EiGw==",
- "CwOd1yULVyKLAABV/9AiGw==",
+ "CwOd1yULVyKLAQBV/9AiGw==",
  };
  for (const auto& match : matches) {
  std::vector<std::string> references;
@@ -1450,7 +1450,7 @@ TEST(Mapmatch, openlr_parameter_true_native_api) {
  "CwOduyULYiKJAAAV//0iGw==",
  "CwOdxCULYCKJAAAN//4iGw==",
  "CwOdySULXyKJAAAf//EiGw==",
- "CwOd1yULVyKLAABV/9AiGw==",
+ "CwOd1yULVyKLAQBV/9AiGw==",
  };
  std::vector<std::string> references;
  for (const auto& reference : response.get_child("trip.linear_references"))

diff --git a/valhalla/odin/enhancedtrippath.h b/valhalla/odin/enhancedtrippath.h
@@ -138,8 +138,8 @@ class EnhancedTripLeg_Edge {
  return mutable_edge_->tagged_name();
  }
 
- float length() const {
- return mutable_edge_->length();
+ float length_km() const {
+ return mutable_edge_->length_km();
  }
 
  float speed() const {