From afa6f432e48b186ed643ac5f349b49b0cdc0114a Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Tue, 10 Nov 2026 01:42:30 +0000 Subject: [PATCH 1/2] shearwater: Report the battery state of transmitters T3 and T4 The parser only reported "T1 battery" and "T2 battery". It was written when two transmitters were the most a Shearwater supported; current models support four. T3 and T4 pressures come from the extended sample record (log version 13 and later), which uses the same encoding as T1/T2 in the regular sample record: battery level in the top 4 bits, pressure in the lower 12. The extended record loop masked the battery bits off and discarded them. Accumulate them into tank[id].battery instead, inside the non-zero pressure check as the regular loop does, so an unpaired or zero reading contributes no state. Report T1 to T4 from a loop over the four open-circuit slots only. The HP CCR slots (indices 4 and 5) carry no battery level, and in older logs they are filled from the same words as T1/T2, so they must not be reported as "T5 battery" and "T6 battery". Add a synthetic test covering four transmitters, special codes and a zero pressure, a log version without the extended record, and an older HP CCR log. See https://github.com/subsurface/subsurface/issues/5119 Co-Authored-By: Claude Opus 5.5 Signed-off-by: Ben Doherty --- src/shearwater_predator_parser.c | 13 +- test/Makefile.am | 5 +- test/test_shearwater_tank_battery.c | 209 ++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+), 4 deletions(-) create mode 100644 test/test_shearwater_tank_battery.c diff --git a/src/shearwater_predator_parser.c b/src/shearwater_predator_parser.c index ff1174d8..20f8327e 100644 --- a/src/shearwater_predator_parser.c +++ b/src/shearwater_predator_parser.c @@ -664,7 +664,10 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser) for (unsigned int i = 0; i < 2; ++i) { unsigned int pressure = array_uint16_be (data + offset + pnf + i * 2); unsigned int id = 2 + i; + // Same encoding as T1/T2 in the regular sample record: + // battery level in the top 4 bits, pressure in the lower 12. if (pressure < 0xFFF0) { + unsigned int battery = 1u << (pressure >> 12); pressure &= 0x0FFF; if (pressure) { if (!tank[id].active) { @@ -673,6 +676,7 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser) tank[id].endpressure = pressure; } tank[id].endpressure = pressure; + tank[id].battery |= battery; } } } @@ -953,8 +957,13 @@ shearwater_predator_parser_cache (shearwater_predator_parser_t *parser) add_deco_model(parser, data); add_battery_type(parser, data); dc_field_add_string_fmt(&parser->cache, "Battery at end", "%.1f V", data[9] / 10.0); - add_battery_info(parser, "T1 battery", tank[0].battery); - add_battery_info(parser, "T2 battery", tank[1].battery); + // Only the four open-circuit transmitters (T1 to T4) report a battery + // state. The HP CCR slots (indices 4 and 5) carry no battery level. + static const char *battery_desc[] = { + "T1 battery", "T2 battery", "T3 battery", "T4 battery", + }; + for (unsigned int i = 0; i < C_ARRAY_SIZE(battery_desc); ++i) + add_battery_info(parser, battery_desc[i], tank[i].battery); switch (divemode) { case M_CC: diff --git a/test/Makefile.am b/test/Makefile.am index 42354868..6c561089 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -3,14 +3,15 @@ AUTOMAKE_OPTIONS = subdir-objects AM_CPPFLAGS = -I$(top_builddir)/include -I$(top_srcdir)/include LDADD = $(top_builddir)/src/libdivecomputer.la -check_PROGRAMS = test_descriptor_product_name test_mares_iconhd_freedive test_halcyon_symbios_gases +check_PROGRAMS = test_descriptor_product_name test_mares_iconhd_freedive test_halcyon_symbios_gases test_shearwater_tank_battery test_descriptor_product_name_SOURCES = test_descriptor_product_name.c test_mares_iconhd_freedive_SOURCES = test_mares_iconhd_freedive.c $(top_srcdir)/src/array.c test_mares_iconhd_freedive_CPPFLAGS = $(AM_CPPFLAGS) -I$(top_srcdir)/src test_halcyon_symbios_gases_SOURCES = test_halcyon_symbios_gases.c +test_shearwater_tank_battery_SOURCES = test_shearwater_tank_battery.c -TESTS = test_descriptor_product_name test_mares_iconhd_freedive test_halcyon_symbios_gases +TESTS = test_descriptor_product_name test_mares_iconhd_freedive test_halcyon_symbios_gases test_shearwater_tank_battery EXTRA_DIST = \ run-parse.sh \ diff --git a/test/test_shearwater_tank_battery.c b/test/test_shearwater_tank_battery.c new file mode 100644 index 00000000..42d0d13a --- /dev/null +++ b/test/test_shearwater_tank_battery.c @@ -0,0 +1,209 @@ +// AI-generated (Claude) +/* + * Synthetic Shearwater PNF logs: no device logs or identifying data. + * + * Checks the per-transmitter battery strings ("T1 battery" to "T4 battery"). + * T1/T2 come from the regular sample record and T3/T4 from the extended + * sample record (log version 13 and later). Each tank pressure word holds + * the battery level in the top 4 bits (0 = normal, 1 = critical, + * 2 = warning) and the pressure in the lower 12 bits; values from 0xFFF0 + * upward are special codes. + */ +#include +#include +#include + +#include +#include +#include + +#define SZ_RECORD 32 +#define NRECORDS_MAX 16 + +/* dc_descriptor_get_model() value for the "Perdix 2", see descriptor.c. */ +#define MODEL_PERDIX2 11 + +#define AI_HPCCR 4 + +/* Pressure word: battery level in the top 4 bits, pressure (2 psi units) below. */ +#define WORD(battery, pressure) (((battery) << 12) | (pressure)) +#define NORMAL 0 +#define CRITICAL 1 +#define WARNING 2 + +typedef struct log_t { + unsigned char data[NRECORDS_MAX * SZ_RECORD]; + unsigned int nrecords; +} log_t; + +static int failures = 0; + +static void +check (const char *name, int condition) +{ + if (!condition) { + fprintf (stderr, "FAIL: %s\n", name); + failures++; + } +} + +static unsigned char * +add_record (log_t *log, unsigned char type) +{ + unsigned char *record = log->data + log->nrecords++ * SZ_RECORD; + memset (record, 0, SZ_RECORD); + record[0] = type; + return record; +} + +static void +put_word (unsigned char *p, unsigned int value) +{ + p[0] = (value >> 8) & 0xFF; + p[1] = value & 0xFF; +} + +/* + * Build a minimal PNF log: opening records 0-4, one regular sample with the + * T1/T2 words, one extended sample with the T3/T4 words (both omitted when + * the log version predates them), and closing records 0-4. + */ +static void +build_log (log_t *log, unsigned int logversion, unsigned int aimode, + unsigned int t1, unsigned int t2, unsigned int t3, unsigned int t4) +{ + log->nrecords = 0; + for (unsigned char type = 0x10; type <= 0x13; ++type) + add_record (log, type); + unsigned char *opening4 = add_record (log, 0x14); + opening4[16] = logversion; + opening4[28] = aimode; + + unsigned char *sample = add_record (log, 0x01); + sample[1 + 11] = 0x10; /* Status: open circuit. */ + put_word (sample + 1 + 27, t1); + put_word (sample + 1 + 19, t2); + + if (logversion >= 13) { + unsigned char *ext = add_record (log, 0xE1); + put_word (ext + 1, t3); + put_word (ext + 3, t4); + } + + for (unsigned char type = 0x20; type <= 0x24; ++type) + add_record (log, type); +} + +static dc_descriptor_t * +perdix2_descriptor (void) +{ + dc_iterator_t *iterator = NULL; + dc_descriptor_t *descriptor = NULL; + dc_descriptor_t *item = NULL; + if (dc_descriptor_iterator (&iterator) != DC_STATUS_SUCCESS) + return NULL; + while (dc_iterator_next (iterator, &item) == DC_STATUS_SUCCESS) { + if (dc_descriptor_get_type (item) == DC_FAMILY_SHEARWATER_PETREL && + dc_descriptor_get_model (item) == MODEL_PERDIX2) { + descriptor = item; + break; + } + } + dc_iterator_free (iterator); + return descriptor; +} + +/* + * Check the battery strings reported for T1 to T4. A NULL expected value + * means the string must be absent. + */ +static void +check_batteries (const char *name, const log_t *log, const char *expected[4]) +{ + static const char *desc[4] = { + "T1 battery", "T2 battery", "T3 battery", "T4 battery", + }; + const char *found[4] = {NULL, NULL, NULL, NULL}; + char *values[4] = {NULL, NULL, NULL, NULL}; + unsigned int other = 0; + dc_parser_t *parser = NULL; + dc_descriptor_t *descriptor = perdix2_descriptor (); + + check ("Perdix 2 descriptor", descriptor != NULL); + if (descriptor == NULL) + return; + check ("create parser", dc_parser_new2 (&parser, NULL, descriptor, log->data, log->nrecords * SZ_RECORD) == DC_STATUS_SUCCESS); + dc_descriptor_free (descriptor); + if (parser == NULL) + return; + + for (unsigned int i = 0; ; ++i) { + dc_field_string_t str; + if (dc_parser_get_field (parser, DC_FIELD_STRING, i, &str) != DC_STATUS_SUCCESS) + break; + int matched = 0; + for (unsigned int t = 0; t < 4; ++t) { + if (strcmp (str.desc, desc[t]) == 0) { + free (values[t]); + values[t] = (char *) str.value; + found[t] = values[t]; + matched = 1; + } + } + if (!matched) { + if (strstr (str.desc, "battery") && str.desc[0] == 'T') + other++; /* e.g. a "T5 battery" for an HP CCR slot */ + free ((void *) str.value); + } + } + + for (unsigned int t = 0; t < 4; ++t) { + int ok = expected[t] == NULL ? found[t] == NULL : + found[t] != NULL && strcmp (found[t], expected[t]) == 0; + if (!ok) { + fprintf (stderr, "FAIL: %s: %s is \"%s\", expected \"%s\"\n", name, desc[t], + found[t] ? found[t] : "(absent)", expected[t] ? expected[t] : "(absent)"); + failures++; + } + free (values[t]); + } + check ("no battery string beyond T4", other == 0); + check ("destroy parser", dc_parser_destroy (parser) == DC_STATUS_SUCCESS); +} + +int +main (void) +{ + log_t log; + + /* All four transmitters, one of each state; T4 reports in the extended record. */ + build_log (&log, 17, 0, + WORD (NORMAL, 1000), WORD (CRITICAL, 1000), + WORD (WARNING, 1000), WORD (NORMAL, 1000)); + check_batteries ("four transmitters", &log, + (const char *[4]) {"normal", "critical", "warning", "normal"}); + + /* Special codes (no comms, not paired) and a zero pressure report no state. */ + build_log (&log, 17, 0, + WORD (NORMAL, 1000), 0xFFFC, + 0xFFFE, WORD (WARNING, 0)); + check_batteries ("special codes and zero pressure", &log, + (const char *[4]) {"normal", NULL, NULL, NULL}); + + /* Before log version 13 there is no extended record, so only T1/T2. */ + build_log (&log, 12, 0, + WORD (WARNING, 1000), WORD (NORMAL, 1000), 0, 0); + check_batteries ("log version 12", &log, + (const char *[4]) {"warning", "normal", NULL, NULL}); + + /* + * HP CCR in an older log: the regular sample words belong to the HP CCR + * slots (indices 4 and 5), which carry no battery state to report. + */ + build_log (&log, 12, AI_HPCCR, + WORD (WARNING, 1000), WORD (WARNING, 1000), 0, 0); + check_batteries ("HP CCR, log version 12", &log, + (const char *[4]) {NULL, NULL, NULL, NULL}); + + return failures != 0; +} From db5a62984be235d9672b835852afd8b7494db38b Mon Sep 17 00:00:00 2001 From: Ben Doherty Date: Tue, 10 Nov 2026 14:08:18 +1100 Subject: [PATCH 2/2] test: Tidy the Shearwater tank battery test after review Add a short comment to each helper function, as the automated review asked for. Check for the exact "T5 battery" and "T6 battery" strings rather than any string starting with "T" and containing "battery", and drop an array that only duplicated another. Correct the comment on the first case, which covers each of the three battery states across the four transmitters. No change to what is tested. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Ben Doherty --- test/test_shearwater_tank_battery.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/test/test_shearwater_tank_battery.c b/test/test_shearwater_tank_battery.c index 42d0d13a..167f6410 100644 --- a/test/test_shearwater_tank_battery.c +++ b/test/test_shearwater_tank_battery.c @@ -38,6 +38,7 @@ typedef struct log_t { static int failures = 0; +/* Count a failure, and report it, if the condition is false. */ static void check (const char *name, int condition) { @@ -47,6 +48,7 @@ check (const char *name, int condition) } } +/* Append an empty 32 byte record of the given type and return it. */ static unsigned char * add_record (log_t *log, unsigned char type) { @@ -56,6 +58,7 @@ add_record (log_t *log, unsigned char type) return record; } +/* Store a 16 bit value big endian, as in the Shearwater logs. */ static void put_word (unsigned char *p, unsigned int value) { @@ -94,6 +97,7 @@ build_log (log_t *log, unsigned int logversion, unsigned int aimode, add_record (log, type); } +/* Find the Perdix 2 descriptor, or return NULL if it is missing. */ static dc_descriptor_t * perdix2_descriptor (void) { @@ -123,8 +127,7 @@ check_batteries (const char *name, const log_t *log, const char *expected[4]) static const char *desc[4] = { "T1 battery", "T2 battery", "T3 battery", "T4 battery", }; - const char *found[4] = {NULL, NULL, NULL, NULL}; - char *values[4] = {NULL, NULL, NULL, NULL}; + char *found[4] = {NULL, NULL, NULL, NULL}; unsigned int other = 0; dc_parser_t *parser = NULL; dc_descriptor_t *descriptor = perdix2_descriptor (); @@ -144,15 +147,15 @@ check_batteries (const char *name, const log_t *log, const char *expected[4]) int matched = 0; for (unsigned int t = 0; t < 4; ++t) { if (strcmp (str.desc, desc[t]) == 0) { - free (values[t]); - values[t] = (char *) str.value; - found[t] = values[t]; + free (found[t]); + found[t] = (char *) str.value; matched = 1; } } if (!matched) { - if (strstr (str.desc, "battery") && str.desc[0] == 'T') - other++; /* e.g. a "T5 battery" for an HP CCR slot */ + /* An HP CCR slot must not show up as "T5 battery" or "T6 battery". */ + if (strcmp (str.desc, "T5 battery") == 0 || strcmp (str.desc, "T6 battery") == 0) + other++; free ((void *) str.value); } } @@ -165,18 +168,19 @@ check_batteries (const char *name, const log_t *log, const char *expected[4]) found[t] ? found[t] : "(absent)", expected[t] ? expected[t] : "(absent)"); failures++; } - free (values[t]); + free (found[t]); } check ("no battery string beyond T4", other == 0); check ("destroy parser", dc_parser_destroy (parser) == DC_STATUS_SUCCESS); } +/* Run each synthetic log through the parser and check its battery strings. */ int main (void) { log_t log; - /* All four transmitters, one of each state; T4 reports in the extended record. */ + /* T1/T2 in the regular sample record, T3/T4 in the extended one, covering each state. */ build_log (&log, 17, 0, WORD (NORMAL, 1000), WORD (CRITICAL, 1000), WORD (WARNING, 1000), WORD (NORMAL, 1000));