Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[not ready] Left hand driving #2670

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions features/guidance/anticipate-lanes.feature
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,38 @@ Feature: Turn Lane Guidance
| waypoints | route | turns | lanes |
| a,h | ab,gh,gh | depart,roundabout-exit-5,arrive | ,slight right:false slight right:true, |

@anticipate @todo
Scenario: Anticipate with lanes in roundabout where we stay on the roundabout for multiple exits
# Scenario requires correct lane handling for clockwise roundabouts
Given the profile "lhs"
And the node map
| | | a | | |
| | | b | | |
| h | c | | g | |
| | | | | |
| | d | | f | |
| | | e | | |
| x | | | | y |

And the ways
| nodes | turn:lanes:forward | highway | junction |
| ab | slight_left\|slight_left | primary | |
| bg | | primary | roundabout |
| gf | | primary | roundabout |
| fe | | primary | roundabout |
| ed | | primary | roundabout |
| dc | slight_left | primary | roundabout |
| cb | | primary | roundabout |
| ch | | primary | |
| ex | | primary | |
| dx | | primary | |
| gy | | primary | |
| fy | | primary | |

When I route I should get
| waypoints | route | turns | lanes |
| a,h | ab,ch,ch | depart,roundabout-exit-5,arrive | ,slight right:false slight right:true, |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot to change the expected lanes output here from slight right to slight left.
If you take a look at the way ab and dc above, they only have slight left lanes.

Counterclockwise vs clockwise does not change the turn lane's direction.
It only changes lane anticipation's behavior in keeping the user to the left/right.

Copy link
Contributor

@oxidase oxidase Jul 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, here must be slight left, but the test fails because query returns an empty result, so i set todo state and kept the original expecation, because had no idea what should be be the expected result.

I will try to find out the reason during the next week. @MoKob showed me the starting place https://github.com/Project-OSRM/osrm-backend/blob/master/src/extractor/guidance/turn_lane_matcher.cpp#L54-L103


@anticipate
Scenario: Departing or arriving inside a roundabout does not yet anticipate lanes
Given the node map
Expand Down
83 changes: 83 additions & 0 deletions features/testbot/side_bias.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
@routing @testbot @sidebias
Feature: Testbot - side bias

Scenario: Left hand bias
Given the profile "lhs"
And the node map
| a | | b | | c |
| | | | | |
| | | d | | |
And the ways
| nodes |
| ab |
| bc |
| bd |

When I route I should get
| from | to | route | time |
| d | a | bd,ab,ab | 82s +-1 |
| d | c | bd,bc,bc | 100s +-1 |

Scenario: Right hand bias
Given the profile "rhs"
And the node map
| a | | b | | c |
| | | | | |
| | | d | | |
And the ways
| nodes |
| ab |
| bc |
| bd |

When I route I should get
| from | to | route | time |
| d | a | bd,ab,ab | 100s +-1 |
| d | c | bd,bc,bc | 82s +-1 |

Scenario: Roundabout exit counting for left sided driving
Given the profile "lhs"
And a grid size of 10 meters
And the node map
| | | a | | |
| | | b | | |
| h | g | | c | d |
| | | e | | |
| | | f | | |
And the ways
| nodes | junction |
| ab | |
| cd | |
| ef | |
| gh | |
| bcegb | roundabout |

When I route I should get
| waypoints | route | turns |
| a,d | ab,cd,cd | depart,roundabout-exit-1,arrive |
| a,f | ab,ef,ef | depart,roundabout-exit-2,arrive |
| a,h | ab,gh,gh | depart,roundabout-exit-3,arrive |

Scenario: Mixed Entry and Exit
Given the profile "lhs"
And a grid size of 10 meters
And the node map
| | c | | a | |
| j | | b | | f |
| | k | | e | |
| l | | h | | d |
| | g | | i | |

And the ways
| nodes | junction | oneway |
| cba | | yes |
| fed | | yes |
| ihg | | yes |
| lkj | | yes |
| behkb | roundabout | yes |

When I route I should get
| waypoints | route | turns |
| c,a | cba,cba,cba | depart,roundabout-exit-1,arrive |
| l,a | lkj,cba,cba | depart,roundabout-exit-2,arrive |
| i,a | ihg,cba,cba | depart,roundabout-exit-3,arrive |
2 changes: 1 addition & 1 deletion include/engine/api/route_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class RouteAPI : public BaseAPI
*/

