On 2014-01-27 22:17, Dirk Hohndel wrote:
On Mon, 2014-01-27 at 20:26 +0100, Jef Driesen wrote:
On 27-01-14 19:10, Dirk Hohndel wrote:
> On Mon, 2014-01-27 at 10:53 +0100, Jef Driesen wrote:
>> I think the real problem is something else. Jun Song's dives fail to
>> parse. Not sure why yet. But that's just one part of the problem. When I
>> try download the memory dump into subsurface, I can confirm no dives are
>> imported. No error message is shown. So it seems subsurface is silently
>> dropping dives that are failing to parse. That's the second part of the
>> problem.
>>
>> I quickly checked the subsurface code. The dive_cb function exits
>> immediately in case of a parsing error, and as a result the dive never
>> gets added anywhere.
>
> Which seems reasonable. If we can't parse it, we clearly can't add it.
Depends. If there is one small parsing error that does not necessary
mean you
didn't get any useful info? It's not always black and white. I also
realize this
is tricky.
This is one of the reasons why I recommend keeping the raw data
around. Because
then you can re-parse the dive again when the bug is fixed, without
having to
download again. If it's a nasty bug that takes a while to fix (or you
dive very
often) then by the time the bug is fixed, those dives may already have
been
pushed out of the dive computer's memory. If you still have the raw
data, that's
not a problem.
A great point in theory. The amount of effort to keep the data
around...
I don't know. Doesn't seem worth it.
In most cases, parsing will of course succeed and there is no need to
re-parse again. But in case you happen to step on some parsing bug, then
you'll probably appreciate this feature ;-)
>> But the error is also never reported back to the
>> user. There is a call to the dev_info function to show an error message
>> in the progress bar, but I assume that because the download dialog is
>> closed almost immediately, the user doesn't have a chance to notice this
>> error message.
>
> I'll fix that.
I think this will already be a great improvement. Silent data loss is
nasty and
confusing.
Try the latest master. Not beautiful, but effective, I think.
I didn't had a chance to test it thoroughly, but it's definitely an
improvement. What happens if only a few dives fail to parse? Are the
good ones still imported? Or is nothing imported?
That brings me to my next idea. Have you ever considered showing some
dialog with an overview of the downloaded dives, before they get added
to the logbook? Then the user can review the dives, before actually
importing them. This allows for example to ignore dives, fix incorrect
date/times (e.g. due traveling to another timezone, dst adjustments,
battery change), etc. You can also indicate which dives are failing to
parse. Right now, all dives go straight into the logbook, and it may be
difficult to notice which dives failed. Also if dives get merged
automatically that may confuse users. But that's something you could
also show in this dialog.
Jef
_______________________________________________
subsurface mailing list
[email protected]
http://lists.hohndel.org/cgi-bin/mailman/listinfo/subsurface
