Hi Marc.
On Thu, Jul 03, 2014 at 12:08:34PM -0700, Marc MERLIN wrote:
> On Thu, Jun 26, 2014 at 12:36:26AM +0200, Salvador Cuñat wrote:
> > I see two failures in actual implementation:
> >
> > 1.- The one corrected by the patch (the first condition goes nowhere if
> > the position fix is not in the diving period range)
> >
> > 2.- The second part which starts when first condition fails, relays on
> > find_dive_n_near() helper and simply applies the dives to the position
> > fixes secuentially, and do weird things when dive times and (specially)
> > position fixes times reach the six hour difference with the first time
> > fixed. This is not trivial.
>
> Do you have a new bug filed for this, or are you able to file one, or
> would you like me to file one? :)
>
Better still, attached is a patch that I think solves the problem. It
worked well on test positions and test dives, and also on real positions
and dives of my own taken yesterday during a dive trip.
Would you be so kind of trying it on those yours that failed earlier?
> > > Also, if I feed better points via a GPX track, will they override the
> > > bad points?
> >
> > I don't think so. The logic depends on matching position fix time with
> > dive time, you would need to have times in the GPX track that match
> > somehow the times of the DC.
>
> That's the whole point of a GPX track. A track has multiple points per
> minute and can often have 10K points in a single track.
> The current code in subsurface doesn't parse tracks, but if it did (and
> it could borrow code from exiftool or gpsphoto as given in my previous
> Email), it could find a point that within a few seconds and even log
> where you dive started and where it ended for a drift dive.
>
> Wouldn't that be cool? :)
This would deserve a feature request in bug trak. Althoug this is
somehow working via companion app importing GPX files capability.
There would be two problems there:
1.- The positions in GPX file need to be named or won't be parsed by
companion or accepted by the server.
2.- The server is still working in time model hh:mm:00 (yes, zero
seconds) so only accepts one position each minute. Yesterday companion
fixed 6 or 7 positions each minute for me while in the boat, but the
server seems to accept the first uploaded and companion is unable to
upload the others in the minute (I thought I've filed a bug for this but
I can't see it, I'll file another one).
Regards.
Salva.
>From 8acbe99023373bb9503756fe5580ee19fec869d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Salvador=20Cu=C3=B1at?= <salvador.cu...@gmail.com>
Date: Sun, 6 Jul 2014 18:19:54 +0200
Subject: [PATCH] Change in logic while aplying gps fixes to dives
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We were actually searching dives which match the dowloaded position
fixes. So we're also trying to take into account if the fix is automatic
or no based on a limited amount of predefined strings (bad idea, as the
user can change in companion app settings the predefined string).
This way, in actual implementation, if program concludes that a fix has
been manually got or, simply, the user is unlucky enough to have all the
position fixes out of the dive time, find_dive_n_near() function will pair
fix and dive in an ordered way (1st fix -> 1st dive; 2nd fix -> 2nd dive ...)
which is probably erroneous.
The patch changes the logic:
- Search positions for defined dives (instead of dives for defined positions)
without care if position has manually or automatically been set.
- It makes two assumptions:
a.- If the position fix has been taken during the dive time, is correct.
b.- If not during diving time, the correct one is the nearest fix
before the dive begins (the usual case if manually fixed from the
smartphone just before jump into the water). But will work too if there
is only one fix *in range* after the dive (another usual case).
- Finally, as copy_gps_location() in dive.h is used only here, let it take
care of naming the dive if user hasn't named it yet.
Reported-by: Marc Merlin <m...@merlins.org>
Signed-off-by: Salvador Cuñat <salvador.cu...@gmail.com>
---
dive.h | 4 ++
qt-ui/subsurfacewebservices.cpp | 93 ++++++++++++++++-----------------------
2 files changed, 43 insertions(+), 54 deletions(-)
diff --git a/dive.h b/dive.h
index 09a4dda..bb22a3f 100644
--- a/dive.h
+++ b/dive.h
@@ -322,7 +322,11 @@ static inline void copy_gps_location(struct dive *from, struct dive *to)
if (from && to) {
to->latitude.udeg = from->latitude.udeg;
to->longitude.udeg = from->longitude.udeg;
+ if (!to->location) {
+ to->location = strdup(from->location);
+ }
}
+
}
static inline int get_surface_pressure_in_mbar(const struct dive *dive, bool non_null)
diff --git a/qt-ui/subsurfacewebservices.cpp b/qt-ui/subsurfacewebservices.cpp
index 27a56ec..fe1a5cb 100644
--- a/qt-ui/subsurfacewebservices.cpp
+++ b/qt-ui/subsurfacewebservices.cpp
@@ -31,69 +31,54 @@
struct dive_table gps_location_table;
static bool merge_locations_into_dives(void);
-static bool is_automatic_fix(struct dive *gpsfix)
-{
- if (gpsfix && gpsfix->location &&
- (!strcmp(gpsfix->location, "automatic fix") ||
- !strcmp(gpsfix->location, "Auto-created dive")))
- return true;
- return false;
-}
-
#define SAME_GROUP 6 * 3600 // six hours
//TODO: C Code. static functions are not good if we plan to have a test for them.
static bool merge_locations_into_dives(void)
{
- int i, nr = 0, changed = 0;
- struct dive *gpsfix, *last_named_fix = NULL, *dive;
-
- sort_table(&gps_location_table);
-
- for_each_gps_location (i, gpsfix) {
- if (is_automatic_fix(gpsfix) && (dive = find_dive_including(gpsfix->when))) {
- if (dive && !dive_has_gps_location(dive)) {
-#if DEBUG_WEBSERVICE
- struct tm tm;
- utc_mkdate(gpsfix->when, &tm);
- printf("found dive named %s @ %04d-%02d-%02d %02d:%02d:%02d\n",
- gpsfix->location,
- tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
- tm.tm_hour, tm.tm_min, tm.tm_sec);
-#endif
- changed++;
- copy_gps_location(gpsfix, dive);
- }
- } else {
- if (last_named_fix && dive_within_time_range(last_named_fix, gpsfix->when, SAME_GROUP)) {
- nr++;
- } else {
- nr = 1;
- last_named_fix = gpsfix;
- }
- dive = find_dive_n_near(gpsfix->when, nr, SAME_GROUP);
- if (dive) {
- if (!dive_has_gps_location(dive)) {
- copy_gps_location(gpsfix, dive);
- changed++;
- }
- if (!dive->location) {
- dive->location = strdup(gpsfix->location);
- changed++;
+ int i, j;
+ struct dive *gpsfix, *nextgpsfix, *dive;
+
+ sort_table(&dive_table);
+
+ for_each_dive (i, dive) {
+ if (!dive_has_gps_location(dive)) {
+ sort_table(&gps_location_table);
+ for_each_gps_location (j, gpsfix) {
+ if (dive_within_time_range (dive, gpsfix->when, SAME_GROUP)) {
+ /*
+ * If position is fixed during dive. This is the good one.
+ * Asign and end for_each_gps loop
+ */
+ if ((dive->when <= gpsfix->when && gpsfix->when <= dive->when + dive->duration.seconds)) {
+ if (!dive_has_gps_location(dive)) {
+ copy_gps_location(gpsfix,dive);
+ break;
+ }
+ } else {
+ /*
+ * If it is not, check if there are more position fixes.
+ * If none this is the last (or the only one) and so the good one.
+ * If there is at least one but later than end of dive, the actual fix
+ * is OK, asign it and end for_each_gps loop.
+ * If next fix is closer or into the dive, it will be evaluated in next
+ * loop iteration
+ */
+ if (nextgpsfix = get_gps_location(j+1,&gps_location_table)) {
+ if ((dive->when + dive->duration.seconds - gpsfix->when) < (nextgpsfix->when - gpsfix->when)) {
+ copy_gps_location(gpsfix,dive);
+ break;
+ }
+ } else {
+ copy_gps_location(gpsfix,dive);
+ break;
+ }
+ }
}
- } else {
- struct tm tm;
- utc_mkdate(gpsfix->when, &tm);
-#if DEBUG_WEBSERVICE
- printf("didn't find dive matching gps fix named %s @ %04d-%02d-%02d %02d:%02d:%02d\n",
- gpsfix->location,
- tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
- tm.tm_hour, tm.tm_min, tm.tm_sec);
-#endif
}
}
}
- return changed > 0;
}
+
//TODO: C-code.
static void clear_table(struct dive_table *table)
{
--
1.7.10.4
_______________________________________________
subsurface mailing list
subsurface@hohndel.org
http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