guidance::trimShortSegments(steps, leg_geometry);
leg.steps = guidance::postProcess(std::move(steps));
leg.steps = guidance::postProcess(std::move(steps), BaseAPI::facade);
leg.steps = guidance::collapseTurns(std::move(leg.steps));
leg.steps = guidance::buildIntersections(std::move(leg.steps));
leg.steps = guidance::assignRelativeLocations(std::move(leg.steps),
Expand Down
2 changes: 2 additions & 0 deletions include/engine/datafacade/datafacade_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ class BaseDataFacade

virtual bool GetContinueStraightDefault() const = 0;

virtual bool UseLeftSideDriving() const = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MockDataFacade needs a dummy impl. for this, too. Otherwise unit tests won't compile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(try with make tests)


virtual BearingClassID GetBearingClassID(const NodeID id) const = 0;

virtual util::guidance::BearingClass
Expand Down
5 changes: 5 additions & 0 deletions include/engine/datafacade/internal_datafacade.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,11 @@ class InternalDataFacade final : public BaseDataFacade
return m_profile_properties.continue_straight_at_waypoint;
}

bool UseLeftSideDriving() const override final
{
return m_profile_properties.left_hand_driving;
}

BearingClassID GetBearingClassID(const NodeID nid) const override final
{
return m_bearing_class_id_table.at(nid);
Expand Down
5 changes: 5 additions & 0 deletions include/engine/datafacade/shared_datafacade.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -793,6 +793,11 @@ class SharedDataFacade final : public BaseDataFacade
return m_profile_properties->continue_straight_at_waypoint;
}

bool UseLeftSideDriving() const override final
{
return m_profile_properties->left_hand_driving;
}

BearingClassID GetBearingClassID(const NodeID id) const override final
{
return m_bearing_class_id_table.at(id);
Expand Down
3 changes: 2 additions & 1 deletion include/engine/guidance/post_processing.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "engine/guidance/leg_geometry.hpp"
#include "engine/guidance/route_step.hpp"
#include "engine/phantom_node.hpp"
#include "engine/datafacade/datafacade_base.hpp"

#include <vector>

Expand All @@ -15,7 +16,7 @@ namespace guidance
{

// passed as none-reference to modify in-place and move out again
std::vector<RouteStep> postProcess(std::vector<RouteStep> steps);
std::vector<RouteStep> postProcess(std::vector<RouteStep> steps, const datafacade::BaseDataFacade &facade);

// Multiple possible reasons can result in unnecessary/confusing instructions
// A prime example would be a segregated intersection. Turning around at this
Expand Down
5 changes: 4 additions & 1 deletion include/extractor/guidance/roundabout_handler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "extractor/guidance/intersection_handler.hpp"
#include "extractor/guidance/roundabout_type.hpp"
#include "extractor/query_node.hpp"
#include "extractor/profile_properties.hpp"

#include "util/name_table.hpp"
#include "util/node_based_graph.hpp"
Expand Down Expand Up @@ -42,7 +43,8 @@ class RoundaboutHandler : public IntersectionHandler
const std::vector<QueryNode> &node_info_list,
const CompressedEdgeContainer &compressed_edge_container,
const util::NameTable &name_table,
const SuffixTable &street_name_suffix_table);
const SuffixTable &street_name_suffix_table,
const ProfileProperties &profile_properties);

~RoundaboutHandler() override final;

Expand Down Expand Up @@ -81,6 +83,7 @@ class RoundaboutHandler : public IntersectionHandler
bool qualifiesAsRoundaboutIntersection(const std::set<NodeID> &roundabout_nodes) const;

const CompressedEdgeContainer &compressed_edge_container;
const ProfileProperties &profile_properties;
};

} // namespace guidance
Expand Down
3 changes: 2 additions & 1 deletion include/extractor/guidance/turn_analysis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ class TurnAnalysis
const std::unordered_set<NodeID> &barrier_nodes,
const CompressedEdgeContainer &compressed_edge_container,
const util::NameTable &name_table,
const SuffixTable &street_name_suffix_table);
const SuffixTable &street_name_suffix_table,
const ProfileProperties &profile_properties);

// the entry into the turn analysis
Intersection getIntersection(const NodeID from_node, const EdgeID via_eid) const;
Expand Down
3 changes: 2 additions & 1 deletion include/extractor/profile_properties.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct ProfileProperties
{
ProfileProperties()
: traffic_signal_penalty(0), u_turn_penalty(0), continue_straight_at_waypoint(true),
use_turn_restrictions(false)
use_turn_restrictions(false), left_hand_driving(false)
{
}

Expand All @@ -36,6 +36,7 @@ struct ProfileProperties
int u_turn_penalty;
bool continue_straight_at_waypoint;
bool use_turn_restrictions;
bool left_hand_driving;
};
}
}
Expand Down
3 changes: 2 additions & 1 deletion profiles/car.lua
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,14 @@ properties.u_turn_penalty = 20
properties.traffic_signal_penalty = 2
properties.use_turn_restrictions = true
properties.continue_straight_at_waypoint = true
properties.left_hand_driving = false

local side_road_speed_multiplier = 0.8

local turn_penalty = 10
-- Note: this biases right-side driving. Should be
-- inverted for left-driving countries.
local turn_bias = 1.2
local turn_bias = properties.left_hand_driving and 1/1.2 or 1.2

local obey_oneway = true
local ignore_areas = true
Expand Down
19 changes: 19 additions & 0 deletions profiles/lhs.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- Testbot, with turn penalty
-- Used for testing turn penalties

