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.

Sign up for GitHub

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

Exposing service-area names in passive maneuvers. #3277

Merged
merged 4 commits into from
Aug 19, 2021
Merged

Exposing service-area names in passive maneuvers. #3277

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd8d4a9
Exposing service-area names in passive maneuvers.
ktatterso Aug 18, 2021
cf11809
changelog update
ktatterso Aug 18, 2021
911bb76
typo
ktatterso Aug 19, 2021
f8f0d9e
Calling destinations() instead of guide_destinations().
ktatterso Aug 19, 2021
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
Next Next commit
Exposing service-area names in passive maneuvers.
  • Loading branch information
@ktatterso
ktatterso committed Aug 18, 2021
commit fd8d4a9270029b2d35ab7a4ee91ca6e100b01752
23 changes: 15 additions & 8 deletions src/tyr/route_serializer_osrm.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,21 +501,28 @@ json::ArrayPtr intersections(const valhalla::DirectionsLeg::Maneuver& maneuver,
auto intersecting_edge = node->GetIntersectingEdge(m);
bool routeable = intersecting_edge->IsTraversableOutbound(curr_edge->travel_mode());

std::string sign_text;
if (intersecting_edge->has_sign()) {
const valhalla::TripSign& trip_leg_sign = intersecting_edge->sign();
// I've looked at the results from guide_destinations(), destinations(), and
// exit_destinations(). exit_destinations() doe not contain rest-area names.
ktatterso marked this conversation as resolved.
Show resolved Hide resolved
// guide_destinations() and destinations() return the same string value for
// the rest area name. So I've decided to use guide_destinations().
sign_text = guide_destinations(trip_leg_sign);
ktatterso marked this conversation as resolved.
Show resolved Hide resolved
}

if (routeable && intersecting_edge->use() == TripLeg_Use_kRestAreaUse) {
rest_stop->emplace("type", std::string("rest_area"));
if (intersecting_edge->has_sign()) {
const valhalla::TripSign& trip_leg_sign = intersecting_edge->sign();
// I've looked at the results from guide_destinations(), destinations(), and
// exit_destinations(). exit_destinations() doe not contain rest-area names.
// guide_destinations() and destinations() return the same string value for
// the rest area name. So I've decided to use guide_destinations().
std::string guide_destinations_str = guide_destinations(trip_leg_sign);
rest_stop->emplace("name", guide_destinations_str);
if (!sign_text.empty()) {
rest_stop->emplace("name", sign_text);
}
intersection->emplace("rest_stop", rest_stop);
break;
} else if (routeable && intersecting_edge->use() == TripLeg_Use_kServiceAreaUse) {
rest_stop->emplace("type", std::string("service_area"));
if (!sign_text.empty()) {
rest_stop->emplace("name", sign_text);
}
intersection->emplace("rest_stop", rest_stop);
break;
}
Expand Down
25 changes: 14 additions & 11 deletions test/gurka/test_use.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ class Use : public ::testing::Test {
{"BC",
{{"highway", "motorway_link"},
{"service", "rest_area"},
{"destination", "Bear Peak"}}},
{"destination", "Bear Peak Rest Area"}}},
{"BD", {{"highway", "motorway"}}},
{"DE", {{"highway", "motorway"}}},
{"EF",
{{"highway", "motorway_link"},
{"service", "rest_area"},
{"amenity", "yes"},
{"destination", "Service Area"}}},
{"destination", "Bear Peak Service Area"}}},
{"EG", {{"highway", "motorway"}}}};

const auto layout = gurka::detail::map_to_coordinates(ascii_map, gridsize_metres);
Expand Down Expand Up @@ -90,7 +90,8 @@ TEST_F(Use, test_passing_rest_area) {
EXPECT_FALSE(steps[step_idx]["intersections"][0].HasMember("rest_stop"));
EXPECT_TRUE(steps[step_idx]["intersections"][1].HasMember("rest_stop"));
EXPECT_STREQ(steps[step_idx]["intersections"][1]["rest_stop"]["type"].GetString(), "rest_area");
EXPECT_STREQ(steps[step_idx]["intersections"][1]["rest_stop"]["name"].GetString(), "Bear Peak");
EXPECT_STREQ(steps[step_idx]["intersections"][1]["rest_stop"]["name"].GetString(),
"Bear Peak Rest Area");

// Expect no rest stops on last step
step_idx++;
Expand Down Expand Up @@ -125,10 +126,10 @@ TEST_F(Use, test_entering_rest_area) {

// Verify the second maneuver instructions
gurka::assert::raw::expect_instructions_at_maneuver_index(
result, step_idx, "Turn left toward Bear Peak.",
"Turn left toward Bear Peak. Then You will arrive at your destination.",
"Turn left toward Bear Peak.",
"Turn left toward Bear Peak. Then You will arrive at your destination.",
result, step_idx, "Turn left toward Bear Peak Rest Area.",
Copy link
Member

Choose a reason for hiding this comment

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

these expected results seem odd - I would try to mark all the edges with oneway - to be closer to real world. I know we still have more work to for Rest/Service Areas Use to make them work as exits instead of forks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. I trust what you say is true. However, I didn't change this behavior. I'm just giving more real world names to the rest-area and service-area in this test.

Copy link
Member

Choose a reason for hiding this comment

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

@ktatterso we will fix later - thanks

"Turn left toward Bear Peak Rest Area. Then You will arrive at your destination.",
"Turn left toward Bear Peak Rest Area.",
"Turn left toward Bear Peak Rest Area. Then You will arrive at your destination.",
"Continue for 400 meters.");

// Expect no rest stops on third step
Expand Down Expand Up @@ -158,6 +159,8 @@ TEST_F(Use, test_passing_service_area) {
EXPECT_FALSE(steps[step_idx]["intersections"][0].HasMember("rest_stop"));
EXPECT_TRUE(steps[step_idx]["intersections"][1].HasMember("rest_stop"));
EXPECT_STREQ(steps[step_idx]["intersections"][1]["rest_stop"]["type"].GetString(), "service_area");
EXPECT_STREQ(steps[step_idx]["intersections"][1]["rest_stop"]["name"].GetString(),
"Bear Peak Service Area");

// Expect no rest stops on last step
step_idx++;
Expand Down Expand Up @@ -192,10 +195,10 @@ TEST_F(Use, test_entering_service_area) {

// Verify the second maneuver instructions
gurka::assert::raw::expect_instructions_at_maneuver_index(
result, step_idx, "Turn left toward Service Area.",
"Turn left toward Service Area. Then You will arrive at your destination.",
"Turn left toward Service Area.",
"Turn left toward Service Area. Then You will arrive at your destination.",
result, step_idx, "Turn left toward Bear Peak Service Area.",
"Turn left toward Bear Peak Service Area. Then You will arrive at your destination.",
"Turn left toward Bear Peak Service Area.",
"Turn left toward Bear Peak Service Area. Then You will arrive at your destination.",
"Continue for 400 meters.");

// Expect no rest stops on third step
Expand Down