diff --git a/motion_connector.py b/motion_connector.py index 9628cc7..b4ff650 100644 --- a/motion_connector.py +++ b/motion_connector.py @@ -27,6 +27,7 @@ from omotion.GitHubReleases import GitHubReleases from motion_singleton import motion_interface from histogram_classifier import classify_histogram +from utils.nvcm_verdict import interpret_nvcm_blob from fpga_laser_config import ( FpgaModel, apply_laser_power_from_config, @@ -548,33 +549,6 @@ def cb(done, total, cam=cam): self.finishedAll.emit(all_ok, summary) -def _interpret_nvcm_blob(blob: bytes) -> tuple[str, str]: - """Reduce an OW_FACTORY_NVCM_CHECK response to a verdict + detail string. - - Ported from openmotion-sdk scripts/nvcm_probe.py. The behaviorally- - definitive signal is the auto-boot test: with a valid IDCODE, the config - port at 0x40 *disappearing* after CRESETB is released (without the - activation key) means the FPGA booted a user design from NVCM. - - Returns (verdict, detail) where verdict is one of: - PROGRAMMED, BLANK, INCONCLUSIVE, NO RESPONSE. - """ - if not blob: - return "NO RESPONSE", "firmware returned no data (camera absent/unpowered?)" - if len(blob) < 27: - return "NO RESPONSE", f"short response ({len(blob)} bytes)" - idcode_ok = blob[4] - boot_probe_done = blob[24] - boot_0x40_responds = blob[25] - if idcode_ok != 1: - return "INCONCLUSIVE", "IDCODE mismatch — check power / mux / CRESETB" - if not boot_probe_done: - return "INCONCLUSIVE", "boot test did not run" - if boot_0x40_responds == 0: - return "PROGRAMMED", "0x40 disappeared after auto-boot" - return "BLANK", "0x40 still ACKs — nothing auto-booted" - - class _NvcmCheckThread(QThread): """Read-only sweep that probes NVCM programmed-state on all 8 cameras. @@ -611,8 +585,12 @@ def run(self): time.sleep(0.3) sensor.switch_camera(idx) time.sleep(0.1) - blob = sensor.nvcm_check() - verdict, detail = _interpret_nvcm_blob(blob) + # boot_test=False: the auto-boot 0x40 probe carries no + # information (0x40 needs the activation key to respond + # at all — issue #44); the verdict comes from the STATUS + # Done bit, which the probe reads regardless. + blob = sensor.nvcm_check(boot_test=False) + verdict, detail = interpret_nvcm_blob(blob) except Exception as exc: # never let the thread die silently logger.exception("NVCM check raised for camera %d", cam) verdict, detail = "NO RESPONSE", str(exc) diff --git a/tests/test_nvcm_verdict.py b/tests/test_nvcm_verdict.py new file mode 100644 index 0000000..11ccf88 --- /dev/null +++ b/tests/test_nvcm_verdict.py @@ -0,0 +1,113 @@ +"""Unit tests for utils.nvcm_verdict.interpret_nvcm_blob (issue #44). + +Run from the repo root with: python -m pytest tests/test_nvcm_verdict.py + +The old interpreter derived PROGRAMMED from the auto-boot "0x40 stopped +ACKing" signal, which is unconditionally true on this part (the CrossLink +config port needs the activation key to respond at all), so every camera +with a valid IDCODE read as PROGRAMMED. The verdict must instead come from +STATUS bit 19 (blob[7] bit 3) — the bit that empirically discriminates +programmed (00 08 02 08) from blank (00 00 02 08) parts in the probe's +ISC flow (16-camera hardware sweep, 2026-07-02). The SRAM Done bit +(blob[8] bit 0) reads 0 on every camera in this flow and cannot be used. +""" +import pytest + +from utils.nvcm_verdict import STEP_STATUS, interpret_nvcm_blob + +IDCODE = bytes([0x01, 0x2C, 0x00, 0x43]) + + +def make_blob(idcode_ok=1, step_status=0x7F, status=b"\x00\x00\x02\x08", + feature_row=b"\xFF" * 8, feabits=b"\xFF\xFF", + usercode=b"\x00" * 4, boot_probe_done=0, + boot_0x40_responds=0, rows=(b"\xFF" * 16,)): + """Build an OW_FACTORY_NVCM_CHECK response blob (see nvcm_verdict).""" + blob = bytearray() + blob += IDCODE + blob.append(idcode_ok) + blob.append(step_status) + blob += status + blob += feature_row + blob += feabits + blob += usercode + blob.append(boot_probe_done) + blob.append(boot_0x40_responds) + blob.append(len(rows)) + for row in rows: + blob += row + return bytes(blob) + + +def test_empty_blob_is_no_response(): + verdict, detail = interpret_nvcm_blob(b"") + assert verdict == "NO RESPONSE" + assert "no data" in detail + + +def test_short_blob_is_no_response(): + verdict, detail = interpret_nvcm_blob(b"\x01\x2C\x00\x43\x01") + assert verdict == "NO RESPONSE" + assert "5 bytes" in detail + + +def test_idcode_mismatch_is_inconclusive(): + verdict, _ = interpret_nvcm_blob(make_blob(idcode_ok=0)) + assert verdict == "INCONCLUSIVE" + + +def test_missing_status_read_is_inconclusive(): + # step_status without the STATUS bit: Done can't be trusted. + verdict, detail = interpret_nvcm_blob( + make_blob(step_status=0x7F & ~STEP_STATUS)) + assert verdict == "INCONCLUSIVE" + assert "STATUS" in detail + + +def test_blank_part_regression_issue_44(): + """Real blank-camera blob (sensor-fw NVCM notebook, cam8 pre-burn): + STATUS 00 00 02 08 -> Done=0, but the boot test reported "0x40 gone" + (boot_0x40_responds=0), which the old code read as PROGRAMMED.""" + blob = make_blob(status=b"\x00\x00\x02\x08", + boot_probe_done=1, boot_0x40_responds=0) + verdict, detail = interpret_nvcm_blob(blob) + assert verdict == "BLANK" + assert "00 00 02 08" in detail + + +def test_programmed_part_bit19_set(): + """Real programmed-camera blob (16-camera bench sweep 2026-07-02: + all 15 programmed cameras read STATUS 00 08 02 08).""" + blob = make_blob(status=b"\x00\x08\x02\x08") + verdict, detail = interpret_nvcm_blob(blob) + assert verdict == "PROGRAMMED" + assert "00 08 02 08" in detail + + +def test_boot_test_bytes_are_ignored(): + """The auto-boot bytes carry no information; the verdict must not + change with them (old code flipped PROGRAMMED/BLANK on blob[25]).""" + for b1, expected in ((0x00, "BLANK"), (0x08, "PROGRAMMED")): + status = bytes([0x00, b1, 0x02, 0x08]) + verdicts = { + interpret_nvcm_blob(make_blob(status=status, + boot_probe_done=probe, + boot_0x40_responds=ack))[0] + for probe in (0, 1) for ack in (0, 1) + } + assert verdicts == {expected} + + +def test_no_rows_blob_is_still_parseable(): + # num_rows=0 gives the 27-byte minimum layout. + blob = make_blob(rows=()) + assert len(blob) == 27 + verdict, _ = interpret_nvcm_blob(blob) + assert verdict == "BLANK" + + +@pytest.mark.parametrize("extra", [b"", b"\x00" * 16]) +def test_verdict_independent_of_row_payload(extra): + blob = make_blob(rows=(b"\x00" * 16,)) + extra + verdict, _ = interpret_nvcm_blob(blob) + assert verdict == "BLANK" diff --git a/utils/nvcm_verdict.py b/utils/nvcm_verdict.py new file mode 100644 index 0000000..fd3dd3e --- /dev/null +++ b/utils/nvcm_verdict.py @@ -0,0 +1,60 @@ +"""Interpret an OW_FACTORY_NVCM_CHECK response blob. Pure logic, no Qt. + +Blob layout (sensor-fw ``if_factory_prog.c``, command 0x6C):: + + [0:4] IDCODE [4] idcode_ok [5] step_status + [6:10] STATUS (big-endian, bits[31:24] first) + [10:18] feature_row [18:20] feabits [20:24] usercode + [24] boot_probe_done [25] boot_0x40_responds + [26] num_rows_read [27:] NVCM rows (16 B each) + +The programmed/blank verdict comes from STATUS **bit 19** (byte +``blob[7]`` bit 3 — big-endian word bit 0x00080000). Empirically verified +on hardware 2026-07-02 with a 16-camera blind sweep: all 15 +NVCM-programmed cameras read STATUS ``00 08 02 08`` (bit 19 set) and the +one known-blank camera read ``00 00 02 08`` (bit 19 clear), matching the +known-blank blob in the openmotion-sensor-fw notebook. The SRAM +configuration Done bit (bit 8 -> ``blob[8]`` bit 0) is NOT usable here: +the probe's ISC sequence holds the part unconfigured, so Done reads 0 on +every camera regardless of NVCM state. The feature-row / feabits / +NVCM-row reads return all-0xFF for programmed and blank parts alike in +this mode, so they cannot discriminate either. + +The auto-boot bytes ([24]/[25]) are deliberately IGNORED: the CrossLink +I2C slave config port at 0x40 only becomes active after receiving the +activation key, so after a key-less CRESETB release 0x40 never ACKs +regardless of NVCM state. Interpreting "0x40 gone" as "programmed" made +the check report PROGRAMMED for blank parts too (issue #44). +""" + +# step_status bits (sensor-fw crosslink.h FPGA_NVCM_STEP_*) +STEP_ACTIVATION = 1 << 0 +STEP_IDCODE = 1 << 1 +STEP_ISC_ENABLE = 1 << 2 +STEP_STATUS = 1 << 3 +STEP_FEATROW = 1 << 4 +STEP_FEABITS = 1 << 5 +STEP_USERCODE = 1 << 6 + +_MIN_BLOB_LEN = 27 + + +def interpret_nvcm_blob(blob: bytes) -> tuple[str, str]: + """Reduce an OW_FACTORY_NVCM_CHECK response to a verdict + detail string. + + Returns (verdict, detail) where verdict is one of: + PROGRAMMED, BLANK, INCONCLUSIVE, NO RESPONSE. + """ + if not blob: + return ("NO RESPONSE", + "firmware returned no data (camera absent/unpowered?)") + if len(blob) < _MIN_BLOB_LEN: + return "NO RESPONSE", f"short response ({len(blob)} bytes)" + if blob[4] != 1: + return "INCONCLUSIVE", "IDCODE mismatch — check power / mux / CRESETB" + if not (blob[5] & STEP_STATUS): + return "INCONCLUSIVE", "STATUS register read failed" + status_hex = " ".join(f"{b:02X}" for b in blob[6:10]) + if blob[7] & 0x08: # STATUS bit 19 — NVCM-programmed discriminator + return "PROGRAMMED", f"STATUS bit 19 set (status {status_hex})" + return "BLANK", f"STATUS bit 19 clear (status {status_hex})"