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
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
