[PATCH] Change in logic while aplying gps fixes to dives
Salvador Cuñat
salvador.cunat at gmail.com
Wed Jul 16 06:27:05 PDT 2014
Hi Dirk,
2014-07-15 12:55 GMT+02:00 Dirk Hohndel <dirk at hohndel.org>:
> Yeah, I wish it wasn't configurable... but maybe your solution will make
> that a moot point...
>
Hmmm, in user-manual.txt, I even recomended changing it. ;-)
> BTW actual implementation is unable to pair the same gps fix with more
> than
> > one dive (which is most likely to happen).
>
> Can you phrase that differently? I'm not sure I understand.
>
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.
>
>
> > a.- If the position fix has been taken during the dive time, is
> correct.
>
> But it shouldn't replace an existing position in the dive
>
It doesn't, if a dive has a position fix, it is not considered.
>
> > b.- If not during diving time, the correct one is the nearest fix
> > before the dive begins (also 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).
>
> 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.
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.
> > - 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.
>
> that will give "automated" names to people - but hey, I always set the
> names first and then import...
>
> No change here, works just as it did before. If the dive has a position
but it's unnamed, assigns the name of position fix. That's why I recomended
to change the settings of background service in companion app. This said, I
always name before import too :-)
> Whitespace... no reason for a new empty line here (I bet you had debugging
> output there :-) )
>
Grrrrrrrr. Cleaned.
> + sort_table(&dive_table);
>
> Umm, why? The table is always sorted. And at this point you haven't made
> any changes, right?
>
Grrrrr, grrrrrr. This had to be sort_table(&gps_location_table);
>
> > + sort_table(&gps_location_table);
>
> Oh no - now you are resorting the gps_location_table for each dive. I have
>
Grrrrrr, grrrrrrr, grrrrrrr. And this just didn't have to be there.
>
> you only go here if the dive has no location, right? See *** above
> so this check is redundant
>
Certainly, cleaned.
>
> > + * 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))
> {
>
> I don't understand this condition.
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 ;-)
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).
> > +
> copy_gps_location(gpsfix,dive);
> > + break;
> > + }
> > + } else {
>
> See above. This needs a time cutoff. Max 30 minutes before or after a
dive.
>
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.
> Did I miss something? Did this turn from a bool function to a void
> function?
>
Hmmm, a boolean function that is never true is quite a strange thing.
Solved.
>
> Overall, I think you have a point with your algorithm. Can you work on the
> feedback and resubmit? I will definitely consider it.
>
>
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.
Regards
Salva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140716/7673267a/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Change-in-logic-while-aplying-gps-fixes-to-dives.patch
Type: text/x-patch
Size: 6603 bytes
Desc: not available
URL: <http://lists.hohndel.org/pipermail/subsurface/attachments/20140716/7673267a/attachment-0001.bin>
More information about the subsurface
mailing list