diff --git a/.gitignore b/.gitignore index bce5b9b9..908a96e0 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ Makefile.in /aclocal.m4 /ar-lib +/test-driver /autom4te.cache/ /config.guess /config.h diff --git a/Makefile.am b/Makefile.am index bc07dc91..88688d98 100644 --- a/Makefile.am +++ b/Makefile.am @@ -8,6 +8,8 @@ if ENABLE_DOC SUBDIRS += doc endif +SUBDIRS += test + AM_MAKEFLAGS = -s ACLOCAL_AMFLAGS = -I m4 @@ -25,4 +27,5 @@ EXTRA_DIST = \ test/fixtures/manifest.txt \ test/fixtures/shearwater_petrel2-0001.bin \ test/fixtures/hw_ostc5-0001.bin \ - test/fixtures/garmin_descent_mk1-0001.bin + test/fixtures/garmin_descent_mk1-0001.bin \ + test/test_descriptor_product_name.c diff --git a/configure.ac b/configure.ac index 38d68dd4..4ba6139c 100644 --- a/configure.ac +++ b/configure.ac @@ -264,6 +264,7 @@ AC_CONFIG_FILES([ doc/doxygen.cfg doc/man/Makefile examples/Makefile + test/Makefile ]) AC_OUTPUT AC_MSG_NOTICE([ diff --git a/include/libdivecomputer/descriptor.h b/include/libdivecomputer/descriptor.h index 62b36738..41486c0d 100644 --- a/include/libdivecomputer/descriptor.h +++ b/include/libdivecomputer/descriptor.h @@ -144,6 +144,31 @@ dc_descriptor_filter (const dc_descriptor_t *descriptor, dc_transport_t transpor dc_descriptor_t * dc_descriptor_find_by_hw_id (dc_family_t family, unsigned int hw_id); +/** + * Find a descriptor by raw product-name string from the device version packet. + * + * Some device families (notably Mares Icon HD) share a coarse numeric model + * across multiple marketed products but expose a distinguishing product-name + * string in their version packet (DC_EVENT_DEVINFO.product_name). This + * function maps that raw string to the best-matching descriptor, while the + * coarse model number remains authoritative for parser layout selection. + * + * The mapping is evidence-based and limited to confirmed product-name strings + * from the device firmware. Returns NULL for an empty, NULL, or unrecognised + * product-name string; callers must fall back to the coarse model descriptor. + * + * The returned descriptor is a direct reference to an internal table entry + * and must be released with dc_descriptor_free(), which is safe to call on + * such a reference. + * + * @param[in] family The device family type. + * @param[in] model The coarse model number from DC_EVENT_DEVINFO. + * @param[in] product_name The raw product-name string from DC_EVENT_DEVINFO. + * @returns A descriptor on success, or NULL if no match. + */ +dc_descriptor_t * +dc_descriptor_find_by_product_name (dc_family_t family, unsigned int model, const char *product_name); + #ifdef __cplusplus } #endif /* __cplusplus */ diff --git a/include/libdivecomputer/device.h b/include/libdivecomputer/device.h index 9ffce79e..f6b7d8ef 100644 --- a/include/libdivecomputer/device.h +++ b/include/libdivecomputer/device.h @@ -48,6 +48,10 @@ typedef struct dc_event_progress_t { unsigned int maximum; } dc_event_progress_t; +/* Maximum length of the product_name field in dc_event_devinfo_t, + * including the NUL terminator. */ +#define DC_DEVINFO_PRODUCT_NAME_SIZE 17 + typedef struct dc_event_devinfo_t { unsigned int model; unsigned int firmware; @@ -59,6 +63,12 @@ typedef struct dc_event_devinfo_t { * consumer-side sub-model differentiation. Consumers that do not use * this field are unaffected; the model field remains authoritative. */ unsigned int hw_id; + /* Raw product-name string from the device's version packet, NUL-terminated. + * Empty string when not available or not applicable for this device family. + * Populated only on live download; use dc_descriptor_find_by_product_name() + * to map this to a refined descriptor. The model field remains the + * authoritative parser-layout selector. */ + char product_name[DC_DEVINFO_PRODUCT_NAME_SIZE]; } dc_event_devinfo_t; typedef struct dc_event_clock_t { diff --git a/src/descriptor.c b/src/descriptor.c index 965405dd..783483db 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -1126,6 +1126,68 @@ dc_descriptor_find_by_hw_id (dc_family_t family, unsigned int hw_id) return NULL; } +/* Product-name to descriptor mapping for Mares Icon HD. + * + * The mares_iconhd driver reads a product-name string from offset 0x46 of + * the version packet and maps it to a coarse numeric model. Several marketed + * products share one numeric model; this table maps the confirmed firmware + * product-name strings to the specific descriptor product name, allowing + * consumers to refine the display label beyond the coarse model. + * + * Evidence: mares_iconhd.c mares_iconhd_get_model() table. Only strings + * that unambiguously identify a single marketed product are listed. */ +typedef struct { + dc_family_t family; + unsigned int model; + const char *firmware_name; + const char *descriptor_product; +} dc_product_name_entry_t; + +static const dc_product_name_entry_t g_product_name_map[] = { + /* PUCKPRO (0x18): "Puck Pro" firmware name — both "Puck Pro" and "Puck Pro +" + * share model 0x18. The version-packet name "Puck Pro" identifies the base + * variant. No confirmed firmware name for the + variant is known; it falls + * back to the coarse model ("Puck Pro"). */ + {DC_FAMILY_MARES_ICONHD, 0x18, "Puck Pro", "Puck Pro"}, + /* PUCK4 (0x35): four marketed products share this model. The firmware + * product-name strings below are taken directly from the mares_iconhd + * matching table and map to specific descriptor product names. */ + {DC_FAMILY_MARES_ICONHD, 0x35, "Puck4", "Puck 4"}, + {DC_FAMILY_MARES_ICONHD, 0x35, "Puck Lite", "Puck Lite"}, + {DC_FAMILY_MARES_ICONHD, 0x35, "Puck Pro U", "Puck Pro Ultra"}, + /* "Puck" (0x35) is ambiguous — the BLE advertisement prefix "Puck" matches + * multiple variants; no single descriptor product is confirmed. Not mapped. */ +}; + +dc_descriptor_t * +dc_descriptor_find_by_product_name (dc_family_t family, unsigned int model, const char *product_name) +{ + if (product_name == NULL || product_name[0] == '\0') + return NULL; + + for (size_t i = 0; i < C_ARRAY_SIZE(g_product_name_map); i++) { + if (g_product_name_map[i].family != family || + g_product_name_map[i].model != model || + strcmp(g_product_name_map[i].firmware_name, product_name) != 0) + continue; + + /* Matched — find the descriptor by family, model, and product name. */ + for (size_t j = 0; j < C_ARRAY_SIZE(g_descriptors); j++) { + if (g_descriptors[j].type == family && + g_descriptors[j].model == model && + strcmp(g_descriptors[j].product, + g_product_name_map[i].descriptor_product) == 0) { + return (dc_descriptor_t *) &g_descriptors[j]; + } + } + /* firmware_name matched but descriptor product not found — g_descriptors[] + * may have been edited without updating g_product_name_map[]. */ + return NULL; + } + + return NULL; +} + const char * dc_descriptor_get_vendor (const dc_descriptor_t *descriptor) { diff --git a/src/device.c b/src/device.c index e09b0eae..1e5780df 100644 --- a/src/device.c +++ b/src/device.c @@ -505,6 +505,9 @@ device_event_emit (dc_device_t *device, dc_event_type_t event, const void *data) device->devinfo.model = devinfo->model; device->devinfo.firmware = devinfo->firmware; device->devinfo.serial = devinfo->serial; + memset (device->devinfo.product_name, 0, sizeof (device->devinfo.product_name)); + memcpy (device->devinfo.product_name, devinfo->product_name, + sizeof (device->devinfo.product_name) - 1); data = &device->devinfo; } break; diff --git a/src/libdivecomputer.symbols b/src/libdivecomputer.symbols index 42e9c32b..b6fa32ff 100644 --- a/src/libdivecomputer.symbols +++ b/src/libdivecomputer.symbols @@ -37,6 +37,7 @@ dc_descriptor_get_model dc_descriptor_get_transports dc_descriptor_filter dc_descriptor_find_by_hw_id +dc_descriptor_find_by_product_name dc_iostream_get_transport dc_iostream_set_timeout diff --git a/src/mares_iconhd.c b/src/mares_iconhd.c index 9a55769f..5fc918c0 100644 --- a/src/mares_iconhd.c +++ b/src/mares_iconhd.c @@ -192,6 +192,7 @@ mares_iconhd_get_model (mares_iconhd_device_t *device) {"Smart Apnea", SMARTAPNEA}, {"Icon HD", ICONHD}, {"Icon AIR", ICONHDNET}, + {"Puck Pro U", PUCK4}, {"Puck Pro", PUCKPRO}, {"Nemo Wide 2", NEMOWIDE2}, {"Genius", GENIUS}, @@ -208,7 +209,6 @@ mares_iconhd_get_model (mares_iconhd_device_t *device) {"Puck4", PUCK4}, {"Puck Lite", PUCK4}, {"Puck", PUCK4}, - {"Puck Pro U", PUCK4}, }; // Check the product name in the version packet against the list @@ -224,6 +224,23 @@ mares_iconhd_get_model (mares_iconhd_device_t *device) return model; } +static void +mares_iconhd_fill_product_name (const mares_iconhd_device_t *device, dc_event_devinfo_t *devinfo) +{ + /* Copy at most DC_DEVINFO_PRODUCT_NAME_SIZE-1 bytes from the version packet + * product-name field (offset 0x46, up to 16 bytes) and NUL-terminate. + * The buffer is already zeroed by the caller initialising devinfo to {0}. */ + const unsigned char *src = device->version + 0x46; + unsigned int n = sizeof (devinfo->product_name) - 1; + unsigned int i; + for (i = 0; i < n; i++) { + if (src[i] == '\0') + break; + devinfo->product_name[i] = (char) src[i]; + } + devinfo->product_name[i] = '\0'; +} + static dc_status_t mares_iconhd_packet_fixed (mares_iconhd_device_t *device, unsigned char cmd, @@ -818,10 +835,11 @@ mares_iconhd_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) // Emit a device info event. unsigned char *data = dc_buffer_get_data (buffer); - dc_event_devinfo_t devinfo; + dc_event_devinfo_t devinfo = {0}; devinfo.model = device->model; devinfo.firmware = 0; devinfo.serial = array_uint32_le (data + 0x0C); + mares_iconhd_fill_product_name (device, &devinfo); device_event_emit (abstract, DC_EVENT_DEVINFO, &devinfo); return status; @@ -844,10 +862,11 @@ mares_iconhd_device_foreach_raw (dc_device_t *abstract, dc_dive_callback_t callb } // Emit a device info event. - dc_event_devinfo_t devinfo; + dc_event_devinfo_t devinfo = {0}; devinfo.model = device->model; devinfo.firmware = 0; devinfo.serial = array_uint32_le (serial); + mares_iconhd_fill_product_name (device, &devinfo); device_event_emit (abstract, DC_EVENT_DEVINFO, &devinfo); // Enable progress notifications. @@ -1097,10 +1116,11 @@ mares_iconhd_device_foreach_object (dc_device_t *abstract, dc_dive_callback_t ca unsigned int serial = array_convert_str2num (dc_buffer_get_data (buffer) + 10, 6); // Emit a device info event. - dc_event_devinfo_t devinfo; + dc_event_devinfo_t devinfo = {0}; devinfo.model = device->model; devinfo.firmware = 0; devinfo.serial = serial; + mares_iconhd_fill_product_name (device, &devinfo); device_event_emit (abstract, DC_EVENT_DEVINFO, &devinfo); // Erase the buffer. diff --git a/test/Makefile.am b/test/Makefile.am new file mode 100644 index 00000000..8f80e8da --- /dev/null +++ b/test/Makefile.am @@ -0,0 +1,15 @@ +AM_CPPFLAGS = -I$(top_builddir)/include -I$(top_srcdir)/include +LDADD = $(top_builddir)/src/libdivecomputer.la + +check_PROGRAMS = test_descriptor_product_name + +test_descriptor_product_name_SOURCES = test_descriptor_product_name.c + +TESTS = test_descriptor_product_name + +EXTRA_DIST = \ + run-parse.sh \ + fixtures/manifest.txt \ + fixtures/shearwater_petrel2-0001.bin \ + fixtures/hw_ostc5-0001.bin \ + fixtures/garmin_descent_mk1-0001.bin diff --git a/test/test_descriptor_product_name.c b/test/test_descriptor_product_name.c new file mode 100644 index 00000000..4d0751d8 --- /dev/null +++ b/test/test_descriptor_product_name.c @@ -0,0 +1,139 @@ +/* + * libdivecomputer + * + * Copyright (C) 2024 The libdivecomputer contributors + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + */ + +/* + * Unit tests for dc_descriptor_find_by_product_name(). + * + * Covers: + * - Mares Puck Pro vs Puck Pro + (shared model 0x18, PUCKPRO): the + * firmware name "Puck Pro" resolves to the "Puck Pro" descriptor. + * - Mares Puck 4 family (shared model 0x35, PUCK4): "Puck4", "Puck Lite", + * and "Puck Pro U" each resolve to the correct distinct descriptor. + * - Coarse-model fallback: an unrecognised product name returns NULL. + * - NULL / empty product name returns NULL. + */ + +#include +#include + +#include +#include + +#define PUCKPRO_MODEL 0x18 +#define PUCK4_MODEL 0x35 + +static int failures = 0; + +static void +check (const char *test, int condition) +{ + if (!condition) { + fprintf (stderr, "FAIL: %s\n", test); + failures++; + } else { + printf ("PASS: %s\n", test); + } +} + +int main (void) +{ + dc_descriptor_t *desc; + + /* --- Puck Pro (model 0x18) --- */ + + /* "Puck Pro" firmware name must resolve to the "Puck Pro" descriptor. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCKPRO_MODEL, "Puck Pro"); + check ("Puck Pro firmware name resolves to a descriptor", desc != NULL); + if (desc != NULL) { + check ("Puck Pro descriptor product is 'Puck Pro'", + strcmp (dc_descriptor_get_product (desc), "Puck Pro") == 0); + check ("Puck Pro descriptor model is 0x18", + dc_descriptor_get_model (desc) == PUCKPRO_MODEL); + dc_descriptor_free (desc); + } + + /* "Puck Pro +" has no confirmed firmware product-name; must return NULL + * so the caller falls back to the coarse-model descriptor. We pass the + * descriptor product name as the query to verify it is not in the table. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCKPRO_MODEL, "Puck Pro +"); + check ("'Puck Pro +' (not in firmware table) returns NULL", desc == NULL); + + /* --- Puck 4 family (model 0x35) --- */ + + /* "Puck4" must resolve to "Puck 4". */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, "Puck4"); + check ("'Puck4' firmware name resolves to a descriptor", desc != NULL); + if (desc != NULL) { + check ("'Puck4' descriptor product is 'Puck 4'", + strcmp (dc_descriptor_get_product (desc), "Puck 4") == 0); + check ("'Puck4' descriptor model is 0x35", + dc_descriptor_get_model (desc) == PUCK4_MODEL); + dc_descriptor_free (desc); + } + + /* "Puck Lite" must resolve to "Puck Lite". */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, "Puck Lite"); + check ("'Puck Lite' firmware name resolves to a descriptor", desc != NULL); + if (desc != NULL) { + check ("'Puck Lite' descriptor product is 'Puck Lite'", + strcmp (dc_descriptor_get_product (desc), "Puck Lite") == 0); + dc_descriptor_free (desc); + } + + /* "Puck Pro U" must resolve to "Puck Pro Ultra". */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, "Puck Pro U"); + check ("'Puck Pro U' firmware name resolves to a descriptor", desc != NULL); + if (desc != NULL) { + check ("'Puck Pro U' descriptor product is 'Puck Pro Ultra'", + strcmp (dc_descriptor_get_product (desc), "Puck Pro Ultra") == 0); + dc_descriptor_free (desc); + } + + /* --- Coarse-model fallback cases --- */ + + /* "Puck" maps to model 0x35 in the driver but is ambiguous; must return NULL. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, "Puck"); + check ("Ambiguous 'Puck' firmware name returns NULL", desc == NULL); + + /* Completely unknown product name returns NULL. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, "Unknown Product"); + check ("Unknown product name returns NULL", desc == NULL); + + /* NULL product name returns NULL. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, NULL); + check ("NULL product name returns NULL", desc == NULL); + + /* Empty product name returns NULL. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_ICONHD, PUCK4_MODEL, ""); + check ("Empty product name returns NULL", desc == NULL); + + /* Wrong family returns NULL even with a valid name. */ + desc = dc_descriptor_find_by_product_name (DC_FAMILY_MARES_PUCK, PUCK4_MODEL, "Puck4"); + check ("Wrong family returns NULL", desc == NULL); + + if (failures == 0) { + printf ("All tests passed.\n"); + return 0; + } + + fprintf (stderr, "%d test(s) failed.\n", failures); + return 1; +}