[PATCH] Import Datatrak/WLog files

Dirk Hohndel dirk at hohndel.org
Sun Oct 26 20:16:12 PDT 2014


Hi Salvador,

thanks for pointing out the mailing list issue. Since you had me copied
directly on this email, I was even less concerned and checked only
briefly.

Oops again.

On Sun, Oct 26, 2014 at 09:23:19PM +0100, Salvador Cuñat wrote:
> Sequentially parses a file, expected to be a Datatrak/WLog divelog, and
> converts the dive info into Subsurface's dive structure.
> 
> As my first DC, back in 90s, was an Aladin Air X, the obvious choice of log
> software was DTrak (Win version). After using it for some time we moved to WLog
> (shareware software more user friendly than Dtrak, printing capable, and still
> better, it runs under wine, which, as linux user, was definitive for me). Then,
> some years later, my last Aladin died and I moved to an OSTC, forcing me to
> look for a software that support this DC.
> I found JDivelog which was capable of import Dtrak logs and used it for some
> time until discovered Subsurface existence and devoted to it.
> 
> The fact was that importing Dtrak dives in JDivelog and then re-importing them
> in Subsurface caused a significant data loss (mainly in the profile events and
> alarms) and weird location of some other info in the dive notes (mostly tag
> items in the original Dtrak software). This situation can't actually be solved
> with tools like divelogs.de which causes similar if no greater data loss.
> 
> Although this won't be a core feature for Subsurface, I expect it can be useful
> for some other divers as has been for me.

I think this is useful. So I'll be willing to add it.

> Comments an issues:
> 
> Datatrak/Wlog files include a lot of diving data which are not directly
> supported in Subsurface, in these cases we choose mostly to use "tags".

Why is this (and the following comments) in "we" form? Did you collaborate
with someone on these patches?

> The lack of some important info in Datatrak archives (e.g. tank's initial
> pressure) forces us to do some arbitrary assumptions (e.g. initial pressure =
> 200 bar).
> 
> There might be archives coming directly from old DOS days, as first versions
> of Datatrak run on that OS; they were coded CP437 or CP850, while dive logs
> coming from Win versions seems to be coded CP1252. Finally, Wlog seems to use a
> mixed confusing style. Program directly converts some of the old encoded chars
> to iso8859 but is expected there be some issues with non alphabetic chars, e.g.
> "ª".

Code Pages. Don't you just love it. Not.

> There are two text fields: "Other activities" and "Dive notes", both limited to
> 256 char size. We have merged them in Subsurface's "Dive Notes" although the
> first one could be "tagged", but we're unsure that the user had filled it in
> a tag friendly way.
> 
> WLog adds some information to the dive and lets the user to write more than
> 256 chars notes. This is achieved, while keeping compatibility with DTrak
> divelogs, by adding a complementary file named equally as the .log file and
> with .add extension where all this info is stored.  We have, still, not worked
> with this complementary files.
> 
> This work is based on the paper referenced in butracker #194 which has some
> errors (e.g. beginning of log and beginning of dive are changed) and a lot of
> bytes of unknown meaning. Example.log shows, at least, one more byte than those
> referred in the paper for the O2 Aladin computer, this could be a byte referred
> to the use of SCR but the lack of an OC dive with O2 computer makes impossible
> for us to compare.

I think it will be hard to get the import perfectly right.

> The only way we have figured out to distinguish a priori between SCR and non
> SCR dives with O2 computers is that the dives are tagged with a "rebreather"
> tag. Obviously this is not a very trusty way of doing things. In SCR dives,
> the O2% in mix means, probably, the maximum O2% in the circuit, not the O2%
> of the EAN mix in the tanks, which would be unknown in this case.
> 
> The list of DCs related in bug #194 paper seems incomplete, we have added
> one or two from WLog and discarded those which are known to exist but whose
> model is unknown, grouping them under the imaginative name of "unknown". The
> list can easily be increased in the future if we ever know the models
> identifiers.
> BTW, in Example.log, 0x00 identifier is used for some DC dives and from my own
> divelogs is inferred that 0x00 is used for manually entered dives, this could
> easily be an error in Example.log coming from a preproduction DC model.

Have you found others who have old divelogs from this software?

> diff --git a/datatrak.c b/datatrak.c
> new file mode 100644
> index 0000000..3c11659
> --- /dev/null
> +++ b/datatrak.c
> @@ -0,0 +1,724 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <time.h>
> +
> +#include "datatrak.h"
> +#include "dive.h"
> +#include "units.h"
> +#include "device.h"
> +#include "gettext.h"
> +#include "time.h"
> +
> +unsigned char lector_bytes[2], lector_word[4], tmp_1byte, *byte;
> +int tmp_2bytes, is_nitrox, is_O2, is_SCR;
> +long tmp_4bytes;
> +
> +
> +static int two_bytes_to_int(unsigned char x, unsigned char y) {
> +
> +	return (x << 8) + y;
> +}

Take a look at the coding style. The opening '{' on a function should be
in column 1 of the folowing line. This seems to be wrong everywhere.

Also, this function will get you in trouble on systems where int is only
16 bits (admittedly, not a major concern these days, but on principle...
why is the result signed?)

> +static long four_bytes_to_long(unsigned char x, unsigned char y, unsigned char z, unsigned char t) {
> +
> +	return (((long)x << 24) + ((long)y << 16) + ((long)z << 8) + ((long)t));
> +}

