implements #949, wrong duration on first segment

DataStructures/RawRouteData.h

@@ -34,15 +34,13 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <osrm/Coordinate.h>
 
-#include <limits>
-
 #include <vector>
 
 struct PathData
 {
     PathData()
-        : node(std::numeric_limits<unsigned>::max()), name_id(std::numeric_limits<unsigned>::max()),
-          segment_duration(std::numeric_limits<unsigned>::max()),
+        : node(SPECIAL_NODEID), name_id(INVALID_EDGE_WEIGHT),
+          segment_duration(INVALID_EDGE_WEIGHT),
           turn_instruction(TurnInstruction::NoTurn)
     {
     }
@@ -63,20 +61,18 @@ struct RawRouteData
     std::vector<PathData> unpacked_alternative;
     std::vector<PhantomNodes> segment_end_coordinates;
     std::vector<FixedPointCoordinate> raw_via_node_coordinates;
+    std::vector<bool> source_traversed_in_reverse;
+    std::vector<bool> target_traversed_in_reverse;
+    std::vector<bool> alt_source_traversed_in_reverse;
+    std::vector<bool> alt_target_traversed_in_reverse;
     unsigned check_sum;
     int shortest_path_length;
     int alternative_path_length;
-    bool source_traversed_in_reverse;
-    bool target_traversed_in_reverse;
-    bool alt_source_traversed_in_reverse;
-    bool alt_target_traversed_in_reverse;
 
     RawRouteData()
-        : check_sum(std::numeric_limits<unsigned>::max()),
-          shortest_path_length(std::numeric_limits<int>::max()),
-          alternative_path_length(std::numeric_limits<int>::max()),
-          source_traversed_in_reverse(false), target_traversed_in_reverse(false),
-          alt_source_traversed_in_reverse(false), alt_target_traversed_in_reverse(false)
+        : check_sum(SPECIAL_NODEID),
+          shortest_path_length(INVALID_EDGE_WEIGHT),
+          alternative_path_length(INVALID_EDGE_WEIGHT)
     {
     }
 };

Descriptors/DescriptionFactory.cpp

@@ -38,17 +38,21 @@ std::vector<unsigned> const & DescriptionFactory::GetViaIndices() const
 }
 
 
-void DescriptionFactory::SetStartSegment(const PhantomNode &source)
+void DescriptionFactory::SetStartSegment(const PhantomNode &source, const bool traversed_in_reverse)
 {
     start_phantom = source;
-    AppendSegment(source.location, PathData(0, source.name_id, TurnInstruction::HeadOn, source.forward_weight));
+    const EdgeWeight segment_duration = (!traversed_in_reverse ? source.forward_weight : source.reverse_weight);
+    AppendSegment(source.location, PathData(0, source.name_id, TurnInstruction::HeadOn, segment_duration));
+    BOOST_ASSERT(path_description.back().duration == segment_duration);
 }
 
