diff --git a/src/ptv2_checker.cpp b/src/ptv2_checker.cpp index 5b3b180..8721e5e 100644 --- a/src/ptv2_checker.cpp +++ b/src/ptv2_checker.cpp @@ -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"); } @@ -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(*obj_it); for (const auto nref : way->nodes()) { node_ids_rank.emplace_back(nref.ref(), way_count); @@ -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(object); m_writer.write_error_point(relation, node->id(), node->location(), "empty role for non-way object", 0); @@ -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())) { @@ -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) { @@ -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; } @@ -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 { @@ -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); diff --git a/src/ptv2_checker.hpp b/src/ptv2_checker.hpp index 7b0aab1..0fc5e01 100644 --- a/src/ptv2_checker.hpp +++ b/src/ptv2_checker.hpp @@ -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`)? * diff --git a/test/t/test_gap_detection.cpp b/test/t/test_gap_detection.cpp index 1f52087..9591a72 100644 --- a/test/t/test_gap_detection.cpp +++ b/test/t/test_gap_detection.cpp @@ -53,6 +53,10 @@ TEST_CASE("check if gap detection works") { std::map tags1; tags1.emplace("highway", "secondary"); + std::map tags2; + tags2.emplace("highway", "secondary"); + tags2.emplace("oneway", "yes"); + static constexpr int buffer_size = 10 * 1000 * 1000; osmium::memory::Buffer buffer(buffer_size); @@ -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 node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)}; + std::vector node_refs2 {new osmium::NodeRef(7), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(4)}; + std::vector 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 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 node_refs1 {new osmium::NodeRef(4), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(1)}; + std::vector node_refs2 {new osmium::NodeRef(4), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)}; + std::vector 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 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 node_refs1 {new osmium::NodeRef(4), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(1)}; + std::vector node_refs2 {new osmium::NodeRef(4), new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)}; + std::vector 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 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 node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)}; + std::vector node_refs2 {new osmium::NodeRef(7), new osmium::NodeRef(6), new osmium::NodeRef(5), new osmium::NodeRef(4)}; + std::vector 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 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 node_refs1 {new osmium::NodeRef(1), new osmium::NodeRef(2), new osmium::NodeRef(3), new osmium::NodeRef(4)}; std::vector node_refs2 {new osmium::NodeRef(5), new osmium::NodeRef(6), new osmium::NodeRef(7)}; @@ -166,6 +238,10 @@ TEST_CASE("check if gap detection works") { std::map tags1; tags1.emplace("highway", "secondary"); + std::map tags2; + tags2.emplace("highway", "secondary"); + tags2.emplace("oneway", "yes"); + static constexpr int buffer_size = 10 * 1000 * 1000; osmium::memory::Buffer buffer(buffer_size);