Improve on location auto-gps editing

Linus Torvalds torvalds at linux-foundation.org
Mon Nov 11 09:21:47 UTC 2013


This is an incredibly ugly patch to try to improve a bit on the gps 
location auto-editing when editing the location string.

In particular, current subsurface completely screws that up if you 
*already* have GPS information for the dive (for example, because you 
downloaded it from the subsurface companion app from your phone).

If you then edit the location field, and enter a location that also has a 
gps entry, the already existing (correct) gps data will be over-written by 
that old gps data from a previous dive that just happened to have the same 
location name. That is very very wrong.

So we should only ever do the auto-editing of the gps information if

 - there was no previous gps information

 - the previous gps information was due to our auto-gps logic (ie we 
   changed the location multiple times).

and in particular, we should *NEVER* change the gps information if we 
already had some, or if it was edited by hand (the exception being "edited 
to empty" - we always auto-fill empty gps data).

See commit de1401144ce0 ("Add default GPS location for dive sites we 
already know about") from the Gtk branch. This patch does things 
differently, but the intent is the same.

I'm not very pleased with the patch, and it's extremely hacky to have this 
silly "gps_set_by_hand" static flag that we set that isn't cleanly 
associated with the actual gps string (compare this with the gtk version 
that at least had that "location_update" structure). But this gets closer 
to usable, unlike the current completely broken behavior.

Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

Tomaz - maybe there is some nicer way to embed that "set_by_hand" flag 
into the actual ui data structure. This separate flag is kind of hacky, 
to say the least. Possibly together with a helper wrapper that does that 
whole

    ui.coordinates->setText(printGPSCoords(d->latitude.udeg, d->longitude.udeg));

initialization? 

It should get more testing. I tried to exercise the relevant cases, but 
maybe I missed some...

 qt-ui/maintab.cpp | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/qt-ui/maintab.cpp b/qt-ui/maintab.cpp
index 0c0d99a35c33..a94e7f3af728 100644
--- a/qt-ui/maintab.cpp
+++ b/qt-ui/maintab.cpp
@@ -23,6 +23,8 @@
 #include <QSettings>
 #include <QPalette>
 
+static int gps_set_by_hand = 0;
+
 MainTab::MainTab(QWidget *parent) : QTabWidget(parent),
 				    weightModel(new WeightModel()),
 				    cylindersModel(CylindersModel::instance()),
@@ -275,6 +277,8 @@ void MainTab::updateDiveInfo(int dive)
 	struct dive *prevd;
 	struct dive *d = get_dive(dive);
 
+	gps_set_by_hand = 0;
+
 	process_selected_dives();
 	process_all_dives(d, &prevd);
 
@@ -286,6 +290,7 @@ void MainTab::updateDiveInfo(int dive)
 	UPDATE_TEMP(d, airtemp);
 	UPDATE_TEMP(d, watertemp);
 	if (d) {
+		gps_set_by_hand = d->latitude.udeg || d->longitude.udeg;
 		ui.coordinates->setText(printGPSCoords(d->latitude.udeg, d->longitude.udeg));
 		ui.dateTimeEdit->setDateTime(QDateTime::fromTime_t(d->when - gettimezoneoffset()));
 		if (mainWindow() && mainWindow()->dive_list()->selectedTrips.count() == 1) {
@@ -402,6 +407,7 @@ void MainTab::updateDiveInfo(int dive)
 		clearEquipment();
 		ui.rating->setCurrentStars(0);
 		ui.coordinates->clear();
+		gps_set_by_hand = 0;
 		ui.visibility->setCurrentStars(0);
 		/* turns out this is non-trivial for a dateTimeEdit... this is a partial hack */
 		QLineEdit *le = ui.dateTimeEdit->findChild<QLineEdit*>();
@@ -450,6 +456,7 @@ void MainTab::acceptChanges()
 		struct dive *curr = current_dive;
 		//Reset coordinates field, in case it contains garbage.
 		ui.coordinates->setText(printGPSCoords(current_dive->latitude.udeg, current_dive->longitude.udeg));
+		gps_set_by_hand = current_dive->latitude.udeg || current_dive->longitude.udeg;
 		if (notesBackup[curr].buddy != ui.buddy->text() ||
 			notesBackup[curr].suit != ui.suit->text() ||
 			notesBackup[curr].notes != ui.notes->toPlainText() ||
@@ -696,8 +703,22 @@ void MainTab::on_location_textChanged(const QString& text)
 		dive_trip_t *currentTrip = *mainWindow()->dive_list()->selectedTrips.begin();
 		EDIT_TEXT(currentTrip->location, text);
 	} else if (editMode == DIVE || editMode == ADD){
-		if (!ui.coordinates->isModified() ||
-		    ui.coordinates->text().trimmed().isEmpty()) {
+		bool edit_gps;
+
+		/*
+		 * If the gps data is empty, we always try to set it,
+		 * even if that empty state was hand-edited
+		 *
+		 * Else, we set it only if it wasn't set originally,
+		 * _and_ hasn't been modified (but it might have been
+		 * changed by this 'edit_gps' logic)
+		 */
+		if (ui.coordinates->text().trimmed().isEmpty())
+			edit_gps = true;
+		else
+			edit_gps = !gps_set_by_hand && !ui.coordinates->isModified();
+
+		if (edit_gps) {
 			struct dive* dive;
 			int i = 0;
 			for_each_dive(i, dive){


More information about the subsurface mailing list