[PATCH] Change in logic while aplying gps fixes to dives

[Dirk Hohndel]

On Wed, Jul 16, 2014 at 03:27:05PM +0200, Salvador Cuñat wrote:
> 
> I mean, a user may make 2 or 3 repetitive dives with the boat anchored (if
> the dive point is well worth, of course), the three dives would share the
> position fix. In old implementation this was imposible to happen (unless
> running three times the download/apply, so the user could find that a dive
> that wasn't paired in the first run was paired suddenly a week later). Now
> the three dives would have the same position fix.

That assumes that the user doesn't have time based logging AND that the
distance based logging is set to a fairly broad range... in general I have
never had more than 90 minutes without another fix - and that was on a
bigger boat at anchor. Little dive boats, even at anchor, tend to drift
more in my experience - but maybe that's different for others (or they
have set up different margins).

> > This does have a relatively short "range", though. Taking a GPS fix from
> > an hour ago (or yesterday) makes no sense.
> 
> I don't agree. Manual positions will probably get fixed just before or just
> after the dive (unlikely during the dive) but automatic fixes are easily 1,
> 2 or 3 hours before the dive. We're just discarding those out of the range
> of SAME_GROUP (6 hours) and evaluating the rest, it would possibly be
> better a position fix 2 hours before the dive than none.

OK, my experience is different, but I understand your point.

> There is a corner case here:  let's say the user forgets to run background
> service before diving and runs it after quitting the water, as he will get
> more than one fix, if time distance between end of dive and first fix is
> longer that distance between 1st fix and 2nd one, the function will fail.
> I'll try to fix this in next patches.

OK

> You didn't . I'll explain
> 
>         PF1   PF2    PF3                              PF4
>                           PF5             PF6
>          |        |        |                                  |
>                     |                 |
> |---------------------------------------------------------------------------------------------------------------|
> 
> |                                              |
> 
> DBegin                                    DEnd
> 
> 
> Condition is true if distance between PF and DEnd is shorter than distance
> between PF and the next PF, so in above example it could only be true for
> PF4 but  this condition wouldn't be reached as PF4 is inside the dive.  If
> there were no PF4, then would be true for PF3-PF5 and PF3 would be the
> chosen position fix.
> As I'm writing on google webmail the wonderful and amazing graph may look
> awful and horrific in text readers ;-)

The graph looks fine, but I'm still confused by the logic :-)

> Oh, and what if the distance (in time) is two years? Should you still take
> > the fix? Hint: NO!!!
> >
> 
> First fix more than 6 hours in the past should have been rejected by
> dive_within_time_range() some lines above, and so will be the 2nd two years
> in the future in its iteration. The dive wouldn't get its position fix
> because there is no good position. The only improvement here would be to
> cut the loop if gpsfix is more than SAME_GROUP (6 hours) into the future to
> avoid spending cpu cycles (done).

6 hours seems long, but see above.

> Not sure, as commented above.  May be 6 hours is too much time but 30
> minutes would be too little in many situations. Anyway we would only have
> to redefine  SAME_GROUP constant to the desired value.

Let's leave it at 6h for now.

> The attached patch solves most of your considerations, please take a look
> on it.
> BTW, as I didn't expect your mail and was changing the
> for_each_gps_position loop, I had modified it including a counter to keep
> track of position in gps table and avoid iterating from 0 each time. This
> reduces heavily the number of iterations (aprox 80%, in the test file I'm
> using, refered to previous patch, 95% refered to actual situation) without
> any efect that I had seen on the applying result.
> 
> My test .xml file covers a bunch of situations (named dives, unamed dives,
> position fixes *in* dive time and *out* dive time, single position fixes
> for a dive, multiple position fixes for a dive, repetitive dives with 5
> hours of position fixes, manually created weird fixes in their time
> relation with test dives, etc, and most of them are real diving
> situations).  That said, would be useful to test the patch against real
> divelogs in situations where old implementation worked well, and those
> situations known to have failed previously, trying to find corner cases I
> haven't foreseen.

Should we add your test file to the sources? And maybe even hand crafted
sample data to the web service (I can easily insert that into the
database) so we can reproduce these tests?


Patch looks good

/D