reviews

CHANGELOG.md

@@ -1,6 +1,6 @@
 # UNRELEASED
  - Features:
-    - ADDED: `table` plugin now optionally returns `distance` matrix as part of response [#4990](https://github.com/Project-OSRM/osrm-backend/pull/4990)
+    - ADDED: `table` plugin now optionally returns `distances` matrix as part of response [#4990](https://github.com/Project-OSRM/osrm-backend/pull/4990)
     - ADDED: New optional parameter `annotations` for `table` that accepts `distance`, `duration`, or both `distance,duration` as values [#4990](https://github.com/Project-OSRM/osrm-backend/pull/4990)
 
 # 5.17.0

docs/http.md

@@ -266,7 +266,7 @@ curl 'http://router.project-osrm.org/table/v1/driving/polyline(egs_Iq_aqAppHzbHu
 # Returns a 3x3 duration matrix:
 curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397634,52.529407;13.428555,52.523219&annotations=duration'
 
-# Returns a 3x3 duration matrix and a 3x3 distance matrix:
+# Returns a 3x3 distance matrix:
 curl 'http://router.project-osrm.org/table/v1/driving/13.388860,52.517037;13.397634,52.529407;13.428555,52.523219&annotations=distance'
 
 # Returns a 3x3 duration matrix and a 3x3 distance matrix:

features/step_definitions/distance_matrix.js

@@ -69,7 +69,7 @@ function tableParse(table, noRoute, annotation, callback) {
 
             var testRow = (row, ri, cb) => {
                 for (var k in result[ri]) {
-                    if (this.FuzzyMatch.match(result[ri][k], row[k])) { // for floating points, we should do an epsilon check which is find the difference and check > or < 0.000001 -- do this in fuzzy match
+                    if (this.FuzzyMatch.match(result[ri][k], row[k])) {
                         result[ri][k] = row[k];
                     } else if (row[k] === '' && result[ri][k] === noRoute) {
                         result[ri][k] = '';

features/support/hooks.js

@@ -51,7 +51,7 @@ module.exports = function () {
             .defer(rimraf, this.scenarioLogFile)
             .awaitAll(callback);
         // uncomment to get path to logfile
-        // console.log('  Writing logging output to ' + this.scenarioLogFile);
+        console.log('  Writing logging output to ' + this.scenarioLogFile);
     });
 
     this.After((scenario, callback) => {

features/testbot/duration_matrix.feature

@@ -111,8 +111,6 @@ Feature: Basic Duration Matrix
             |   | a | b  | e | g |
             | a | 0 | 15 |   |   |
 
-
-
     Scenario: Testbot - Travel time matrix with different way speeds
         Given the node map
             """

include/engine/api/table_api.hpp

@@ -65,8 +65,11 @@ class TableAPI final : public BaseAPI
             response.values["destinations"] = MakeWaypoints(phantoms, parameters.destinations);
         }
 
-        response.values["durations"] =
-            MakeDurationTable(tables.first, number_of_sources, number_of_destinations);
+        if (parameters.annotations & TableParameters::AnnotationsType::Duration)
+        {
+            response.values["durations"] =
+                MakeDurationTable(tables.first, number_of_sources, number_of_destinations);
+        }
 
         if (parameters.annotations & TableParameters::AnnotationsType::Distance)
         {

include/engine/routing_algorithms.hpp

@@ -34,7 +34,8 @@ class RoutingAlgorithmsInterface
     ManyToManySearch(const std::vector<PhantomNode> &phantom_nodes,
                      const std::vector<std::size_t> &source_indices,
                      const std::vector<std::size_t> &target_indices,
-                     const bool calculate_distance) const = 0;
+                     const bool calculate_distance,
+                     const bool calculate_duration) const = 0;
 
     virtual routing_algorithms::SubMatchingList
     MapMatching(const routing_algorithms::CandidateLists &candidates_list,
@@ -86,7 +87,8 @@ template <typename Algorithm> class RoutingAlgorithms final : public RoutingAlgo
     ManyToManySearch(const std::vector<PhantomNode> &phantom_nodes,
                      const std::vector<std::size_t> &source_indices,
                      const std::vector<std::size_t> &target_indices,
-                     const bool calculate_distance) const final override;
+                     const bool calculate_distance,
+                     const bool calculate_duration) const final override;
 
     routing_algorithms::SubMatchingList
     MapMatching(const routing_algorithms::CandidateLists &candidates_list,
@@ -190,7 +192,8 @@ std::pair<std::vector<EdgeDuration>, std::vector<EdgeDistance>>
 RoutingAlgorithms<Algorithm>::ManyToManySearch(const std::vector<PhantomNode> &phantom_nodes,
                                                const std::vector<std::size_t> &_source_indices,
                                                const std::vector<std::size_t> &_target_indices,
-                                               const bool calculate_distance) const
+                                               const bool calculate_distance,
+                                               const bool calculate_duration) const
 {
     BOOST_ASSERT(!phantom_nodes.empty());
 
@@ -213,7 +216,8 @@ RoutingAlgorithms<Algorithm>::ManyToManySearch(const std::vector<PhantomNode> &p
                                                 phantom_nodes,
                                                 std::move(source_indices),
                                                 std::move(target_indices),
-                                                calculate_distance);
+                                                calculate_distance,
+                                                calculate_duration);
 }
 
 template <typename Algorithm>

include/engine/routing_algorithms/many_to_many.hpp

@@ -83,7 +83,8 @@ manyToManySearch(SearchEngineData<Algorithm> &engine_working_data,
                  const std::vector<PhantomNode> &phantom_nodes,
                  const std::vector<std::size_t> &source_indices,
                  const std::vector<std::size_t> &target_indices,
-                 const bool calculate_distance);
+                 const bool calculate_distance,
+                 const bool calculate_duration);
 
 } // namespace routing_algorithms
 } // namespace engine

src/engine/plugins/table.cpp

@@ -82,12 +82,14 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
 
     auto snapped_phantoms = SnapPhantomNodes(phantom_nodes);
 
-    bool requestDistance = params.annotations & api::TableParameters::AnnotationsType::Distance;
+    bool request_distance = params.annotations & api::TableParameters::AnnotationsType::Distance;
+    bool request_duration = params.annotations & api::TableParameters::AnnotationsType::Duration;
 
     auto result_tables_pair = algorithms.ManyToManySearch(
-        snapped_phantoms, params.sources, params.destinations, requestDistance);
+        snapped_phantoms, params.sources, params.destinations, request_distance, request_duration);
 
-    if (result_tables_pair.first.empty() || (requestDistance && result_tables_pair.first.empty()))
+    if ((request_duration & result_tables_pair.first.empty()) ||
+        (request_distance && result_tables_pair.second.empty()))
     {
         return Error("NoTable", "No table found", result);
     }

src/engine/plugins/trip.cpp

@@ -217,7 +217,10 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
 
     // compute the duration table of all phantom nodes
     auto result_duration_table = util::DistTableWrapper<EdgeWeight>(
-        algorithms.ManyToManySearch(snapped_phantoms, {}, {}, /*requestDistance*/ false).first,
+        algorithms
+            .ManyToManySearch(
+                snapped_phantoms, {}, {}, /*requestDistance*/ false, /*requestDuration*/ true)
+            .first,
         number_of_locations);
 
     if (result_duration_table.size() == 0)