Project-OSRM · mjjbell · Aug 22, 2022 · Jun 8, 2021
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,8 @@
       - FIXED: Fixed Node docs generation check in CI. [#6058](https://github.com/Project-OSRM/osrm-backend/pull/6058)
       - CHANGED: Docker build, enabled arm64 build layer [#6172](https://github.com/Project-OSRM/osrm-backend/pull/6172)
       - CHANGED: Docker build, enabled apt-get update/install caching in separate layer for build phase [#6175](https://github.com/Project-OSRM/osrm-backend/pull/6175)
+    - Routing:
+      - ADDED: Add support for non-round-trips with a single fixed endpoint. [#6050](https://github.com/Project-OSRM/osrm-backend/pull/6050)
 
 # 5.26.0
   - Changes from 5.25.0

diff --git a/docs/http.md b/docs/http.md
@@ -507,8 +507,8 @@ Right now, the following combinations are possible:
 | true | any | last | **yes** |
 | true | any | any | **yes** |
 | false | first | last | **yes** |
-| false | first | any | no |
-| false | any | last | no |
+| false | first | any | **yes** |
+| false | any | last | **yes** |
 | false | any | any | no |
 
 #### Example Requests

diff --git a/features/step_definitions/trip.js b/features/step_definitions/trip.js
@@ -61,7 +61,8 @@ module.exports = function () {
                     var subTrips;
                     var trip_durations;
                     var trip_distance;
-                    if (res.statusCode === 200) {
+                    var ok = res.statusCode === 200;
+                    if (ok) {
                         if (headers.has('trips')) {
                             subTrips = json.trips.filter(t => !!t).map(t => t.legs).map(tl => Array.prototype.concat.apply([], tl.map((sl, i) => {
                                 var toAdd = [];
@@ -84,8 +85,7 @@ module.exports = function () {
                         }
                     }
 
-                    var ok = true,
-                        encodedResult = '';
+                    var encodedResult = '';
 
                     if (json.trips) row.trips.split(',').forEach((sub, si) => {
                         if (si >= subTrips.length) {

diff --git a/features/testbot/trip.feature b/features/testbot/trip.feature
@@ -5,7 +5,7 @@ Feature: Basic trip planning
         Given the profile "testbot"
         Given a grid size of 10 meters
 
-    Scenario: Testbot - Trip: Roundtrip with one waypoint
+    Scenario: Testbot - Trip: Roundtrip between same waypoint
         Given the node map
             """
             a b
@@ -21,7 +21,7 @@ Feature: Basic trip planning
 
         When I plan a trip I should get
             | waypoints | trips  |
-            | a         | aa     |
+            | a,a       | aa     |
 
     Scenario: Testbot - Trip: Roundtrip with waypoints (less than 10)
         Given the node map
@@ -69,7 +69,37 @@ Feature: Basic trip planning
             | waypoints               | trips         |
             | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba |
 
-    Scenario: Testbot - Trip: Roundtrip FS waypoints (more than 10)
+    Scenario: Testbot - Trip: FS waypoints (less than 10)
+        Given the query options
+            | source | first  |
+        Given the node map
+            """
+            a b c d
+            l     e
+
+            j i   g
+            """
+
+        And the ways
+            | nodes |
+            | ab    |
+            | bc    |
+            | de    |
+            | eg    |
+            | gi    |
+            | ij    |
+            | jl    |
+            | la    |
+
+        When I plan a trip I should get
+            | waypoints               | trips         | roundtrip | durations |
+            | a,b,c,d,e,g,i,j,l       | abcdegijla    | true      | 22        |
+            | a,b,c,d,e,g,i,j,l       | abcljiged     | false     | 13        |
+
+
+    Scenario: Testbot - Trip: FS waypoints (more than 10)
+        Given the query options
+            | source | first  |
         Given the node map
             """
             a b c d
@@ -93,12 +123,41 @@ Feature: Basic trip planning
             | la    |
 
         When I plan a trip I should get
-            | waypoints               | source | trips         |
-            | a,b,c,d,e,f,g,h,i,j,k,l | first  | alkjihgfedcba |
+            | waypoints               | trips         | roundtrip | durations |
+            | a,b,c,d,e,f,g,h,i,j,k,l | alkjihgfedcba | true      | 22        |
+            | a,b,c,d,e,f,g,h,i,j,k,l | acblkjihgfed  | false     | 13        |
 
-    Scenario: Testbot - Trip: Roundtrip FE waypoints (more than 10)
+
+    Scenario: Testbot - Trip: FE waypoints (less than 10)
+        Given the query options
+            | destination | last  |
+        Given the node map
+            """
+            a b c d
+            l     e
+
+            j i   g
+            """
+
+        And the ways
+            | nodes |
+            | ab    |
+            | bc    |
+            | de    |
+            | eg    |
+            | gi    |
+            | ij    |
+            | jl    |
+            | la    |
+
+        When I plan a trip I should get
+            | waypoints             | trips        | roundtrip | durations |
+            | a,b,c,d,e,g,i,j,l     | labcdegijl   | true      | 22        |
+            | a,b,c,d,e,g,i,j,l     | degijabcl    | false     | 14        |
+
+    Scenario: Testbot - Trip: FE waypoints (more than 10)
         Given the query options
-            | source | last  |
+            | destination | last  |
         Given the node map
             """
             a b c d
@@ -122,8 +181,9 @@ Feature: Basic trip planning
             | la    |
 
         When I plan a trip I should get
-            | waypoints               | trips         |
-            | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal |
+            | waypoints               | trips         | roundtrip | durations |
+            | a,b,c,d,e,f,g,h,i,j,k,l | lkjihgfedcbal | true      | 22        |
+            | a,b,c,d,e,f,g,h,i,j,k,l | cbakjihgfedl  | false     | 19        |
 
     Scenario: Testbot - Trip: Unroutable roundtrip with waypoints (less than 10)
         Given the node map
@@ -274,7 +334,7 @@ Feature: Basic trip planning
             |  a,b,d,e,c  | first  | last        | true      | abedca  |
 
 
-    Scenario: Testbot - Trip: midway points in isoldated roads should return no trips
+    Scenario: Testbot - Trip: midway points in isolated roads should return no trips
         Given the node map
             """
             a 1 b
@@ -370,4 +430,4 @@ Feature: Basic trip planning
         When I plan a trip I should get
             | waypoints | trips | durations | geometry                                                              |
             | a,b,c,d   | abcda |       7.6 | 1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009,1,1             |
-            | d,b,c,a   | dbcad |       7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 |
+            | d,b,c,a   | dbcad |       7.6 | 0.99991,1.00009,1,1,1,1.00009,0.99991,1,1,1.00009,1,1,0.99991,1.00009 |
diff --git a/features/testbot/zero-speed-updates.feature b/features/testbot/zero-speed-updates.feature
@@ -187,5 +187,5 @@ Feature: Check zero speed updates
 
         When I plan a trip I should get
             | waypoints | trips | code    |
-            | a,b,c,d   | abcda | NoTrips |
-            | d,b,c,a   | dbcad | NoTrips |
+            | a,b,c,d   |       | NoTrips |
+            | d,b,c,a   |       | NoTrips |
diff --git a/src/engine/plugins/trip.cpp b/src/engine/plugins/trip.cpp
@@ -36,18 +36,7 @@ bool IsSupportedParameterCombination(const bool fixed_start,
                                      const bool fixed_end,
                                      const bool roundtrip)
 {
-    if (fixed_start && fixed_end && !roundtrip)
-    {
-        return true;
-    }
-    else if (roundtrip)
-    {
-        return true;
-    }
-    else
-    {
-        return false;
-    }
+    return roundtrip || fixed_start || fixed_end;
 }
 
 // given the node order in which to visit, compute the actual route (with geometry, travel time and
@@ -142,6 +131,32 @@ void ManipulateTableForFSE(const std::size_t source_id,
     //*********  End of changes to table  *************************************
 }
 
+void ManipulateTableForNonRoundtripFS(const std::size_t source_id,
+                                      util::DistTableWrapper<EdgeWeight> &result_table)
+{
+    // We can use the round-trip calculation to simulate non-round-trip fixed start
+    // by making all paths to the source location zero. Effectively finding an 'optimal'
+    // round-trip path that ignores the cost of getting back from any destination to the
+    // source.
+    for (const auto i : util::irange<size_t>(0, result_table.GetNumberOfNodes()))
+    {
+        result_table.SetValue(i, source_id, 0);
+    }
+}
+
+void ManipulateTableForNonRoundtripFE(const std::size_t destination_id,
+                                      util::DistTableWrapper<EdgeWeight> &result_table)
+{
+    // We can use the round-trip calculation to simulate non-round-trip fixed end
+    // by making all paths from the destination to other locations zero.
+    // Effectively, finding an 'optimal' round-trip path that ignores the cost of getting
+    // from the destination to any source.
+    for (const auto i : util::irange<size_t>(0, result_table.GetNumberOfNodes()))
+    {
+        result_table.SetValue(destination_id, i, 0);
+    }
+}
+
 Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
                                  const api::TripParameters &parameters,
                                  osrm::engine::api::ResultT &result) const
@@ -225,7 +240,7 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
         return Status::Error;
     }
 
-    const constexpr std::size_t BF_MAX_FEASABLE = 10;
+    const constexpr std::size_t BF_MAX_FEASIBLE = 10;
     BOOST_ASSERT_MSG(result_duration_table.size() == number_of_locations * number_of_locations,
                      "Distance Table has wrong size");
 
@@ -238,11 +253,19 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
     {
         ManipulateTableForFSE(source_id, destination_id, result_duration_table);
     }
+    else if (!parameters.roundtrip && fixed_start)
+    {
+        ManipulateTableForNonRoundtripFS(source_id, result_duration_table);
+    }
+    else if (!parameters.roundtrip && fixed_end)
+    {
+        ManipulateTableForNonRoundtripFE(destination_id, result_duration_table);
+    }
 
     std::vector<NodeID> duration_trip;
     duration_trip.reserve(number_of_locations);
     // get an optimized order in which the destinations should be visited
-    if (number_of_locations < BF_MAX_FEASABLE)
+    if (number_of_locations < BF_MAX_FEASIBLE)
     {
         duration_trip = trip::BruteForceTrip(number_of_locations, result_duration_table);
     }
@@ -251,20 +274,28 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
         duration_trip = trip::FarthestInsertionTrip(number_of_locations, result_duration_table);
     }
 
-    // rotate result such that roundtrip starts at node with index 0
-    // thist first if covers scenarios: !fixed_end || fixed_start || (fixed_start && fixed_end)
     if (!fixed_end || fixed_start)
     {
+        // rotate result such that trip starts at node with index 0
         auto desired_start_index = std::find(std::begin(duration_trip), std::end(duration_trip), 0);
         BOOST_ASSERT(desired_start_index != std::end(duration_trip));
         std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip));
     }
-    else if (fixed_end && !fixed_start && parameters.roundtrip)
-    {
-        auto desired_start_index =
+    else
+    { // fixed_end
+        auto destination_index =
             std::find(std::begin(duration_trip), std::end(duration_trip), destination_id);
-        BOOST_ASSERT(desired_start_index != std::end(duration_trip));
-        std::rotate(std::begin(duration_trip), desired_start_index, std::end(duration_trip));
+        BOOST_ASSERT(destination_index != std::end(duration_trip));
+        if (!parameters.roundtrip)
+        {
+            // We want the location after destination to be at the front
+            std::advance(destination_index, 1);
+            if (destination_index == std::end(duration_trip))
+            {
+                destination_index = std::begin(duration_trip);
+            }
+        }
+        std::rotate(std::begin(duration_trip), destination_index, std::end(duration_trip));
     }
 
     // get the route when visiting all destinations in optimized order

diff --git a/test/nodejs/trip.js b/test/nodejs/trip.js
@@ -262,33 +262,19 @@ test('trip: routes Monaco with null hints', function(assert) {
 });
 
 test('trip: service combinations that are not implemented', function(assert) {
-    assert.plan(3);
+    assert.plan(1);
     var osrm = new OSRM(data_path);
 
-    // fixed start, non-roundtrip
+    // no fixed start, no fixed end, non-roundtrip
     var options = {
         coordinates: two_test_coordinates,
-        source: 'first',
+        source: 'any',
+        destination: 'any',
         roundtrip: false
     };
     osrm.trip(options, function(err, second) {
         assert.equal('NotImplemented', err.message);
     });
-
-    // fixed start, fixed end, non-roundtrip
-    options.source = 'any';
-    options.destination = 'any';
-    osrm.trip(options, function(err, second) {
-        assert.equal('NotImplemented', err.message);
-    });
-
-    // fixed end, non-roundtrip
-    delete options.source;
-    options.destination = 'last';
-    osrm.trip(options, function(err, second) {
-        assert.equal('NotImplemented', err.message);
-    });
-
 });
 
 test('trip: fixed start and end combinations', function(assert) {
@@ -302,31 +288,46 @@ test('trip: fixed start and end combinations', function(assert) {
         geometries: 'geojson'
     };
 
-    // fixed start and end, non-roundtrip
-    osrm.trip(options, function(err, fseTrip) {
-        assert.ifError(err);
-        assert.equal(1, fseTrip.trips.length);
-        var coordinates = fseTrip.trips[0].geometry.coordinates;
-        assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1]));
-    });
+    // variations of non roundtrip
+    var nonRoundtripChecks = function(options) {
+        osrm.trip(options, function(err, fseTrip) {
+            assert.ifError(err);
+            assert.equal(1, fseTrip.trips.length);
+            var coordinates = fseTrip.trips[0].geometry.coordinates;
+            assert.notEqual(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1]));
+        });
+    };
 
     // variations of roundtrip
-
     var roundtripChecks = function(options) {
         osrm.trip(options, function(err, trip) {
             assert.ifError(err);
             assert.equal(1, trip.trips.length);
             var coordinates = trip.trips[0].geometry.coordinates;
             assert.equal(JSON.stringify(coordinates[0]), JSON.stringify(coordinates[coordinates.length - 1]));
         });
-    }
+    };
+
+    // fixed start and end, non-roundtrip
+    nonRoundtripChecks(options);
+
+    // fixed start, non-roundtrip
+    delete options.destination;
+    options.source = 'first';
+    nonRoundtripChecks(options);
+
+    // fixed end, non-roundtrip
+    delete options.source;
+    options.destination = 'last';
+    nonRoundtripChecks(options);
 
     // roundtrip, source and destination not specified
     roundtripChecks({coordinates: options.coordinates, geometries: options.geometries});
 
     // roundtrip, fixed destination
     options.roundtrip = true;
     delete options.source;
+    options.destination = 'last';
     roundtripChecks(options);
 
     //roundtrip, fixed source