[PATCH] Preferentially use existing device ID when setting serial number

[Linus Torvalds]

From: Linus Torvalds <torvalds at linux-foundation.org>
Date: Mon, 20 Jun 2016 17:59:26 -0700
Subject: [PATCH] Preferentially use existing device ID when setting serial number

We have two different models for setting the deviceid associated with a
dive computer: either take the value from the libdivecomputer 'devinfo'
field (from the DC_EVENT_DEVINFO event), or generate the device ID by
just hashing the serial number string.

The one thing we do *not* want to have, is to use both methods, so that
the same device generates different device IDs.  Because then we'll
think we have two different dive computers even though they are one and
the same.

Usually, this is not an issue, because libdivecomputer either sends the
DEVINFO event or gives us the serial number string, and we'll always
just pick one or the other.

However, in the case of at least the Suunto EON Steel, I intentionally
did *not* send the DC_EVENT_DEVINFO event, because it gives no useful
information.  We used the serial number string to generate a device ID,
and everything was fine.

However, in commit d40cdb4755ee ("Add the devinfo event") in the
libdivecomputer tree, Jeff started generating those DC_EVENT_DEVINFO
events for the EON Steel too, and suddenly subsurface would start using
a device ID based on that instead.

The situation is inherently ambiguous - for the EON Steel, we want to
use the hash of the serial number (because that is what we've
historically done), but other dive computers might want to use the
DEVINFO data (because that is what _those_ backends have historically
done, even if they might also implement the new serial string model).

This commit makes subsurface resolve this ambiguity by simply preferring
whatever previous device ID it has associated with that particular
serial number string.  If you have no previous device IDs, it doesn't
matter which one you pick.

Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
---

Ugh. My first approach didn't work at all, because while I *intended* to 
instead just deprecate the use of "deviceid" as the primary index in favor 
of using the serial number string when available, that foundered on the 
basic problem that we don't actually restore the serial number in the 
"struct divecomputer" structure (we keep track of them in a separate 
table, based on the deviceid).

So what I thought was going to be a simple fix ended up requiring a 
completely different approach, after wasting way too much time on trying 
to make serial numbers more reliably available.

But this patch works for me, and is fairly straigthforward, even if it 
wasn't my first preferred approach.

With this, downloading from the EON Steel works fine with current 
libdivecomputer too.

Of course, having wasted all that time on this issue, I haven't actually 
looked at the CCR issues at all. I'll hopefully get around to that 
tomorrow.

 core/libdivecomputer.c | 46 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/core/libdivecomputer.c b/core/libdivecomputer.c
index e7f88a517b5b..cfe6a8049550 100644
--- a/core/libdivecomputer.c
+++ b/core/libdivecomputer.c
@@ -452,6 +452,39 @@ static uint32_t calculate_string_hash(const char *str)
 	return calculate_diveid((const unsigned char *)str, strlen(str));
 }
 
+/*
+ * Find an existing device ID for this device model and serial number
+ */
+static void dc_match_serial(void *_dc, const char *model, uint32_t deviceid, const char *nickname, const char *serial, const char *firmware)
+{
+	struct divecomputer *dc = _dc;
+
+	if (!deviceid)
+		return;
+	if (!model || strcasecmp(dc->model, model))
+		return;
+	if (!serial || strcasecmp(dc->serial, serial))
+		return;
+	dc->deviceid = deviceid;
+}
+
+/*
+ * Set the serial number.
+ *
+ * This also sets the device ID by looking for existing devices that
+ * have that serial number.
+ *
+ * If no existing device ID exists, create a new by hashing the serial
+ * number string.
+ */
+static void set_dc_serial(struct divecomputer *dc, const char *serial)
+{
+	dc->serial = serial;
+	call_for_each_dc(dc, dc_match_serial, false);
+	if (!dc->deviceid)
+		dc->deviceid = calculate_string_hash(serial);
+}
+
 static void parse_string_field(struct dive *dive, dc_field_string_t *str)
 {
 	// Our dive ID is the string hash of the "Dive ID" string
@@ -462,11 +495,7 @@ static void parse_string_field(struct dive *dive, dc_field_string_t *str)
 	}
 	add_extra_data(&dive->dc, str->desc, str->value);
 	if (!strcmp(str->desc, "Serial")) {
-		dive->dc.serial = strdup(str->value);
-		/* should we just overwrite this whenever we have the "Serial" field?
-		 * It's a much better deviceid then what we have so far... for now I'm leaving it as is */
-		if (!dive->dc.deviceid)
-			dive->dc.deviceid = calculate_string_hash(str->value);
+		set_dc_serial(&dive->dc, str->value);
 		return;
 	}
 	if (!strcmp(str->desc, "FW Version")) {
@@ -660,6 +689,10 @@ static int dive_cb(const unsigned char *data, unsigned int size,
 	import_dive_number++;
 	dive = alloc_dive();
 
+	// Fill in basic fields
+	dive->dc.model = strdup(devdata->model);
+	dive->dc.diveid = calculate_diveid(fingerprint, fsize);
+
 	// Parse the dive's header data
 	rc = libdc_header_parser (parser, devdata, dive);
 	if (rc != DC_STATUS_SUCCESS) {
@@ -667,9 +700,6 @@ static int dive_cb(const unsigned char *data, unsigned int size,
 		goto error_exit;
 	}
 
-	dive->dc.model = strdup(devdata->model);
-	dive->dc.diveid = calculate_diveid(fingerprint, fsize);
-
 	// Initialize the sample data.
 	rc = parse_samples(devdata, &dive->dc, parser);
 	if (rc != DC_STATUS_SUCCESS) {
-- 
2.9.0.rc1.20.g6326f19