From 1cbcae9795ff15de4513f19581e1b84468d28d12 Mon Sep 17 00:00:00 2001 From: James Turner Date: Mon, 8 Oct 2018 16:40:15 +0100 Subject: [PATCH] Bugfix: fix range/ident priority of loaded wpts When the lat-lon of a waypoint disagrees with the discovered ident by more than a threshold, assume we have a DB mismatch, and just revert to a basic wpt specified by lat-lon. This avoids inserting huge legs by selecting a very distant navaid with matching ident https://sourceforge.net/p/flightgear/codetickets/1814/ --- src/Navaids/FlightPlan.cxx | 5 +- src/Navaids/airways.cxx | 1 + src/Navaids/route.cxx | 3 ++ src/Navaids/waypoint.cxx | 12 ++++- .../unit_tests/Navaids/test_flightplan.cxx | 48 +++++++++++++++++++ .../unit_tests/Navaids/test_flightplan.hxx | 9 ++-- 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/Navaids/FlightPlan.cxx b/src/Navaids/FlightPlan.cxx index b57359452..d33401b99 100644 --- a/src/Navaids/FlightPlan.cxx +++ b/src/Navaids/FlightPlan.cxx @@ -1111,9 +1111,8 @@ void FlightPlan::loadVersion2XMLRoute(SGPropertyNode_ptr routeData) _legs.clear(); SGPropertyNode_ptr routeNode = routeData->getChild("route", 0); if (routeNode.valid()) { - for (int i=0; inChildren(); ++i) { - SGPropertyNode_ptr wpNode = routeNode->getChild("wp", i); - Leg* l = new Leg(this, Waypt::createFromProperties(this, wpNode)); + for (auto wpNode : routeNode->getChildren("wp")) { + Leg* l = new Leg{this, Waypt::createFromProperties(this, wpNode)}; _legs.push_back(l); } // of route iteration } diff --git a/src/Navaids/airways.cxx b/src/Navaids/airways.cxx index f4a309742..c30f82ba7 100644 --- a/src/Navaids/airways.cxx +++ b/src/Navaids/airways.cxx @@ -369,6 +369,7 @@ AirwayRef Airway::findByIdentAndNavaid(const std::string& aIdent, const FGPositi WayptRef Airway::findEnroute(const std::string &aIdent) const { + loadWaypoints(); auto it = std::find_if(_elements.begin(), _elements.end(), [&aIdent](WayptRef w) { diff --git a/src/Navaids/route.cxx b/src/Navaids/route.cxx index 87ffb6429..6bf3915db 100644 --- a/src/Navaids/route.cxx +++ b/src/Navaids/route.cxx @@ -256,6 +256,9 @@ WayptRef Waypt::createFromProperties(RouteBase* aOwner, SGPropertyNode_ptr aProp // if we failed to make the waypoint, try again making a basic waypoint. // this handles the case where a navaid waypoint is missing, for example +// we also reject navaids that don't look correct (too far form the specified +// lat-lon, eg see https://sourceforge.net/p/flightgear/codetickets/1814/ ) +// and again fallback to here. WayptRef nd(new BasicWaypt(aOwner)); nd->initFromProperties(aProp); return nd; diff --git a/src/Navaids/waypoint.cxx b/src/Navaids/waypoint.cxx index a0b6ac9e8..e8701e489 100644 --- a/src/Navaids/waypoint.cxx +++ b/src/Navaids/waypoint.cxx @@ -119,11 +119,21 @@ void NavaidWaypoint::initFromProperties(SGPropertyNode_ptr aProp) // FIXME - resolve co-located DME, etc // is it sufficent just to ignore DMEs, actually? - FGPositionedRef nav = FGPositioned::findClosestWithIdent(idn, p, NULL); + FGPositionedRef nav = FGPositioned::findClosestWithIdent(idn, p, nullptr); if (!nav) { throw sg_io_exception("unknown navaid ident:" + idn, "NavaidWaypoint::initFromProperties"); } + + if (p.isValid() && (SGGeodesy::distanceM(nav->geod(), p) > 4000)) { + // the looked up navaid was more than 4000 metres from the lat/lon. + // in this case, throw an exception here so we fall back to using + // a basic waypoint + // see https://sourceforge.net/p/flightgear/codetickets/1814/ + throw sg_io_exception("Waypoint navaid for ident:" + idn + + " is too far from specified lat/lon in flight-plan", + "NavaidWaypoint::initFromProperties"); + } _navaid = nav; } diff --git a/test_suite/unit_tests/Navaids/test_flightplan.cxx b/test_suite/unit_tests/Navaids/test_flightplan.cxx index c214ef64d..00feb3dfd 100644 --- a/test_suite/unit_tests/Navaids/test_flightplan.cxx +++ b/test_suite/unit_tests/Navaids/test_flightplan.cxx @@ -226,3 +226,51 @@ void FlightplanTests::testParseICANLowLevelRoute() bool ok = f->parseICAORouteString(route); CPPUNIT_ASSERT(ok); } + +// https://sourceforge.net/p/flightgear/codetickets/1814/ +void FlightplanTests::testBug1814() +{ + const std::string fpXML = R"( + + 2 + + SAWG + 25 + + + SUMU + + + + navaid + PUGLI + -60.552200 + -40.490000 + + + navaid + SIGUL + -59.655800 + -38.312800 + + + navaid + MDP + -57.576400 + -37.929800 + + + + )"; + + std::istringstream stream(fpXML); + FlightPlanRef f = new FlightPlan; + bool ok = f->load(stream); + CPPUNIT_ASSERT(ok); + + auto leg = f->legAtIndex(1); + CPPUNIT_ASSERT_EQUAL(std::string("SIGUL"), leg->waypoint()->ident()); + + CPPUNIT_ASSERT_DOUBLES_EQUAL(137, leg->distanceNm(), 0.5); + CPPUNIT_ASSERT_DOUBLES_EQUAL(101, f->legAtIndex(2)->distanceNm(), 0.5); +} diff --git a/test_suite/unit_tests/Navaids/test_flightplan.hxx b/test_suite/unit_tests/Navaids/test_flightplan.hxx index 0e8d0aaf3..6d849012e 100644 --- a/test_suite/unit_tests/Navaids/test_flightplan.hxx +++ b/test_suite/unit_tests/Navaids/test_flightplan.hxx @@ -18,8 +18,8 @@ */ -#ifndef _FG_FLIGHTPLAN_UNIT_TESTS_HXX -#define _FG_FLIGHTPLAN_UNIT_TESTS_HXX +#ifndef FG_FLIGHTPLAN_UNIT_TESTS_HXX +#define FG_FLIGHTPLAN_UNIT_TESTS_HXX #include @@ -37,6 +37,8 @@ class FlightplanTests : public CppUnit::TestFixture CPPUNIT_TEST(testRoutePathTrivialFlightPlan); CPPUNIT_TEST(testBasicAirways); CPPUNIT_TEST(testAirwayNetworkRoute); + CPPUNIT_TEST(testBug1814); + // CPPUNIT_TEST(testParseICAORoute); // CPPUNIT_TEST(testParseICANLowLevelRoute); CPPUNIT_TEST_SUITE_END(); @@ -57,6 +59,7 @@ public: void testAirwayNetworkRoute(); void testParseICAORoute(); void testParseICANLowLevelRoute(); + void testBug1814(); }; -#endif // _FG_FLIGHTPLAN_UNIT_TESTS_HXX +#endif // FG_FLIGHTPLAN_UNIT_TESTS_HXX