From 822cce1ef79507a7a0121a4cb0267b3a04a95c9b Mon Sep 17 00:00:00 2001 From: Patrick Valsecchi Date: Wed, 4 Feb 2015 09:30:24 +0100 Subject: [PATCH] More tolerant when parsing GPS coordinates. Refactored the parsing logic to make it more solid (no more guessing) and more flexible (support more formats). Added a test for checking that. Fixed a few warnings. [Dirk Hohndel: some changes to coding style] Signed-off-by: Patrick Valsecchi Signed-off-by: Dirk Hohndel --- CMakeLists.txt | 16 +-- Documentation/user-manual.txt | 3 + qthelper.cpp | 211 +++++++++++++++++----------------- tests/testgpscoords.cpp | 95 +++++++++++++++ tests/testgpscoords.h | 28 +++++ 5 files changed, 240 insertions(+), 113 deletions(-) create mode 100644 tests/testgpscoords.cpp create mode 100644 tests/testgpscoords.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 8c8cad1bb..4a260e729 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -235,11 +235,13 @@ ADD_DEPENDENCIES(subsurface_interface subsurface_generated_ui) ADD_DEPENDENCIES(subsurface_generated_ui version) ADD_DEPENDENCIES(subsurface_corelib version) -ENABLE_TESTING() -ADD_EXECUTABLE(TestUnitConversion tests/testunitconversion.cpp) -TARGET_LINK_LIBRARIES(TestUnitConversion subsurface_corelib ${QT_TEST_LIBRARIES} ${SUBSURFACE_LINK_LIBRARIES} -lzip -ldivecomputer) -ADD_TEST(NAME TestUnitConversion COMMAND TestUnitConversion) +MACRO(test NAME FILE) + ADD_EXECUTABLE(${NAME} tests/${FILE}) + TARGET_LINK_LIBRARIES(${NAME} subsurface_corelib ${QT_TEST_LIBRARIES} ${SUBSURFACE_LINK_LIBRARIES} -lzip -ldivecomputer) + ADD_TEST(NAME ${NAME} COMMAND ${NAME}) +ENDMACRO() -ADD_EXECUTABLE(TestProfile tests/testprofile.cpp) -TARGET_LINK_LIBRARIES(TestProfile subsurface_corelib ${QT_TEST_LIBRARIES} ${SUBSURFACE_LINK_LIBRARIES} -lzip -ldivecomputer) -ADD_TEST(NAME TestProfile COMMAND TestProfile) +ENABLE_TESTING() +test(TestUnitConversion testunitconversion.cpp) +test(TestProfile testprofile.cpp) +test(TestGpsCoords testgpscoords.cpp) diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index e13ea098e..886c687d7 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -277,6 +277,9 @@ Southern hemisphere latitudes are given with a *S*, e.g. S30°, or with a negative value, e.g. -30.22496. Similarly western longitudes are given with a *W*, e.g. W07°, or with a negative value, e.g. -7.34323. +Some keyboards don't have the degree sign (°). It can be replaced by a d like +that: N30d W20d. + Please note that GPS coordinates of a dive site are linked to the Location name - so adding coordinates to dives that do not have a location description will cause unexpected behaviour (Subsurface will think that all of these diff --git a/qthelper.cpp b/qthelper.cpp index 0c1c68ab1..acb1e103a 100644 --- a/qthelper.cpp +++ b/qthelper.cpp @@ -1,19 +1,17 @@ #include "qthelper.h" -#include "qt-gui.h" #include "gettextfromc.h" -#include "dive.h" #include "statistics.h" #include #include "file.h" #include #include #include -#include #include #include #include #define translate(_context, arg) trGettext(arg) +static const QString DEGREE_SIGNS("dD" UTF8_DEGREE); QString weight_string(int weight_in_grams) { @@ -47,10 +45,10 @@ QString printGPSCoords(int lat, int lon) lonh = lon >= 0 ? translate("gettextFromC", "E") : translate("gettextFromC", "W"); lat = abs(lat); lon = abs(lon); - latdeg = lat / 1000000; - londeg = lon / 1000000; - latmin = (lat % 1000000) * 60; - lonmin = (lon % 1000000) * 60; + latdeg = lat / 1000000U; + londeg = lon / 1000000U; + latmin = (lat % 1000000U) * 60U; + lonmin = (lon % 1000000U) * 60U; latsec = (latmin % 1000000) * 60; lonsec = (lonmin % 1000000) * 60; result.sprintf("%u%s%02d\'%06.3f\"%s %u%s%02d\'%06.3f\"%s", @@ -59,111 +57,112 @@ QString printGPSCoords(int lat, int lon) return result; } +static bool parseCoord(const QString& txt, int& pos, const QString& positives, + const QString& negatives, const QString& others, + double& value) +{ + bool numberDefined = false, degreesDefined = false, + minutesDefined = false, secondsDefined = false; + double number = 0.0; + int posBeforeNumber = pos; + int sign = 0; + value = 0.0; + while (pos < txt.size()) { + if (txt[pos].isDigit()) { + if (numberDefined) + return false; + QRegExp numberRe("(\\d+(?:[\\.,]\\d+)?).*"); + if (!numberRe.exactMatch(txt.mid(pos))) + return false; + number = numberRe.cap(1).toDouble(); + numberDefined = true; + posBeforeNumber = pos; + pos += numberRe.cap(1).size() - 1; + } else if (positives.indexOf(txt[pos].toUpper()) >= 0) { + if (sign != 0) + return false; + sign = 1; + if (degreesDefined || numberDefined) { + //sign after the degrees => + //at the end of the coordinate + ++pos; + break; + } + } else if (negatives.indexOf(txt[pos].toUpper()) >= 0) { + if (sign != 0) { + if (others.indexOf(txt[pos]) >= 0) + //special case for the '-' sign => next coordinate + break; + return false; + } + sign = -1; + if (degreesDefined || numberDefined) { + //sign after the degrees => + //at the end of the coordinate + ++pos; + break; + } + } else if (others.indexOf(txt[pos].toUpper()) >= 0) { + //we are at the next coordinate. + break; + } else if (DEGREE_SIGNS.indexOf(txt[pos]) >= 0) { + if (!numberDefined) + return false; + if (degreesDefined) { + //next coordinate => need to put back the number + pos = posBeforeNumber; + numberDefined = false; + break; + } + value += number; + numberDefined = false; + degreesDefined = true; + } else if (txt[pos] == '\'') { + if (!numberDefined || minutesDefined) + return false; + value += number / 60.0; + numberDefined = false; + minutesDefined = true; + } else if (txt[pos] == '"') { + if (!numberDefined || secondsDefined) + return false; + value += number / 3600.0; + numberDefined = false; + secondsDefined = true; + } else if (txt[pos] == ' ' || txt[pos] == '\t') { + //ignore spaces + } else { + return false; + } + ++pos; + } + if (!degreesDefined && numberDefined) { + value = number; //just a single number => degrees + numberDefined = false; + degreesDefined = true; + } + if (!degreesDefined || numberDefined) + return false; + if (sign == -1) value *= -1.0; + return true; +} + bool parseGpsText(const QString &gps_text, double *latitude, double *longitude) { - enum { - ISO6709D, - SECONDS, - MINUTES, - DECIMAL - } gpsStyle = ISO6709D; - int eastWest = 4; - int northSouth = 1; - QString trHemisphere[4]; - trHemisphere[0] = translate("gettextFromC", "N"); - trHemisphere[1] = translate("gettextFromC", "S"); - trHemisphere[2] = translate("gettextFromC", "E"); - trHemisphere[3] = translate("gettextFromC", "W"); - QString regExp; - /* an empty string is interpreted as 0.0,0.0 and therefore "no gps location" */ - if (gps_text.trimmed().isEmpty()) { + const QString trimmed = gps_text.trimmed(); + if (trimmed.isEmpty()) { *latitude = 0.0; *longitude = 0.0; return true; } - // trying to parse all formats in one regexp might be possible, but it seems insane - // so handle the four formats we understand separately - - // ISO 6709 Annex D representation - // http://en.wikipedia.org/wiki/ISO_6709#Representation_at_the_human_interface_.28Annex_D.29 - // e.g. 52°49'02.388"N 1°36'17.388"E - if (gps_text.at(0).isDigit() && (gps_text.count(",") % 2) == 0) { - gpsStyle = ISO6709D; - regExp = QString("(\\d+)[" UTF8_DEGREE "\\s](\\d+)[\'\\s](\\d+)([,\\.](\\d+))?[\"\\s]([NS%1%2])" - "\\s*(\\d+)[" UTF8_DEGREE "\\s](\\d+)[\'\\s](\\d+)([,\\.](\\d+))?[\"\\s]([EW%3%4])") - .arg(trHemisphere[0]) - .arg(trHemisphere[1]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]); - } else if (gps_text.count(QChar('"')) == 2) { - gpsStyle = SECONDS; - regExp = QString("\\s*([NS%1%2])\\s*(\\d+)[" UTF8_DEGREE "\\s]+(\\d+)[\'\\s]+(\\d+)([,\\.](\\d+))?[^EW%3%4]*" - "([EW%5%6])\\s*(\\d+)[" UTF8_DEGREE "\\s]+(\\d+)[\'\\s]+(\\d+)([,\\.](\\d+))?") - .arg(trHemisphere[0]) - .arg(trHemisphere[1]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]); - } else if (gps_text.count(QChar('\'')) == 2) { - gpsStyle = MINUTES; - regExp = QString("\\s*([NS%1%2])\\s*(\\d+)[" UTF8_DEGREE "\\s]+(\\d+)([,\\.](\\d+))?[^EW%3%4]*" - "([EW%5%6])\\s*(\\d+)[" UTF8_DEGREE "\\s]+(\\d+)([,\\.](\\d+))?") - .arg(trHemisphere[0]) - .arg(trHemisphere[1]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]); - } else { - gpsStyle = DECIMAL; - regExp = QString("\\s*([-NS%1%2]?)\\s*(\\d+)[,\\.](\\d+)[^-EW%3%4\\d]*([-EW%5%6]?)\\s*(\\d+)[,\\.](\\d+)") - .arg(trHemisphere[0]) - .arg(trHemisphere[1]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]) - .arg(trHemisphere[2]) - .arg(trHemisphere[3]); - } - QRegExp r(regExp); - if (r.indexIn(gps_text) != -1) { - // qDebug() << "Hemisphere" << r.cap(1) << "deg" << r.cap(2) << "min" << r.cap(3) << "decimal" << r.cap(4); - // qDebug() << "Hemisphere" << r.cap(5) << "deg" << r.cap(6) << "min" << r.cap(7) << "decimal" << r.cap(8); - switch (gpsStyle) { - case ISO6709D: - *latitude = r.cap(1).toInt() + r.cap(2).toInt() / 60.0 + - (r.cap(3) + QString(".") + r.cap(5)).toDouble() / 3600.0; - *longitude = r.cap(7).toInt() + r.cap(8).toInt() / 60.0 + - (r.cap(9) + QString(".") + r.cap(11)).toDouble() / 3600.0; - northSouth = 6; - eastWest = 12; - break; - case SECONDS: - *latitude = r.cap(2).toInt() + r.cap(3).toInt() / 60.0 + - (r.cap(4) + QString(".") + r.cap(6)).toDouble() / 3600.0; - *longitude = r.cap(8).toInt() + r.cap(9).toInt() / 60.0 + - (r.cap(10) + QString(".") + r.cap(12)).toDouble() / 3600.0; - eastWest = 7; - break; - case MINUTES: - *latitude = r.cap(2).toInt() + (r.cap(3) + QString(".") + r.cap(5)).toDouble() / 60.0; - *longitude = r.cap(7).toInt() + (r.cap(8) + QString(".") + r.cap(10)).toDouble() / 60.0; - eastWest = 6; - break; - case DECIMAL: - default: - *latitude = (r.cap(2) + QString(".") + r.cap(3)).toDouble(); - *longitude = (r.cap(5) + QString(".") + r.cap(6)).toDouble(); - break; - } - if (r.cap(northSouth) == "S" || r.cap(northSouth) == trHemisphere[1] || r.cap(northSouth) == "-") - *latitude *= -1.0; - if (r.cap(eastWest) == "W" || r.cap(eastWest) == trHemisphere[3] || r.cap(eastWest) == "-") - *longitude *= -1.0; - // qDebug("%s -> %8.5f / %8.5f", gps_text.toLocal8Bit().data(), *latitude, *longitude); - return true; - } - return false; + int pos = 0; + static const QString POS_LAT = QString("+N") + translate("gettextFromC", "N"); + static const QString NEG_LAT = QString("-S") + translate("gettextFromC", "S"); + static const QString POS_LON = QString("+E") + translate("gettextFromC", "E"); + static const QString NEG_LON = QString("-W") + translate("gettextFromC", "W"); + return parseCoord(gps_text, pos, POS_LAT, NEG_LAT, POS_LON + NEG_LON, *latitude) && + parseCoord(gps_text, pos, POS_LON, NEG_LON, "", *longitude) && + pos == gps_text.size(); } bool gpsHasChanged(struct dive *dive, struct dive *master, const QString &gps_text, bool *parsed_out) diff --git a/tests/testgpscoords.cpp b/tests/testgpscoords.cpp new file mode 100644 index 000000000..a38dcc807 --- /dev/null +++ b/tests/testgpscoords.cpp @@ -0,0 +1,95 @@ +#include +#include "testgpscoords.h" + +//unit under test +extern bool parseGpsText(const QString &gps_text, double *latitude, double *longitude); + + +void TestGpsCoords::testISO6709DParse() +{ + testParseOK("52°49'02.388\"N 1°36'17.388\"E", + coord2double(52, 49, 2.388), coord2double(1, 36, 17.388)); +} + +void TestGpsCoords::testNegativeISO6709DParse() +{ + testParseOK("52°49'02.388\"S 1°36'17.388\"W", + coord2double(-52, -49, -2.388), coord2double(-1, -36, -17.388)); +} + +void TestGpsCoords::testSpaceISO6709DParse() +{ + testParseOK("52 ° 49 ' 02.388 \" N 1 ° 36 ' 17.388 \" E", + coord2double(52, 49, 2.388), coord2double(1, 36, 17.388)); +} + +void TestGpsCoords::testSecondsParse() +{ + testParseOK("N52°49'02.388\" E1°36'17.388\"", + coord2double(52, 49, 2.388), coord2double(1, 36, 17.388)); +} + +void TestGpsCoords::testSpaceSecondsParse() +{ + testParseOK(" N 52 ° 49 ' 02.388 \" E 1 ° 36 ' 17.388 \"", + coord2double(52, 49, 2.388), coord2double(1, 36, 17.388)); +} + +void TestGpsCoords::testNegativeSecondsParse() +{ + testParseOK("-52°49'02.388\" -1°36'17.388\"", + coord2double(-52, -49, -2.388), coord2double(-1, -36, -17.388)); +} + +void TestGpsCoords::testMinutesParse() +{ + testParseOK("N52°49.03' E1d36.23'", + coord2double(52, 49.03), coord2double(1, 36.23)); +} + +void TestGpsCoords::testSpaceMinutesParse() +{ + testParseOK(" N 52 ° 49.03 ' E 1 ° 36.23 ' ", + coord2double(52, 49.03), coord2double(1, 36.23)); +} + +void TestGpsCoords::testMinutesInversedParse() +{ + testParseOK("2° 53.836' N 73° 32.839' E", + coord2double(2, 53.836), coord2double(73, 32.839)); +} + +void TestGpsCoords::testDecimalParse() +{ + testParseOK("N52.83° E1.61", + coord2double(52.83), coord2double(1.61)); +} + +void TestGpsCoords::testDecimalInversedParse() +{ + testParseOK("52.83N 1.61E", + coord2double(52.83), coord2double(1.61)); +} + +void TestGpsCoords::testSpaceDecimalParse() +{ + testParseOK(" N 52.83 E 1.61 ", + coord2double(52.83), coord2double(1.61)); +} + + +void TestGpsCoords::testParseOK(const QString &txt, double expectedLat, + double expectedLon) +{ + double actualLat, actualLon; + QVERIFY(parseGpsText(txt, &actualLat, &actualLon)); + QCOMPARE(actualLat, expectedLat); + QCOMPARE(actualLon, expectedLon); +} + +double TestGpsCoords::coord2double(double deg, double min, double sec) +{ + return deg + min / 60.0 + sec / 3600.0; +} + +QTEST_MAIN(TestGpsCoords) diff --git a/tests/testgpscoords.h b/tests/testgpscoords.h new file mode 100644 index 000000000..5add3da93 --- /dev/null +++ b/tests/testgpscoords.h @@ -0,0 +1,28 @@ +#ifndef TESTGPSCOORDS_H +#define TESTGPSCOORDS_H + +#include + +class TestGpsCoords : public QObject { +Q_OBJECT +private slots: + void testISO6709DParse(); + void testNegativeISO6709DParse(); + void testSpaceISO6709DParse(); + void testSecondsParse(); + void testSpaceSecondsParse(); + void testNegativeSecondsParse(); + void testMinutesParse(); + void testSpaceMinutesParse(); + void testMinutesInversedParse(); + void testDecimalParse(); + void testSpaceDecimalParse(); + void testDecimalInversedParse(); + +private: + static void testParseOK(const QString &txt, double expectedLat, + double expectedLon); + static double coord2double(double deg, double min = 0.0, double sec = 0.0); +}; + +#endif