From d918289a0456230788a4135721eaf995b41c9950 Mon Sep 17 00:00:00 2001 From: Linus Torvalds 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 Signed-off-by: Dirk Hohndel --- core/libdivecomputer.c | 46 ++++++++++++++++++++++++++++++++++-------- 1 file 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) {