Assemble the actual Suunto serial number

It turns out that the serial number returned by libdivecomputer isn't
really the serial number as interpreted by the vendor. Those tend to be
strings, but libdivecomputer gives us a 32bit number.

Some experimenting showed that for the Suunto devies tested the serial
number is encoded in that 32bit number:

It so happens that the Suunto serial number strings are strings that have
all numbers, but they aren't *one* number. They are four bytes
representing two numbers each, and the "23500027" string is actually the
four bytes 23 50 00 27 (0x17 0x32 0x00 0x1b). And libdivecomputer has
incorrectly parsed those four bytes as one number, not as the encoded
serial number string it is. So the value 389152795 is actually hex
0x1732001b, which is 0x17 0x32 0x00 0x1b, which is - 23 50 00 27.

This should be done by libdivecomputer, but hey, in the meantime this at
least shows the concept. And helps test the XML save/restore code.

It depends on the two patches that create the whole "device.c"
infrastructure, of course. With this, my dive file ends up having the
settings section look like this:

  <divecomputerid model='Suunto Vyper Air' deviceid='d4629110'
serial='01201094' firmware='1.1.22'/>
  <divecomputerid model='Suunto HelO2' deviceid='995dd566'
serial='23500027' firmware='1.0.4'/>

where the format of the firmware version is something I guessed at,
but it was the obvious choice (again, it's byte-based, I'm ignoring
the high byte that is zero for both of my Suuntos).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

5 files changed, 93 insertions, 11 deletions

