Skip to content

Commit

Permalink
this implements both geofabrik#19 and geofabrik#20.
Browse files Browse the repository at this point in the history 
hail_and_ride, gap detection for oneway roads included in opposite direction.
  • Loading branch information
@mfrasca
mfrasca committed Jul 7, 2022
1 parent 6296238 commit 8241c92
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 12 deletions.
31 changes: 19 additions & 12 deletions src/ptv2_checker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ RouteType PTv2Checker::get_route_type(const char* route) {
return RouteType::NONE;
}

bool PTv2Checker::is_way(const char* role) {
return !strcmp(role, "") || !strcmp(role, "hail_and_ride");
}

bool PTv2Checker::is_stop(const char* role) {
return !strcmp(role, "stop") || !strcmp(role, "stop_entry_only") || !strcmp(role, "stop_exit_only");
}
Expand Down Expand Up @@ -233,7 +237,7 @@ RouteError PTv2Checker::check_roles_order_and_type(const osmium::Relation& relat
size_t way_count = 0;
for (; obj_it != member_objects.cend(), member_it != relation.members().cend();
++obj_it, ++member_it) {
if (member_it->type() == osmium::item_type::way && !strcmp(member_it->role(), "") && *obj_it) {
if (member_it->type() == osmium::item_type::way && is_way(member_it->role()) && *obj_it) {
const osmium::Way* way = static_cast<const osmium::Way*>(*obj_it);
for (const auto nref : way->nodes()) {
node_ids_rank.emplace_back(nref.ref(), way_count);
Expand Down Expand Up @@ -262,10 +266,10 @@ RouteError PTv2Checker::check_roles_order_and_type(const osmium::Relation& relat
incomplete = true;
}
const osmium::OSMObject* object = *obj_it;
if (member_it->type() == osmium::item_type::way && !strcmp(member_it->role(), "")) {
if (member_it->type() == osmium::item_type::way && is_way(member_it->role())) {
seen_road_member = true;
error |= role_check_handle_road_member(relation, type, object, seen_stop_platform);
} else if (member_it->type() != osmium::item_type::way && !strcmp(member_it->role(), "")) {
} else if (member_it->type() != osmium::item_type::way && is_way(member_it->role())) {
if (member_it->type() == osmium::item_type::node && object) {
const osmium::Node* node = static_cast<const osmium::Node*>(object);
m_writer.write_error_point(relation, node->id(), node->location(), "empty role for non-way object", 0);
Expand All @@ -290,7 +294,7 @@ RouteError PTv2Checker::check_roles_order_and_type(const osmium::Relation& relat
if (object) {
check_platform_tags(relation, type, object);
}
} else if (strcmp(member_it->role(), "") && !is_stop(member_it->role()) && !is_platform(member_it->role())) {
} else if (!is_way(member_it->role()) && !is_stop(member_it->role()) && !is_platform(member_it->role())) {
error |= handle_unknown_role(relation, object, member_it->role());
}
if (member_it->type() == osmium::item_type::node && *obj_it != nullptr && is_stop(member_it->role())) {
Expand Down Expand Up @@ -449,7 +453,7 @@ int PTv2Checker::gap_detector_member_handling(const osmium::Relation& relation,
return 0;
}
// check role and type
if (status == MemberStatus::BEFORE_FIRST && member_it->type() == osmium::item_type::way && !strcmp(role, "")) {
if (status == MemberStatus::BEFORE_FIRST && member_it->type() == osmium::item_type::way && is_way(role)) {
status = MemberStatus::FIRST;
}
if (status == MemberStatus::BEFORE_FIRST) {
Expand All @@ -458,7 +462,7 @@ int PTv2Checker::gap_detector_member_handling(const osmium::Relation& relation,
}

// check if it is a way
if (member_it->type() != osmium::item_type::way || strcmp(role, "")) {
if (member_it->type() != osmium::item_type::way || !is_way(role)) {
status = MemberStatus::AFTER_GAP;
return 0;
}
Expand Down Expand Up @@ -525,18 +529,21 @@ int PTv2Checker::gap_detector_member_handling(const osmium::Relation& relation,
return 1;
}
}
else if (status == MemberStatus::SECOND) {
else if (status == MemberStatus::SECOND) { // we don't know which orientation we used for `previous_way`.
if (previous_way->id() == way->id()) {
// The first and the second way are equal. This is valid in some cases. So we just return.
return 0;
}
//TODO secure for incomplete relations
if (way->nodes().front().ref() == previous_way->nodes().front().ref()
|| way->nodes().front().ref() == previous_way->nodes().back().ref()) {
if (way->nodes().front().ref() == previous_way->nodes().back().ref() ||
(way->nodes().front().ref() == previous_way->nodes().front().ref() && !previous_way->tags().has_tag("oneway", "yes"))) {
// start normal status, entered from the front, exiting from the back.
previous_way_end = BackOrFront::BACK;
status = MemberStatus::NORMAL;
} else if (way->nodes().back().ref() == previous_way->nodes().front().ref()
|| way->nodes().back().ref() == previous_way->nodes().back().ref()) {
} else if (!way->tags().has_tag("oneway", "yes") && // not oneway, then we can enter `way` from the back
(way->nodes().back().ref() == previous_way->nodes().back().ref()
|| (way->nodes().back().ref() == previous_way->nodes().front().ref() && !previous_way->tags().has_tag("oneway", "yes")))) {
// start normal status, entered from the back, exiting from the front.
previous_way_end = BackOrFront::FRONT;
status = MemberStatus::NORMAL;
} else {
Expand All @@ -548,7 +555,7 @@ int PTv2Checker::gap_detector_member_handling(const osmium::Relation& relation,
const osmium::NodeRef* next_node = back_or_front_to_node_ref(previous_way_end, previous_way);
if (way->nodes().front().ref() == next_node->ref()) {
previous_way_end = BackOrFront::BACK;
} else if (way->nodes().back().ref() == next_node->ref()) {
} else if (way->nodes().back().ref() == next_node->ref() && !way->tags().has_tag("oneway", "yes")) {
previous_way_end = BackOrFront::FRONT;
} else {
m_writer.write_error_way(relation, next_node->ref(), "gap", previous_way);
Expand Down
7 changes: 7 additions & 0 deletions src/ptv2_checker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,13 @@ class PTv2Checker {
*/
RouteType get_route_type(const char* route);

/**
* Is the role a way (including the empty string and `hail_and_stop`)?
*
* \param role string
*/
bool is_way(const char* role);

/**
* Is the role a stop (including `stop_exit_only` and `stop_entry_only`)?
*
Expand Down
76 changes: 76 additions & 0 deletions test/t/test_gap_detection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ TEST_CASE("check if gap detection works") {
std::map<std::string, std::string> tags1;
tags1.emplace("highway", "secondary");

std::map<std::string, std::string> tags2;
tags2.emplace("highway", "secondary");
tags2.emplace("oneway", "yes");

static constexpr int buffer_size = 10 * 1000 * 1000;
osmium::memory::Buffer buffer(buffer_size);

Expand All @@ -75,6 +79,74 @@ TEST_CASE("check if gap detection works") {
CHECK(checker.find_gaps(relation1, objects) == 0);
}

SECTION("three ways, second goes in opposite direction, but is not oneway, no gap.") {
std::vector<const osmium::NodeRef*> node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)};
std::vector<const osmium::NodeRef*> node_refs2 {new osmium::NodeRef(7), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(4)};
std::vector<const osmium::NodeRef*> node_refs3 {new osmium::NodeRef(7), new osmium::NodeRef(8), new osmium::NodeRef(9)};

osmium::Way& way1 = test_utils::create_way(buffer, 1, node_refs1, tags1);
buffer.commit();
osmium::Way& way2 = test_utils::create_way(buffer, 2, node_refs2, tags1);
buffer.commit();
osmium::Way& way3 = test_utils::create_way(buffer, 3, node_refs3, tags1);
buffer.commit();
std::vector<const osmium::OSMObject*> objects {nullptr, nullptr, &way1, &way2, &way3};

osmium::Relation& relation1 = test_utils::create_relation(buffer, 1, tags_rel, ids, types, roles, objects);
CHECK(checker.find_gaps(relation1, objects) == 0);
}

SECTION("three ways, first goes in opposite direction, but is not oneway, no gap.") {
std::vector<const osmium::NodeRef*> node_refs1 {new osmium::NodeRef(4), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(1)};
std::vector<const osmium::NodeRef*> node_refs2 {new osmium::NodeRef(4), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)};
std::vector<const osmium::NodeRef*> node_refs3 {new osmium::NodeRef(7), new osmium::NodeRef(8), new osmium::NodeRef(9)};

osmium::Way& way1 = test_utils::create_way(buffer, 1, node_refs1, tags1);
buffer.commit();
osmium::Way& way2 = test_utils::create_way(buffer, 2, node_refs2, tags1);
buffer.commit();
osmium::Way& way3 = test_utils::create_way(buffer, 3, node_refs3, tags1);
buffer.commit();
std::vector<const osmium::OSMObject*> objects {nullptr, nullptr, &way1, &way2, &way3};

osmium::Relation& relation1 = test_utils::create_relation(buffer, 1, tags_rel, ids, types, roles, objects);
CHECK(checker.find_gaps(relation1, objects) == 0);
}

SECTION("three ways, first is oneway in opposite direction, one gap.") {
std::vector<const osmium::NodeRef*> node_refs1 {new osmium::NodeRef(4), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(1)};
std::vector<const osmium::NodeRef*> node_refs2 {new osmium::NodeRef(4), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)};
std::vector<const osmium::NodeRef*> node_refs3 {new osmium::NodeRef(7), new osmium::NodeRef(8), new osmium::NodeRef(9)};

osmium::Way& way1 = test_utils::create_way(buffer, 1, node_refs1, tags2);
buffer.commit();
osmium::Way& way2 = test_utils::create_way(buffer, 2, node_refs2, tags1);
buffer.commit();
osmium::Way& way3 = test_utils::create_way(buffer, 3, node_refs3, tags1);
buffer.commit();
std::vector<const osmium::OSMObject*> objects {nullptr, nullptr, &way1, &way2, &way3};

osmium::Relation& relation1 = test_utils::create_relation(buffer, 1, tags_rel, ids, types, roles, objects);
CHECK(checker.find_gaps(relation1, objects) == 1);
}

SECTION("three ways, second is oneway in opposite direction, seen as one gap.") {
std::vector<const osmium::NodeRef*> node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)};
std::vector<const osmium::NodeRef*> node_refs2 {new osmium::NodeRef(7), new osmium::NodeRef(6), new osmium::NodeRef(5), new osmium::NodeRef(4)};
std::vector<const osmium::NodeRef*> node_refs3 {new osmium::NodeRef(7), new osmium::NodeRef(8), new osmium::NodeRef(9)};

osmium::Way& way1 = test_utils::create_way(buffer, 1, node_refs1, tags1);
buffer.commit();
osmium::Way& way2 = test_utils::create_way(buffer, 2, node_refs2, tags2);
buffer.commit();
osmium::Way& way3 = test_utils::create_way(buffer, 3, node_refs3, tags1);
buffer.commit();
std::vector<const osmium::OSMObject*> objects {nullptr, nullptr, &way1, &way2, &way3};

osmium::Relation& relation1 = test_utils::create_relation(buffer, 1, tags_rel, ids, types, roles, objects);
CHECK(checker.find_gaps(relation1, objects) == 1);
}

SECTION("three ways, gaps between first and second") {
std::vector<const osmium::NodeRef*> node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)};
std::vector<const osmium::NodeRef*> node_refs2 {new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)};
Expand Down Expand Up @@ -166,6 +238,10 @@ TEST_CASE("check if gap detection works") {
std::map<std::string, std::string> tags1;
tags1.emplace("highway", "secondary");

std::map<std::string, std::string> tags2;
tags2.emplace("highway", "secondary");
tags2.emplace("oneway", "yes");

static constexpr int buffer_size = 10 * 1000 * 1000;
osmium::memory::Buffer buffer(buffer_size);

Expand Down

0 comments on commit 8241c92

Please sign in to comment.