On 02/03/2015 04:41 PM, Dirk Hohndel wrote:
On Tue, Feb 03, 2015 at 04:32:40PM +0100, Patrick Valsecchi wrote:
Hi,
This is a tentative to make subsurface a bit more flexible when parsing GPS
coordinates. I like to geo-reference my dives, but copy and pasting the
coordinates from web sites was quite a pain (needed a lot of manual
correction).
Please have a look and comment. I'm quite tempted to go wild and do another
rewrite without using regular expressions... would be more flexible and less
code, IMHO.
Absolutely. By all means, please rewrite it without regular expression if
that makes it more flexible.
Given where we are in the testing cycle, I won't apply this fix to master.
I'm hoping to cut 4.4 in the next day or two and there's no way this will
get enough testing.
OK, here is a total rewrite of the GPS coordinates parsing including UTs.
It is a lot more flexible on the format and supports all the formats of
the previous parser (AFAIK).
I share your opinion on not including it in 4.4.
>From a1104086f8de2921648e9997983507e6ae3f8753 Mon Sep 17 00:00:00 2001
From: Patrick Valsecchi <[email protected]>
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.
Signed-off-by: Patrick Valsecchi <[email protected]>
---
CMakeLists.txt | 16 ++--
Documentation/user-manual.txt | 3 +
qthelper.cpp | 204 ++++++++++++++++++++----------------------
tests/testgpscoords.cpp | 95 ++++++++++++++++++++
tests/testgpscoords.h | 28 ++++++
5 files changed, 233 insertions(+), 113 deletions(-)
create mode 100644 tests/testgpscoords.cpp
create mode 100644 tests/testgpscoords.h
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8c8cad1..4a260e7 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 7129f61..465f083 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 0c1c68a..b44f40d 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 <exif.h>
#include "file.h"
#include <QFile>
#include <QRegExp>
#include <QDir>
-#include <QMap>
#include <QDebug>
#include <QSettings>
#include <libxslt/documents.h>
#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,105 @@ 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 0000000..a38dcc8
--- /dev/null
+++ b/tests/testgpscoords.cpp
@@ -0,0 +1,95 @@
+#include <dive.h>
+#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 0000000..5add3da
--- /dev/null
+++ b/tests/testgpscoords.h
@@ -0,0 +1,28 @@
+#ifndef TESTGPSCOORDS_H
+#define TESTGPSCOORDS_H
+
+#include <QtTest>
+
+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
--
2.1.0
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
