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@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

1 files changed, 38 insertions, 8 deletions

diff --git a/core/libdivecomputer.c b/core/libdivecomputer.c
index e7f88a517..cfe6a8049 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) {