-void DescriptionFactory::SetEndSegment(const PhantomNode &target)
+void DescriptionFactory::SetEndSegment(const PhantomNode &target, const bool traversed_in_reverse)
 {
     target_phantom = target;
+    const EdgeWeight segment_duration = (traversed_in_reverse ? target.reverse_weight : target.forward_weight);
     path_description.emplace_back(
-        target.location, target.name_id, 0, target.reverse_weight, TurnInstruction::NoTurn, true, true);
+        target.location, target.name_id, segment_duration, 0, TurnInstruction::NoTurn, true, true);
+    BOOST_ASSERT(path_description.back().duration == segment_duration);
 }
 
 void DescriptionFactory::AppendSegment(const FixedPointCoordinate &coordinate,

Descriptors/DescriptionFactory.h

@@ -81,8 +81,8 @@ class DescriptionFactory
     JSON::Value AppendUnencodedPolylineString() const;
     void AppendSegment(const FixedPointCoordinate &coordinate, const PathData &data);
     void BuildRouteSummary(const double distance, const unsigned time);
-    void SetStartSegment(const PhantomNode &start_phantom);
-    void SetEndSegment(const PhantomNode &start_phantom);
+    void SetStartSegment(const PhantomNode &start_phantom, const bool traversed_in_reverse);
+    void SetEndSegment(const PhantomNode &start_phantom, const bool traversed_in_reverse);
     JSON::Value AppendEncodedPolylineString(const bool return_encoded);
     std::vector<unsigned> const & GetViaIndices() const;
 
@@ -155,6 +155,7 @@ class DescriptionFactory
             entireLength += path_description[i].length;
             segment_length += path_description[i].length;
             segment_duration += path_description[i].duration;
+            SimpleLogger().Write() << "length: " << path_description[i].length << ", duration: " << path_description[i].duration;
             path_description[segment_start_index].length = segment_length;
             path_description[segment_start_index].duration = segment_duration;
 
@@ -172,6 +173,7 @@ class DescriptionFactory
         {
             if (path_description.size() > 2)
             {
+                SimpleLogger().Write() << "removing last segment";
                 path_description.pop_back();
                 path_description.back().necessary = true;
                 path_description.back().turn_instruction = TurnInstruction::NoTurn;
@@ -182,6 +184,7 @@ class DescriptionFactory
         {
             if (path_description.size() > 2)
             {
+                SimpleLogger().Write() << "removing first segment";
                 path_description.erase(path_description.begin());
                 path_description.front().turn_instruction = TurnInstruction::HeadOn;
                 path_description.front().necessary = true;

Descriptors/JSONDescriptor.h

@@ -73,7 +73,7 @@ template <class DataFacadeT> class JSONDescriptor : public BaseDescriptor<DataFa
 
     void SetConfig(const DescriptorConfig &c) { config = c; }
 
-    unsigned DescribeLeg(const std::vector<PathData> route_leg, const PhantomNodes &leg_phantoms)
+    unsigned DescribeLeg(const std::vector<PathData> route_leg, const PhantomNodes &leg_phantoms, const bool target_traversed_in_reverse)
     {
         unsigned added_element_count = 0;
         // Get all the coordinates for the computed route
@@ -84,7 +84,7 @@ template <class DataFacadeT> class JSONDescriptor : public BaseDescriptor<DataFa
             description_factory.AppendSegment(current_coordinate, path_data);
             ++added_element_count;
         }
-        description_factory.SetEndSegment(leg_phantoms.target_phantom);
+        description_factory.SetEndSegment(leg_phantoms.target_phantom, target_traversed_in_reverse);
         ++added_element_count;
         BOOST_ASSERT((route_leg.size() + 1) == added_element_count);
         return added_element_count;
@@ -109,7 +109,8 @@ template <class DataFacadeT> class JSONDescriptor : public BaseDescriptor<DataFa
         BOOST_ASSERT(raw_route.unpacked_path_segments.size() ==
                      raw_route.segment_end_coordinates.size());
 
-        description_factory.SetStartSegment(raw_route.segment_end_coordinates.front().source_phantom);
+        description_factory.SetStartSegment(raw_route.segment_end_coordinates.front().source_phantom,
+                                            raw_route.source_traversed_in_reverse.front());
         json_result.values["status"] = 0;
         json_result.values["status_message"] = "Found route between points";
 
@@ -120,7 +121,8 @@ template <class DataFacadeT> class JSONDescriptor : public BaseDescriptor<DataFa
             const int added_segments =
 #endif
             DescribeLeg(raw_route.unpacked_path_segments[i],
-                                                   raw_route.segment_end_coordinates[i]);
+                        raw_route.segment_end_coordinates[i],
+                        raw_route.target_traversed_in_reverse[i]);
             BOOST_ASSERT(0 < added_segments);
         }
         description_factory.Run(facade, config.zoom_level);
@@ -175,7 +177,9 @@ template <class DataFacadeT> class JSONDescriptor : public BaseDescriptor<DataFa
         if (INVALID_EDGE_WEIGHT != raw_route.alternative_path_length)
         {
             json_result.values["found_alternative"] = JSON::True();
-            alternate_description_factory.SetStartSegment(raw_route.segment_end_coordinates.front().source_phantom);
+            BOOST_ASSERT(!raw_route.alt_source_traversed_in_reverse.empty());
+            alternate_description_factory.SetStartSegment(raw_route.segment_end_coordinates.front().source_phantom,
+                                                          raw_route.alt_source_traversed_in_reverse.front());
             // Get all the coordinates for the computed route
             for (const PathData &path_data : raw_route.unpacked_alternative)
             {

RoutingAlgorithms/AlternativePathRouting.h

@@ -307,10 +307,10 @@ template <class DataFacadeT> class AlternativeRouting : private BasicRoutingInte
         {
             BOOST_ASSERT(!packed_shortest_path.empty());
             raw_route_data.unpacked_path_segments.resize(1);
-            raw_route_data.source_traversed_in_reverse =
-                (packed_shortest_path.front() != phantom_node_pair.source_phantom.forward_node_id);
-            raw_route_data.target_traversed_in_reverse =
-                (packed_shortest_path.back() != phantom_node_pair.target_phantom.forward_node_id);
+            raw_route_data.source_traversed_in_reverse.push_back(
+                (packed_shortest_path.front() != phantom_node_pair.source_phantom.forward_node_id));
+            raw_route_data.target_traversed_in_reverse.push_back(
+                (packed_shortest_path.back() != phantom_node_pair.target_phantom.forward_node_id));
 
             super::UnpackPath(
                 // -- packed input
@@ -334,16 +334,19 @@ template <class DataFacadeT> class AlternativeRouting : private BasicRoutingInte
                                         v_t_middle,
                                         packed_alternate_path);
 
-            raw_route_data.source_traversed_in_reverse =
-                (packed_alternate_path.front() != phantom_node_pair.source_phantom.forward_node_id);
-            raw_route_data.target_traversed_in_reverse =
-                (packed_alternate_path.back() != phantom_node_pair.target_phantom.forward_node_id);
+            raw_route_data.alt_source_traversed_in_reverse.push_back(
+                (packed_alternate_path.front() != phantom_node_pair.source_phantom.forward_node_id));
+            raw_route_data.alt_target_traversed_in_reverse.push_back(
+                (packed_alternate_path.back() != phantom_node_pair.target_phantom.forward_node_id));
 
             // unpack the alternate path
             super::UnpackPath(
                 packed_alternate_path, phantom_node_pair, raw_route_data.unpacked_alternative);
 
             raw_route_data.alternative_path_length = length_of_via_path;
+            SimpleLogger().Write() << "length of via path: " << length_of_via_path << " & selected_via_node: " << selected_via_node;
+        } else {
+            BOOST_ASSERT(raw_route_data.alternative_path_length == INVALID_EDGE_WEIGHT);
         }
     }
 

RoutingAlgorithms/BasicRoutingInterface.h

@@ -231,8 +231,7 @@ template <class DataFacadeT> class BasicRoutingInterface
                     BOOST_ASSERT(start_index <= end_index);
                     for (int i = start_index; i < end_index; ++i)
                     {
-                        unpacked_path.emplace_back(PathData{
-                            id_vector[i], name_index, TurnInstruction::NoTurn, 0});
+                        unpacked_path.emplace_back(id_vector[i], name_index, TurnInstruction::NoTurn, 0);
                     }
                     unpacked_path.back().turn_instruction = turn_instruction;
                     unpacked_path.back().segment_duration = ed.distance;

RoutingAlgorithms/ShortestPathRouting.h

@@ -292,13 +292,6 @@ template <class DataFacadeT> class ShortestPathRouting : public BasicRoutingInte
         }
         raw_route_data.unpacked_path_segments.resize(packed_legs1.size());
 
-        raw_route_data.source_traversed_in_reverse =
-            (packed_legs1.front().front() !=
-             phantom_nodes_vector.front().source_phantom.forward_node_id);
-        raw_route_data.target_traversed_in_reverse =
-            (packed_legs1.back().back() !=
-             phantom_nodes_vector.back().target_phantom.forward_node_id);
-
         for (unsigned i = 0; i < packed_legs1.size(); ++i)
         {
             BOOST_ASSERT(!phantom_nodes_vector.empty());
@@ -312,6 +305,11 @@ template <class DataFacadeT> class ShortestPathRouting : public BasicRoutingInte
                 unpack_phantom_node_pair,
                 // -- unpacked output
                 raw_route_data.unpacked_path_segments[i]);
+
+            raw_route_data.source_traversed_in_reverse.push_back(
+            (packed_legs1[i].front() != phantom_nodes_vector[i].source_phantom.forward_node_id));
+            raw_route_data.target_traversed_in_reverse.push_back(
+            (packed_legs1[i].back() != phantom_nodes_vector[i].target_phantom.forward_node_id));
         }
         raw_route_data.shortest_path_length = std::min(distance1, distance2);
     }

features/testbot/time.feature

@@ -213,8 +213,7 @@ Feature: Estimation of travel time
             | 4    | 3  | ab    | 10s +-1 |
             | 4    | 2  | ab    | 20s +-1 |
             | 4    | 1  | ab    | 30s +-1 |
-
-    @bug
+
     Scenario: Total travel time should match sum of times of individual ways
         Given a grid size of 1000 meters
         And the node map