From 782d28932c27fabc8ab9fa8b827856a581d7e297 Mon Sep 17 00:00:00 2001 From: Matthew Carroll Date: Thu, 10 Sep 2026 10:33:17 -0700 Subject: [PATCH] lf: restore the graph buffer length along with its contents restore_bufferS32() returns the length it put back and every graph-buffer caller dropped it, so g_GraphTraceLen kept whatever value the code between save and restore had left. In try_detect_modulation() that is the 160 sample psk antenna-settle trim: `data modulation` gave the buffer's samples back but left it 160 short, and a following demod lost round(160 / clk) bits off the end. check_chiptype() loses the same way through the acquisitions its detections make, on the `lf search` path, and for g_DemodBuffer too. demodTI() loses convLen + 16 on its error path. save_graphbuffer() / restore_graphbuffer() pair the length and grid offset with the contents so no call site has to remember them. The visa2000 and zx8211 sites do not change the length, so converting them is a no-op today and keeps the next one from being a bug. Regression test in tools/pm3_tests.sh: `data modulation` on a saved psk1 trace must leave all 20000 samples. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + client/src/cmddata.c | 6 ++---- client/src/cmdlf.c | 9 ++++----- client/src/cmdlfti.c | 6 ++---- client/src/cmdlfvisa2000.c | 15 +++++---------- client/src/cmdlfzx8211.c | 9 +++------ client/src/graph.c | 18 ++++++++++++++++++ client/src/graph.h | 3 +++ tools/pm3_tests.sh | 1 + 9 files changed, 39 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ceef461b4..1dcb9a79c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to this project will be documented in this file. This project uses the changelog in accordance with [keepchangelog](http://keepachangelog.com/). Please use this to write notable changes, which is not the same as git commit log... ## [unreleased][unreleased] +- Fixed `data modulation` - the 160 sample antenna settle trim was never undone, so the graph buffer stayed short for everything after it (@mfcarroll) - Fixed `lf t55xx detect` - PSK1 now detects at every bit rate and subcarrier. (@iceman1001) - Fixed `lf t55xx detect` - FSK was skipped entirely when the field-clock pair measured as neither legal pair, losing FSK1 at RF/32 and RF/40 and every variant at RF/16 (@iceman1001) - Fixed `lf t55xx dump/read` - blocks were extracted at a bit offset cached from the last `detect` The offset is now anchored in graph samples (@iceman1001) diff --git a/client/src/cmddata.c b/client/src/cmddata.c index 411515da9..979ecc31b 100644 --- a/client/src/cmddata.c +++ b/client/src/cmddata.c @@ -2778,8 +2778,7 @@ static int try_detect_modulation(void) { clk = GetPskClock("", false); if (clk > 0) { // allow undo - buffer_savestate_t saveState = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); - saveState.offset = g_GridOffset; + buffer_savestate_t saveState = save_graphbuffer(); // skip first 160 samples to allow antenna to settle in (psk gets inverted occasionally otherwise) CmdLtrim("-i 160"); if ((PSKDemod(0, 0, 6, false) == PM3_SUCCESS)) { @@ -2791,8 +2790,7 @@ static int try_detect_modulation(void) { tests[hits].carrier = GetPskCarrier(false); } //undo trim samples - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); } } diff --git a/client/src/cmdlf.c b/client/src/cmdlf.c index bf8186d73..26fb3d83f 100644 --- a/client/src/cmdlf.c +++ b/client/src/cmdlf.c @@ -1609,8 +1609,7 @@ static bool check_chiptype(bool getDeviceData) { } //Save the state of the Graph and Demod Buffers - buffer_savestate_t saveState_gb = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); - saveState_gb.offset = g_GridOffset; + buffer_savestate_t saveState_gb = save_graphbuffer(); buffer_savestate_t saveState_db = save_buffer8(g_DemodBuffer, g_DemodBufferLen); saveState_db.clock = g_DemodClock; saveState_db.offset = g_DemodStartIdx; @@ -1683,12 +1682,12 @@ static bool check_chiptype(bool getDeviceData) { PrintAndLogEx(INFO, "Couldn't identify a chipset"); out: - restore_buffer8(saveState_db, g_DemodBuffer); + // the detections above demodulate, so the length comes back with the contents + g_DemodBufferLen = restore_buffer8(saveState_db, g_DemodBuffer); g_DemodClock = saveState_db.clock; g_DemodStartIdx = saveState_db.offset; - restore_bufferS32(saveState_gb, g_GraphBuffer); - g_GridOffset = saveState_gb.offset; + restore_graphbuffer(saveState_gb); return retval; } diff --git a/client/src/cmdlfti.c b/client/src/cmdlfti.c index 17d843a7c..57965d8d2 100644 --- a/client/src/cmdlfti.c +++ b/client/src/cmdlfti.c @@ -89,8 +89,7 @@ int demodTI(bool verbose) { 1, 1, 1, 1, 1, 1, 1, 1 }; - buffer_savestate_t saveState = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); - saveState.offset = g_GridOffset; + buffer_savestate_t saveState = save_graphbuffer(); int lowLen = ARRAYLEN(LowTone); int highLen = ARRAYLEN(HighTone); @@ -278,8 +277,7 @@ int demodTI(bool verbose) { out: if (retval != PM3_SUCCESS) { - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); } return retval; diff --git a/client/src/cmdlfvisa2000.c b/client/src/cmdlfvisa2000.c index 092f38e30..93e8ff466 100644 --- a/client/src/cmdlfvisa2000.c +++ b/client/src/cmdlfvisa2000.c @@ -84,8 +84,7 @@ static uint8_t visa_parity(uint32_t id) { //see ASKDemod for what args are accepted int demodVisa2k(bool verbose) { (void) verbose; // unused so far - buffer_savestate_t saveState = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); - saveState.offset = g_GridOffset; + buffer_savestate_t saveState = save_graphbuffer(); //CmdAskEdgeDetect(""); @@ -93,8 +92,7 @@ int demodVisa2k(bool verbose) { bool st = true; if (ASKDemod_ext(64, 0, 0, 0, false, false, false, 1, &st) != PM3_SUCCESS) { PrintAndLogEx(DEBUG, "DEBUG: Error - Visa2k: ASK/Manchester Demod failed"); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } size_t size = g_DemodBufferLen; @@ -109,8 +107,7 @@ int demodVisa2k(bool verbose) { else PrintAndLogEx(DEBUG, "DEBUG: Error - Visa2k: ans: %d", ans); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } setDemodBuff(g_DemodBuffer, 96, ans); @@ -128,8 +125,7 @@ int demodVisa2k(bool verbose) { // test checksums if (chk != calc) { PrintAndLogEx(DEBUG, "DEBUG: error: Visa2000 checksum (%s) %x - %x\n", _RED_("fail"), chk, calc); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } // parity @@ -137,8 +133,7 @@ int demodVisa2k(bool verbose) { uint8_t chk_par = (raw3 & 0xFF0) >> 4; if (calc_par != chk_par) { PrintAndLogEx(DEBUG, "DEBUG: error: Visa2000 parity (%s) %x - %x\n", _RED_("fail"), chk_par, calc_par); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } PrintAndLogEx(SUCCESS, "Visa2000 - Card " _GREEN_("%u") ", Raw: %08X%08X%08X", raw2, raw1, raw2, raw3); diff --git a/client/src/cmdlfzx8211.c b/client/src/cmdlfzx8211.c index 793cc3d54..00e19d152 100644 --- a/client/src/cmdlfzx8211.c +++ b/client/src/cmdlfzx8211.c @@ -41,8 +41,7 @@ static int CmdHelp(const char *Cmd); // see ASKDemod for what args are accepted int demodzx(bool verbose) { (void) verbose; // unused so far - buffer_savestate_t saveState = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); - saveState.offset = g_GridOffset; + buffer_savestate_t saveState = save_graphbuffer(); // CmdAskEdgeDetect(""); @@ -50,8 +49,7 @@ int demodzx(bool verbose) { bool st = true; if (ASKDemod_ext(64, 0, 0, 0, false, false, false, 1, &st) != PM3_SUCCESS) { PrintAndLogEx(DEBUG, "DEBUG: Error - ZX: ASK/Manchester Demod failed"); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } size_t size = g_DemodBufferLen; @@ -66,8 +64,7 @@ int demodzx(bool verbose) { else PrintAndLogEx(DEBUG, "DEBUG: Error - ZX: ans: %d", ans); - restore_bufferS32(saveState, g_GraphBuffer); - g_GridOffset = saveState.offset; + restore_graphbuffer(saveState); return PM3_ESOFT; } setDemodBuff(g_DemodBuffer, 96, ans); diff --git a/client/src/graph.c b/client/src/graph.c index 80c1e4dc0..8df84d829 100644 --- a/client/src/graph.c +++ b/client/src/graph.c @@ -648,3 +648,21 @@ size_t restore_buffer8(buffer_savestate_t saveState, uint8_t *dest) { return index; } + +// Save/restore the graph buffer as a unit: contents, length and grid offset. +// A caller that drops restore_bufferS32()'s return leaves g_GraphTraceLen short. +buffer_savestate_t save_graphbuffer(void) { + buffer_savestate_t saveState = save_bufferS32(g_GraphBuffer, g_GraphTraceLen); + saveState.offset = g_GridOffset; + return saveState; +} + +void restore_graphbuffer(buffer_savestate_t saveState) { + size_t len = restore_bufferS32(saveState, g_GraphBuffer); + if (len == 0) { + // nothing was restored, so leave the buffer and its length alone + return; + } + g_GraphTraceLen = len; + g_GridOffset = saveState.offset; +} diff --git a/client/src/graph.h b/client/src/graph.h index 6f0822f96..1a46d6362 100644 --- a/client/src/graph.h +++ b/client/src/graph.h @@ -67,6 +67,9 @@ size_t restore_buffer32(buffer_savestate_t saveState, uint32_t *dest); size_t restore_bufferS32(buffer_savestate_t saveState, int32_t *dest); size_t restore_buffer8(buffer_savestate_t saveState, uint8_t *dest); +buffer_savestate_t save_graphbuffer(void); +void restore_graphbuffer(buffer_savestate_t saveState); + #define MAX_GRAPH_TRACE_LEN (40000 * 32) #define GRAPH_SAVE 1 #define GRAPH_RESTORE 0 diff --git a/tools/pm3_tests.sh b/tools/pm3_tests.sh index a63279319..b0f257ab6 100755 --- a/tools/pm3_tests.sh +++ b/tools/pm3_tests.sh @@ -468,6 +468,7 @@ while true; do if ! CheckExecute "data qrcode invalid hex" "$CLIENTBIN -c 'data qrcode -d zz' 2>&1" "QR data must contain only hex characters"; then break; fi if ! CheckExecute "data qrcode odd hex" "$CLIENTBIN -c 'data qrcode -d a' 2>&1" "QR data must contain an even number of hex digits"; then break; fi if ! CheckExecute "data qrcode spaced hex" "$CLIENTBIN -c 'data qrcode -d \"aa bb\"' 2>&1" "Spaces are not supported; encode a space byte as 20"; then break; fi + if ! CheckExecute "data modulation keeps trace" "$CLIENTBIN -c 'data load -f traces/lf_Q5_mod-psk1-32-4.pm3; data modulation; data autocorr -w 99999'" "smaller than trace \\( 20000 samples \\)"; then break; fi if ! CheckExecute "mfu pwdgen test" "$CLIENTBIN -c 'hf mfu pwdgen --test'" "Selftest ok"; then break; fi if ! CheckExecute "mfu keygen test" "$CLIENTBIN -c 'hf mfu keygen --uid 11223344556677'" "80 B1 C2 71 D8 A0"; then break; fi if ! CheckExecute "jooki encode test" "$CLIENTBIN -c 'hf jooki encode --test'" "04 28 F4 DA F0 4A 81 \( ok \)"; then break; fi