Same comments again. If long is 32 bit you are in trouble. Can this be
unsigned, please?

> +static unsigned char* byte_to_bits(unsigned char byte) {
> +
> +	unsigned char i, *bits = (unsigned char*) malloc(8 * sizeof(unsigned char));

sizeof(unsigned char) is always 1. Well, technically that's only
guaranteed in C++, I guess. But I'm not aware of any C compiler today
(certainly not on Windows, Mac, Linux) that does anything else.

> +
> +	for (i = 0; i < 8; i++) {
> +		bits[i] = (byte & (unsigned char) pow(2,i))/pow(2,i);

Yikes... this can be done much easier with shifts...

> +	}
> +	return bits;
> +}
> +
> +static time_t date_time_to_ssrfc(long date, int time) {
> +
> +	time_t tmp;
> +	tmp = ((date -135140)*86400)+(time*60);
> +	return tmp;
> +}

Fascinating formula - seriously, this is in days since Jan 1st, 1600?
This really deserves a comment, don't you think?

> +static unsigned char to_8859(unsigned char char_cp850) {
> +	unsigned char outchar;
> +	unsigned char char_8859[46] = {0xc7, 0xfc, 0xe9, 0xe2, 0xe4, 0xe0, 0xe5, 0xe7,
> +				      0xea, 0xeb, 0xe8, 0xef, 0xee, 0xec, 0xc4, 0xc5,
> +				      0xc9, 0xe6, 0xc6, 0xf4, 0xf6, 0xf2, 0xfb, 0xf9,
> +				      0xff, 0xd6, 0xdc, 0xf8, 0xa3, 0xd8, 0xd7, 0x66,
> +				      0xe1, 0xed, 0xf3, 0xfa, 0xf1, 0xd1, 0xaa, 0xba,
> +				      0xbf, 0xae, 0xac, 0xbd, 0xbc, 0xa1};
> +	outchar = char_8859[char_cp850 - 0x80];
> +	return (outchar);
> +}
> +
> +static char *to_utf8(unsigned char *in_string) {
> +	int outlen, inlen, i=0, j=0;
> +	inlen = strlen(in_string);
> +	outlen = (inlen * 2)+1;
> +
> +	char *out_string = calloc(outlen, sizeof(char));
> +	for (i = 0; i < inlen; i++) {
> +		if (in_string[i] < 127) {
> +			out_string[j] = in_string[i];
> +		} else {
> +			if (in_string[i] > 127 && in_string[i] <= 173) {
> +				in_string[i] = to_8859(in_string[i]);
> +			}
> +			out_string[j] = (in_string[i] >> 6) | 0xC0;
> +			j++;
> +			out_string[j] = (in_string[i] & 0x3F) | 0x80;
> +		}
> +		j++;
> +	}
> +	out_string[j+1] = '\0';
> +	return(out_string);
> +}

This also deserves some comments
> +
> +/*
> + * Shameless copy paste from dive.c add_sample() with ligth changes

ligth?

> + */
> +static struct sample *add_dt_sample(int time, struct divecomputer *dc)
> +{
> +	struct sample *p = prepare_sample(dc);
> +
> +	if (p) {
> +		p->time.seconds = time;
> +		finish_sample(dc);
> +	}
> +	return p;
> +}
[...]
> +	/*
> +	 * Found dive header 0xA000, verify second byte
> +	 */
> +	fread(&lector_bytes[n+1], sizeof(char), 1, archivo);
> +	if (two_bytes_to_int (lector_bytes[0],lector_bytes[1]) != 0xA000) {

more whitespace issues.....^^^

> +		printf("ERROR, Byte = %4x\n", two_bytes_to_int (lector_bytes[0],lector_bytes[1]));

and more of the same.........................................^^^^

> +	/*
> +	 * Altitude. Don't exist in Subsurface, the equivalent would be
> +	 * surface air pressure which can, be calculated from altitude.
> +	 * As dtrak registers altitude intervals, we, arbitrarily, choose
> +	 * the lower altitude/pressure equivalence for each segment.
> +	 */

Yes, we do this for some divecomputer that have similar concepts.

> +	switch (tmp_1byte) {
> +		case 1:
> +			dt_dive->dc.surface_pressure.mbar = 1013;
> +			break;
> +		case 2:
> +			dt_dive->dc.surface_pressure.mbar = 928;
> +			break;
> +		case 3:
> +			dt_dive->dc.surface_pressure.mbar = 806;
> +			break;
> +		case 4:
> +			dt_dive->dc.surface_pressure.mbar = 684;
> +			break;

It would be nice to have the altitude ranges in comments.
735m, 1888m, 3194m seem very odd thresholds. Or maybe you used a different
conversion formula?

> +			dt_dive->suit = strdup(QT_TRANSLATE_NOOP("gettextFromC","No suit"));

Again, whitespace... you want a space between comma and double quote..........^^^

> +	if (tmp_2bytes != 0x7fff) {
> +		dt_dive->watertemp.mkelvin = dt_dive->dc.watertemp.mkelvin = C_to_mkelvin((double)(tmp_2bytes/100));
> +	} else {
> +		dt_dive->watertemp.mkelvin = 0;
> +	}

You don't really need the curly braces here...

Not a lot of issues, but enough for me to ask you to resubmit.

/D


More information about the subsurface mailing list