require 'testbot'

properties.left_hand_driving = true

local turn_penalty = 500
local turn_bias = properties.left_hand_driving and 1/1.2 or 1.2

function turn_function (angle)
---- compute turn penalty as angle^2, with a left/right bias
k = turn_penalty/(90.0*90.0)
if angle>=0 then
return angle*angle*k/turn_bias
else
return angle*angle*k*turn_bias
end
end
19 changes: 19 additions & 0 deletions profiles/rhs.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- Testbot, with turn penalty
-- Used for testing turn penalties

require 'testbot'

properties.left_hand_driving = false

local turn_penalty = 500
local turn_bias = properties.left_hand_driving and 1/1.2 or 1.2

function turn_function (angle)
---- compute turn penalty as angle^2, with a left/right bias
k = turn_penalty/(90.0*90.0)
if angle>=0 then
return angle*angle*k/turn_bias
else
return angle*angle*k*turn_bias
end
end
23 changes: 15 additions & 8 deletions src/engine/guidance/post_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ void collapseTurnAt(std::vector<RouteStep> &steps,

// Works on steps including silent and invalid instructions in order to do lane anticipation for
// roundabouts which later on get collapsed into a single multi-hop instruction.
std::vector<RouteStep> anticipateLaneChangeForRoundabouts(std::vector<RouteStep> steps)
std::vector<RouteStep> anticipateLaneChangeForRoundabouts(std::vector<RouteStep> steps, const datafacade::BaseDataFacade &facade)
{
using namespace util::guidance;

Expand All @@ -517,17 +517,24 @@ std::vector<RouteStep> anticipateLaneChangeForRoundabouts(std::vector<RouteStep>

// Although the enter instruction may be a left/right turn, for right-sided driving the
// roundabout is counter-clockwise and therefore we need to always set it to a left turn.
// FIXME: assumes right-side driving (counter-clockwise roundabout flow)
const auto enter_direction = enter.maneuver.instruction.direction_modifier;

if (util::guidance::isRightTurn(enter.maneuver.instruction))
enter.maneuver.instruction.direction_modifier =
mirrorDirectionModifier(enter_direction);
if (facade.UseLeftSideDriving())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove FIXME comment above about right-sided assumptions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also below in this file)

{
if (util::guidance::isLeftTurn(enter.maneuver.instruction))
enter.maneuver.instruction.direction_modifier =
mirrorDirectionModifier(enter_direction);
}
else
{
if (util::guidance::isRightTurn(enter.maneuver.instruction))
enter.maneuver.instruction.direction_modifier =
mirrorDirectionModifier(enter_direction);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to test this it would be great to have clockwise-flowing roundabouts modeled as Cucumber scenarios, you can take this one as example which assumes right-sided driving (and therefore counter-clockwise roundabout flow): https://github.com/Project-OSRM/osrm-backend/blob/master/features/guidance/anticipate-lanes.feature#L317


auto enterAndLeave = anticipateLaneChange({enter, leave});

// Undo flipping direction on a right turn in a right-sided counter-clockwise roundabout.
// FIXME: assumes right-side driving (counter-clockwise roundabout flow)
enterAndLeave[0].maneuver.instruction.direction_modifier = enter_direction;

std::swap(*roundabout.first, enterAndLeave[0]);
Expand Down Expand Up @@ -579,7 +586,7 @@ std::vector<RouteStep> removeNoTurnInstructions(std::vector<RouteStep> steps)
// They are required for maintenance purposes. We can calculate the number
// of exits to pass in a roundabout and the number of intersections
// that we come across.
std::vector<RouteStep> postProcess(std::vector<RouteStep> steps)
std::vector<RouteStep> postProcess(std::vector<RouteStep> steps, const datafacade::BaseDataFacade &facade)
{
// the steps should always include the first/last step in form of a location
BOOST_ASSERT(steps.size() >= 2);
Expand All @@ -589,7 +596,7 @@ std::vector<RouteStep> postProcess(std::vector<RouteStep> steps)
// Before we invalidate and remove silent instructions, we handle roundabouts (before they're
// getting collapsed into a single multi-hop instruction) by back-propagating exit lane
// constraints already to a roundabout's enter instruction.
steps = anticipateLaneChangeForRoundabouts(std::move(steps));
steps = anticipateLaneChangeForRoundabouts(std::move(steps), facade);

// Count Street Exits forward
bool on_roundabout = false;
Expand Down
3 changes: 2 additions & 1 deletion src/extractor/edge_based_graph_factory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,8 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
m_barrier_nodes,
m_compressed_edge_container,
name_table,
street_name_suffix_table);
street_name_suffix_table,
profile_properties);
guidance::lanes::TurnLaneHandler turn_lane_handler(
*m_node_based_graph, turn_lane_offsets, turn_lane_masks, m_node_info_list, turn_analysis);

Expand Down
Loading