On Sat, Feb 18, 2017 at 3:58 PM, Alessandro Volpi <[email protected]> wrote:
>
> I ran subsurface once again, opening file first_482_2.xml and importing file
> complete_2.xml over it. The patched program DID NOT CRASH as the merge
> operation was performed. THIS IS GOOD NEWS, albeit not unexpected. The
> resulting log is file complete_3.xml .
>
> I compared files complete_2.xml and complete_3.xml :
>
> In my opinion there is still something to be fixed in the merge procedure.
> Files complete_2 and complete_3 should be identical, since I would expect
> the merge operation to ignore the content of existing dives fro #15 to #482
> and simply to add the deleted dives once again.

Right you are.

You found real bugs (although perhaps not all that fatal this time).

> Looking into file diff_2_3 I have observed the following two unexpected
> behaviors:
>
> The records related to dive site description have been DUPLICATED

Yes. I absolutely detest our broken dive site handling. It's stupid
and wrong, and I still think we did so much better before when we
didn't have dive sites at all, just plain location descriptions and
gps.

And it looks like the dive site handling also does XML reading
completely wrong, resulting in just duplicated UUID's, which makes the
whole concept of a uuid entirely pointless.

I tried to fix it up, but I would almost prefer to just rip out that
garbage entirely. Nobody ever wrote the actual code to make the dive
site management _useful_, and it's only been problematic.

Oh well.

> A large number of strings like <divetemperature air='0.0 C' water='20.0 C'/>
> has been replaced with <divetemperature air='0.0 C'/>

Ok, the 0.0C thing is obviously some bug in some importer or
libdivecomputer that *should* have been "no temperature" and turned
into "0.0C" instead.  Oh well. 0C is actually a perfectly valid
temperature (even for water - seawater freezes at about -2°C), so
using it as a "no temperature" thing is stupid but sadly common.

Subsurface itself uses "0 K" as the "no temperature" marker, because
subsurface wasn't written by f*cking morons. Sadly, there's much too
few of us competent people..

Anyway, the fact that the water temperature disappeared entirely due
to the merge is obviously a bug. A simple one to fix too.

> This is now even more important since I HAVE FOUND A BUG in the irda-utils
> package.

Ok, sadly, this I can't do anything about. Practically speaking, IRDA
is dead. Nobody uses it any more, and no developer I know has any
hardware or test-cases. We've found bugs over the years, and
realistically they will not really get fixed.

The Uwatec dive computers are literally the only use of IRDA I know
of, and even there it's dying, with all modern hardware using other
models of communication (ie BT or just waterproof USB connectors).

But I fixed the merging issues you pointed out, and your test-case now
gives me the idential result to "complete.xml" apart from some
cleanups we do (ie whitespace in strings, and removing redundant
cylinder pressures when the data exists in the sample data).

I've created a pull request for Dirk at

    https://github.com/Subsurface-divelog/subsurface/pull/213

but this obviously missed the release that Dirk just did.

Which admittedly is probably just as well, because the changes for
dive site merging definitely need some testing. I did test the code
that uses *both* names/descriptions/notes if you have the same UUID
but different strings (you didn't have that, but you can create XML
files that do), but people should look at it.

Anyway, thanks for testing things that people generally have not tested.

                        Linus
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to