diff --git a/device.h b/device.h
index 5a207f256..896bcb8e0 100644
--- a/device.h
+++ b/device.h
@@ -5,6 +5,8 @@ struct device_info {
 	const char *model;
 	uint32_t deviceid;
 
+	const char *serial_nr;
+	const char *firmware;
 	const char *nickname;
 	struct device_info *next;
 	gboolean saved;
diff --git a/libdivecomputer.c b/libdivecomputer.c
index 47c7b6367..9529861dc 100644
--- a/libdivecomputer.c
+++ b/libdivecomputer.c
@@ -5,6 +5,7 @@
 #include <glib/gi18n.h>
 
 #include "dive.h"
+#include "device.h"
 #include "divelist.h"
 #include "display.h"
 #include "display-gtk.h"
@@ -418,7 +419,7 @@ static int dive_cb(const unsigned char *data, unsigned int size,
 		dc_parser_destroy(parser);
 		return rc;
 	}
-	dive->dc.model = str_printf("%s %s", devdata->vendor, devdata->product);
+	dive->dc.model = devdata->model;
 	dive->dc.deviceid = devdata->deviceid;
 	dive->dc.diveid = calculate_diveid(fingerprint, fsize);
 
@@ -526,6 +527,35 @@ static uint32_t calculate_sha1(unsigned int model, unsigned int firmware, unsign
 	return csum[0];
 }
 
+static void fixup_suunto_versions(device_data_t *devdata, const dc_event_devinfo_t *devinfo)
+{
+	struct device_info *info;
+
+	info = create_device_info(devdata->model, devdata->deviceid);
+	if (!info)
+		return;
+
+	if (!info->serial_nr && devinfo->serial) {
+		char serial_nr[13];
+
+		snprintf(serial_nr, sizeof(serial_nr), "%02d%02d%02d%02d",
+			(devinfo->serial >> 24) & 0xff,
+			(devinfo->serial >> 16) & 0xff,
+			(devinfo->serial >> 8)  & 0xff,
+			(devinfo->serial >> 0)  & 0xff);
+		info->serial_nr = strdup(serial_nr);
+	}
+
+	if (!info->firmware && devinfo->firmware) {
+		char firmware[13];
+		snprintf(firmware, sizeof(firmware), "%d.%d.%d",
+			(devinfo->firmware >> 16) & 0xff,
+			(devinfo->firmware >> 8)  & 0xff,
+			(devinfo->firmware >> 0)  & 0xff);
+		info->firmware = strdup(firmware);
+	}
+}
+
 static void event_cb(dc_device_t *device, dc_event_type_t event, const void *data, void *userdata)
 {
 	const dc_event_progress_t *progress = data;
@@ -548,6 +578,13 @@ static void event_cb(dc_device_t *device, dc_event_type_t event, const void *dat
 			devinfo->firmware, devinfo->firmware,
 			devinfo->serial, devinfo->serial);
 		devdata->deviceid = calculate_sha1(devinfo->model, devinfo->firmware, devinfo->serial);
+
+		/*
+		 * libdivecomputer doesn't give serial numbers in the proper string form,
+		 * so we have to see if we can do some vendor-specific munging.
+		 */
+		if (!strcmp(devdata->vendor, "Suunto"))
+			fixup_suunto_versions(devdata, devinfo);
 		break;
 	case DC_EVENT_CLOCK:
 			dev_info(devdata, _("Event: systime=%"PRId64", devtime=%u\n"),
@@ -571,6 +608,8 @@ static const char *do_device_import(device_data_t *data)
 	dc_status_t rc;
 	dc_device_t *device = data->device;
 
+	data->model = str_printf("%s %s", data->vendor, data->product);
+
 	// Register the event handler.
 	int events = DC_EVENT_WAITING | DC_EVENT_PROGRESS | DC_EVENT_DEVINFO | DC_EVENT_CLOCK;
 	rc = dc_device_set_events(device, events, event_cb, data);
diff --git a/libdivecomputer.h b/libdivecomputer.h
index 81eb78abd..0950d32ae 100644
--- a/libdivecomputer.h
+++ b/libdivecomputer.h
@@ -13,6 +13,7 @@
 typedef struct device_data_t {
 	dc_descriptor_t *descriptor;
 	const char *vendor, *product, *devname;
+	const char *model;
 	unsigned int deviceid, diveid;
 	dc_device_t *device;
 	dc_context_t *context;
diff --git a/parse-xml.c b/parse-xml.c
index e7cb1a182..6de518d02 100644
--- a/parse-xml.c
+++ b/parse-xml.c
@@ -14,6 +14,7 @@
 #include <glib/gi18n.h>
 
 #include "dive.h"
+#include "device.h"
 
 int verbose;
 
@@ -148,7 +149,7 @@ static struct {
 struct {
 	const char *model;
 	uint32_t deviceid;
-	const char *nickname;
+	const char *nickname, *serial_nr, *firmware;
 } dc;
 } cur_settings;
 static gboolean in_settings = FALSE;
@@ -619,6 +620,10 @@ static void try_to_fill_dc_settings(const char *name, char *buf)
 		return;
 	if (MATCH("divecomputerid.nickname", utf8_string, &cur_settings.dc.nickname))
 		return;
+	if (MATCH("divecomputerid.serial", utf8_string, &cur_settings.dc.serial_nr))
+		return;
+	if (MATCH("divecomputerid.firmware", utf8_string, &cur_settings.dc.firmware))
+		return;
 
 	nonmatch("divecomputerid", name, buf);
 }
@@ -1061,8 +1066,12 @@ static void reset_dc_settings(void)
 {
 	free((void *)cur_settings.dc.model);
 	free((void *)cur_settings.dc.nickname);
+	free((void *)cur_settings.dc.serial_nr);
+	free((void *)cur_settings.dc.firmware);
 	cur_settings.dc.model = NULL;
 	cur_settings.dc.nickname = NULL;
+	cur_settings.dc.serial_nr = NULL;
+	cur_settings.dc.firmware = NULL;
 	cur_settings.dc.deviceid = 0;
 }
 
@@ -1083,8 +1092,20 @@ static void dc_settings_start(void)
 
 static void dc_settings_end(void)
 {
+	struct device_info *info;
+
 	if (cur_settings.dc.model)
 		remember_dc(cur_settings.dc.model, cur_settings.dc.deviceid, cur_settings.dc.nickname, TRUE);
+
+	info = create_device_info(cur_settings.dc.model, cur_settings.dc.deviceid);
+	if (info) {
+		if (!info->serial_nr && cur_settings.dc.serial_nr)
+			info->serial_nr = strdup(cur_settings.dc.serial_nr);
+		if (!info->firmware && cur_settings.dc.firmware)
+			info->firmware = strdup(cur_settings.dc.firmware);
+		if (!info->nickname && cur_settings.dc.nickname)
+			info->nickname = strdup(cur_settings.dc.nickname);
+	}
 	reset_dc_settings();
 }
 
diff --git a/save-xml.c b/save-xml.c
index 465010cd1..0fa0aee2a 100644
--- a/save-xml.c
+++ b/save-xml.c
@@ -485,23 +485,42 @@ static void save_trip(FILE *f, dive_trip_t *trip)
 
 static void save_dc_if_needed(FILE *f, struct divecomputer *dc)
 {
-	const char *nickname;
+	struct device_info *info = get_device_info(dc->model, dc->deviceid);
+	const char *nickname, *serial_nr, *firmware;
 
 	/* we have no dc or no model or no deviceid information... nothing to do here */
-	if (!dc || !dc->model || !*dc->model || !dc->deviceid)
+	if (!info || info->saved)
 		return;
+	info->saved = 1;
 
-	if (dc_was_saved(dc))
-		return;
+	/* Nicknames that are empty or the same as the device model are not interesting */
+	nickname = info->nickname;
+	if (nickname) {
+		if (!*nickname || !strcmp(dc->model, nickname))
+			nickname = NULL;
+	}
+
+	/* Serial numbers that are empty are not interesting */
+	serial_nr = info->serial_nr;
+	if (serial_nr && !*serial_nr)
+		serial_nr = NULL;
+
+	/* Firmware strings that are empty are not interesting */
+	firmware = info->firmware;
+	if (firmware && !*firmware)
+		firmware = NULL;
 
-	mark_dc_saved(dc);
-	nickname = get_dc_nickname(dc->model, dc->deviceid);
-	/* We have no nickname, or it is the same as the model ID - nothing interesting */
-	if (!nickname || !*nickname || !strcmp(dc->model, nickname))
+	/* Do we have anything interesting about this dive computer to save? */
+	if (!serial_nr && !nickname && !firmware)
 		return;
 
 	fprintf(f, "<divecomputerid model='%s' deviceid='%08x'", dc->model, dc->deviceid);
-	show_utf8(f, nickname, " nickname='", "'", 1);
+	if (serial_nr)
+		show_utf8(f, serial_nr, " serial='", "'", 1);
+	if (firmware)
+		show_utf8(f, firmware, " firmware='", "'", 1);
+	if (nickname)
+		show_utf8(f, nickname, " nickname='", "'", 1);
 	fprintf(f, "/>\n");
 	return;
 }