chore: check in WIP docs and da07 frame capture before machine switch

2026-06-12 11:36:57 -04:00
parent 0e9761e0c3
commit e20680bfe0
3 changed files with 1686 additions and 0 deletions
--- a/da07-frames.txt
+++ b/da07-frames.txt
--- a/docs/guided-acquire-spec.md
+++ b/docs/guided-acquire-spec.md
@@ -0,0 +1,247 @@
+# Guided Acquire — service-tool flow specification
+
+A specification for a **safe, operator-guided sensor acquisition** feature in the new Python
+service tool. It replaces both the raw "set Acquire = 1" register write and the field procedure of
+button-press beep commands with a wizard that snapshots, guards, scans, verifies, and repairs.
+Like the [protocol reference](service-tool-protocol.md), this document is self-contained and can be
+copied into the service-tool repo. All firmware behavior is cited as `file.c:line`.
+
+!!! note "Scope & baseline"
+    Targets the **DA-12C** build (wired SensorBus sensors only, `MAX_SENSORS = 16`). Firmware
+    changes are **not authorized** — everything here is host-side, built on the existing `{…}`
+    protocol (see the [DA-12C Service-Tool Protocol reference](service-tool-protocol.md), §4–§5).
+    Command builders and a decoder already exist in
+    [`reference/da12c_status.py`](reference/da12c_status.py).
+
+---
+
+## 1. Why a guided flow is needed
+
+The firmware's acquire operation is **wipe-then-rescan**, and it has a failure mode that
+empties the station permanently:
+
+1. **Every scan begins by deleting the database.** `SensorsAcquire()` calls
+   `SbRemoveAllSensors()` *before* searching the bus (`sensors.c:1130`), which clears each
+   sensor's EEPROM active bit via `RemoveSensor()` → `SetSensorActive(num, FALSE)`
+   (`pointsix.c:108-129`, `eepromat.c:588`). The wipe is **persistent**: at boot,
+   `InitSensors()` only loads sensors whose active bit is set (`pointsix.c:70`).
+2. **In restricted mode, the rescan recovers nothing.** Found bus devices are added through
+   `GetP6Num(1, serial)` (`owi.c:945`), and the restricted-mode gate rejects any type that is
+   not an RF *service-mode* type (`pointsix.c:693-704`). Wired sensors are always added with
+   temporary type `1`, which never passes. So **triggering a scan while setting #3
+   (Sensor mode) = 1 wipes every sensor and re-adds none.** This applies equally to the
+   host-triggered scan (setting #4) and to the 3/4-beep button commands.
+3. **There is no automatic recovery.** The firmware never rescans on its own — not at boot,
+   not periodically. The only production triggers for `SensorsAcquire()` are the button
+   commands (`indicators.c:889,900,910`) and a setting-#4 write (`op05.c:1167-1168`).
+4. **Mitigating fact that makes a guided flow viable:** serial numbers, names, and calibration
+   survive the wipe (only the active bit is cleared). A later successful scan restores each
+   sensor into its old slot with settings intact via `FindSnIndex()` (`pointsix.c:714-730`).
+   Repeating a scan is therefore cheap and safe — *if* the mode guard holds.
+
+The guided flow exists to make item 2 impossible, item 3 invisible to the operator, and item 4
+automatic.
+
+---
+
+## 2. Firmware contract (what the tool builds on)
+
+| Behavior | Detail | Source |
+|----------|--------|--------|
+| Trigger | Writing setting **#4 = 1** sets `gStation.acquire` **and** `AcquireSensors` | `op05.c:1167-1168` |
+| Execution | Next main-loop pass runs `SwitchesAcquire()` + `SensorsAcquire()` **synchronously**; the loop is blocked for the duration (watchdog is fed inside) | `main.c:874-880` |
+| Comm gap | While the scan runs, the unit emits nothing — the periodic `{F}` status frame (~1.2 s cadence) **pauses, then resumes** when the scan completes. This is the completion signal. | `op05.c:1644-1686` |
+| Persistence | Setting #4 is deliberately **not** saved to EEPROM (`num != 4` guard) | `op05.c:1303` |
+| Lingering flag | `gStation.acquire` stays 1 after the scan (fast-blinking green LED) until written back to 0 | `indicators.c:670-674` |
+| Mode guard | Setting **#3** (Sensor mode / `opMode`) **must be 0** when the scan runs; writes to #3 persist via `SaveStationSettings()` | `pointsix.c:693-704`, `op05.c:1303` |
+| Echo | Every accepted setting write is echoed back as a `{D}`/`{E}` record — use as the write-ack | `op05.c:1409-1411` |
+| Slot recovery | Re-found serials reclaim their old EEPROM slot, keeping name/scale/offset/units/alarms | `pointsix.c:714-730` |
+| New sensors | Genuinely new serials get a fresh slot with blank name and default settings | `pointsix.c:614-651` (`AddNewSensor`) |
+| Ghost entries | `RemoveSensor()` deactivates the wrong index for **linked** (dual-channel) sensors — the linked half keeps its active bit and can reappear as a database entry with no bus link (never polled) | bug at `pointsix.c:124` |
+| Manual add | `{S}` (add by serial) calls `AddNewSensor(serial, 1)`: persists and marks active, but type stays `1` and no bus link is built — the sensor is **not functional** until a successful scan; `InitSensorSettings()` also resets name/calibration | `op05.c:1353-1360`, `pointsix.c:372-392` |
+
+!!! warning "The one invariant"
+    **Never write setting #4 = 1 while setting #3 = 1.** Everything else in this flow is
+    polish; this rule is the difference between an acquire and a wipe. The tool must enforce it
+    in code, not in documentation — the raw #4 toggle should not be exposed in the UI at all.
+
+---
+
+## 3. The flow
+
+Eight states. All wire commands below are the §4 inbound frames from the protocol reference
+(`{ cmd typecode index value }` + CR); use the builders in `da12c_status.py` rather than
+hand-formatting.
+
+```
+S0 Connect → S1 Snapshot → S2 Mode guard → S3 Trigger → S4 Wait
+   → S5 Verify/Diff → S6 Remediate (loop to S3) → S7 Cleanup & report
+```
+
+### S0 — Connect
+
+1. Open the service port, **19200 8N1**.
+2. Send `{Y}` (refresh-all). Parse the full settings stream (`{D}`/`{E}`), sensor records
+   (`{A}`), alarm settings (`{G}`), statistics (`{H}`).
+3. Confirm the unit is a DA-12C (setting #20 firmware version, #5 MAC) and that `{F}` frames
+   are arriving on the ~1.2 s cadence — the cadence baseline is needed in S4.
+
+**Abort if:** no `{Y}` response, or `{F}` cadence not established within 10 s.
+
+### S1 — Snapshot (the safety net)
+
+Write a **recovery file** to local disk *before any mutation*. Contents (JSON, schema in §5):
+
+- Station identity: MAC, name, firmware version, timestamp.
+- All settings (#1–#33) with raw values — including the **original value of #3**.
+- Every sensor record: index, serial, type, name, scale, offset, units, decimal digits,
+  stats setting, alarm config (active, delay, 4 limits).
+
+This file is the audit trail and the manual-repair source of last resort. The flow must not
+proceed until the file is written and re-read successfully.
+
+### S2 — Mode guard
+
+1. Read setting #3 from the S1 snapshot.
+2. If `#3 == 1` (restricted): write `#3 = 0` and wait for the `{E03…}` echo confirming `0`.
+   Surface this to the operator: *"Station was in restricted sensor mode; switched to normal
+   for acquisition."* Note that this write persists to EEPROM (`op05.c:1303`) — intentional,
+   and it also heals any RAM-vs-EEPROM mode drift left behind by earlier button commands
+   (button cases 4/5 change `opMode` without saving — `indicators.c:893,904`).
+3. Record `original_mode` for S7.
+
+**Abort if:** echo not received or echoes a non-zero value (write rejected/garbled).
+
+### S3 — Trigger
+
+1. Write `#4 = 1`. Expect the `{E04…}` echo.
+2. Mark the trigger timestamp. Do **not** send further commands until S4 completes — the main
+   loop is about to block, and queued traffic complicates the gap detection.
+
+### S4 — Wait for completion
+
+The scan blocks the firmware main loop, so the `{F}` cadence is the progress indicator:
+
+1. Wait for `{F}` frames to **stop** (no frame for > 3 s ⇒ scan underway).
+2. Wait for `{F}` frames to **resume** (first frame after the gap ⇒ scan finished).
+3. **Timeout: 60 s** from trigger. The bus walk visits every DS2413 switch tap with 3×
+   retries and 30–90 ms delays per failure (`switches.c:144-151`, `sensors.c:1125-1158`), so a
+   healthy 16-sensor unit finishes in seconds; a minute means a wedged bus.
+
+Edge case: if `{F}` never pauses, the scan may have run between two frames (small bus) — treat
+a stable post-trigger table in S5 as success regardless.
+
+**On timeout:** skip to S7-cleanup, then report *"scan did not complete — check 1-Wire bus
+wiring"* alongside the S1 snapshot contents. Do not retry automatically into a dead bus.
+
+### S5 — Verify & diff
+
+1. Send `{Y}`; parse the sensor table. Repeat until **two consecutive reads are identical**
+   (guards against reading mid-stream).
+2. Diff against the S1 snapshot, keyed by **serial number**:
+
+| Category | Definition | Disposition |
+|----------|------------|-------------|
+| **Recovered** | serial in both snapshots | OK — old slot, name/calibration intact |
+| **New** | serial only in post-scan table, has bus link | prompt for name in S6 |
+| **Missing** | serial only in S1 snapshot | remediation list for S6 |
+| **Ghost** | post-scan entry with no bus link / never updating (linked-sensor bug, `pointsix.c:124`) | offer `{R}` removal in S6 |
+
+3. Present the diff to the operator **by sensor name and serial**, not index.
+
+### S6 — Remediate
+
+- **Missing sensors:** show the list; operator checks wiring/taps; one click re-runs
+  S3→S5. Repeats are safe — settings persist in EEPROM and recovered sensors reclaim their
+  slots. Loop as many times as the operator wants.
+- **New sensors:** prompt for names; push via `{B}` (and `{C}`/`{D}`/`{E}`/`{F}`/alarms if the
+  operator supplies calibration). Each write is echoed with the updated `{A}` record
+  (`op05.c:1415-1418`).
+- **Ghosts:** offer one-click `{R}` (remove by index) per ghost entry.
+- **Unrecoverable missing sensor the operator insists on keeping registered:** `{S}` re-adds
+  the serial as a persistent placeholder, then re-push its name/calibration from the snapshot
+  (the `{S}` path resets them — `pointsix.c:377-387`). Display it clearly as **inactive until a
+  future scan finds it on the bus**.
+
+### S7 — Cleanup & report
+
+1. Write `#4 = 0`; confirm echo. (Otherwise the acquire flag and fast-blink LED persist until
+   the next button press — `indicators.c:670-674`.)
+2. **Mode restore decision.** If `original_mode == 1`, ask the operator whether to restore it,
+   with this default and warning:
+    - **Default: leave #3 = 0.** On a wired-only DA-12C, restricted mode adds no security
+      against neighboring RF traffic and makes every future acquire (button *or* tool)
+      destructive (§1.2).
+    - If site policy requires restricted mode, write `#3 = 1` *only after* S5 shows zero
+      missing sensors.
+3. Final `{Y}`; store the post-state next to the S1 snapshot as the **after** record.
+4. Show the closing summary: recovered / new / removed / still-missing counts, mode
+   disposition, and the recovery-file path. Send `{Z}` if disconnecting.
+
+---
+
+## 4. Safety invariants (MUSTs for implementation review)
+
+1. Never write #4 = 1 unless #3 is confirmed 0 **in the same session** (echo-verified, not
+   assumed from a stale read).
+2. Never expose a raw #4 toggle in the UI.
+3. Never trigger a scan before the S1 recovery file is durably written.
+4. Always clear #4 in cleanup, **including on every abort path**.
+5. Always end with a verify pass; never report success from the trigger alone.
+6. Treat `{S}` as a registration placeholder only — never present an `{S}`-added wired sensor
+   as live.
+7. All operator-facing output identifies sensors by **name + serial**; indices are unstable
+   across acquires.
+
+---
+
+## 5. Recovery-file schema
+
+```json
+{
+  "schema": "da12c-acquire-snapshot/1",
+  "taken_at": "2026-06-10T14:03:22Z",
+  "phase": "before",
+  "station": { "mac": "0090C2…", "name": "Cooler 3", "firmware": "4.21" },
+  "settings": { "1": "Cooler 3", "3": 1, "4": 0, "…": "raw values, all 33" },
+  "sensors": [
+    {
+      "index": 1,
+      "serial": "28A4E1020000009B",
+      "type": "0x2801",
+      "name": "Freezer A",
+      "scale": 1.0, "offset": 0.0,
+      "units": 1, "decimals": 1, "stats": 1,
+      "alarm": { "active": 1, "delay": 30, "limits": [-300, -250, 0x8000, 0x8000] }
+    }
+  ]
+}
+```
+
+One file per phase (`before` / `after`), named
+`acquire_<MAC>_<UTC timestamp>_<phase>.json`, kept indefinitely — these are the audit trail.
+
+---
+
+## 6. Open items to verify on hardware
+
+- [ ] Measure real scan duration on a fully-loaded unit (16 sensors, multiple switches) to
+      confirm the 60 s timeout and the `{F}`-gap detection threshold.
+- [ ] Confirm `{F}` emission genuinely pauses during the scan (expected, since the scan blocks
+      the main loop that drives `Op05SendInfo`, but unverified on hardware).
+- [ ] Confirm the `{E04}` echo arrives *before* the scan blocks the loop (echo is sent from the
+      same pass that parses the command — `op05.c:1409`; expected yes).
+- [ ] Characterize ghost-entry behavior end-to-end with a real linked (dual-channel) sensor.
+
+---
+
+## 7. Cross-references
+
+- [DA-12C Service-Tool Protocol reference](service-tool-protocol.md) — wire formats for every
+  command used here (§2 encoding, §4 inbound commands, §5 settings table).
+- [`reference/da12c_status.py`](reference/da12c_status.py) — decoder + command builders.
+- [Doc vs. Code Discrepancies](discrepancies.md) — registry; the restricted-mode acquire trap
+  and the linked-sensor `RemoveSensor` bug originate from the button-command investigation.
+- Firmware: `indicators.c:834-929` (`DoButtonCommand`), `sensors.c:1125-1184`
+  (`SensorsAcquire`/`SwitchesAcquire`), `pointsix.c:668-734` (`GetP6Num` gate),
+  `owi.c:822-1010` (`OwSearch`), `op05.c:1063-1420` (command parser).
--- a/docs/superpowers/plans/2026-06-05-iomodbus-firmware-version-display.md
+++ b/docs/superpowers/plans/2026-06-05-iomodbus-firmware-version-display.md
@@ -0,0 +1,286 @@
+# IOModbus Firmware Version Display Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Show the IOModbus Device Settings "Firmware Version" value as `641D (100.29)` — the stored 4-char hex word plus its human-readable `<hi-byte>.<lo-byte>` decimal form.
+
+**Architecture:** A pure codec (`format_firmware`) converts the raw register word to the display string. The firmware setting cell is tagged once at device-state build time (`is_firmware`), and the controller's read handler substitutes `format_firmware` for the normal datatype decode on that cell. `RegisterCell.value` stays the single source of truth, so the grid (and any downstream consumer) is consistent.
+
+**Tech Stack:** Python 3.11+, PySide6, pytest / pytest-qt. Tests run headless against the in-memory simulator.
+
+---
+
+## File Structure
+
+- `cim_suite/modules/iomodbus/protocol/codecs.py` — add `format_firmware(raw)` pure function.
+- `cim_suite/modules/iomodbus/domain/models.py` — add `FIRMWARE_LABEL` constant, `is_firmware` field on `RegisterCell`, and tag the cell in `DeviceState._build`.
+- `cim_suite/modules/iomodbus/domain/controller.py` — in `_apply_read`, override the firmware cell's value with `format_firmware`.
+- `tests/iomodbus/test_codecs.py` — unit tests for `format_firmware`.
+- `tests/iomodbus/test_controller.py` — integration test against the simulated CS-05.
+
+**Reference facts (verified):**
+- The simulator seeds CS-05 (new) register 0 with the device header version `0x640F` → expected display `640F (100.15)`.
+- The CS-05 (new) firmware row is `Firmware Version~0|R|1|S||` (register 0, read-only). It is read on the first poll sweep because `select_device` sets `_init_all = True`.
+- Existing test helpers in `tests/iomodbus/test_controller.py`: `_wired(catalog)`, `_pump(ctrl, n)`, `_select_cs05(ctrl, catalog)`. The `catalog` fixture is in `tests/iomodbus/conftest.py`.
+
+---
+
+### Task 1: `format_firmware` codec
+
+**Files:**
+- Modify: `cim_suite/modules/iomodbus/protocol/codecs.py`
+- Test: `tests/iomodbus/test_codecs.py`
+
+- [ ] **Step 1: Write the failing test**
+
+Add to the end of `tests/iomodbus/test_codecs.py`:
+
+```python
+# --- firmware version ------------------------------------------------------ #
+
+
+def test_format_firmware():
+    assert c.format_firmware(0x641D) == "641D (100.29)"
+    assert c.format_firmware(0x6405) == "6405 (100.05)"   # minor zero-padded to 2
+    assert c.format_firmware(0x640F) == "640F (100.15)"
+    assert c.format_firmware(0x64FF) == "64FF (100.255)"  # minor > 99 keeps all digits
+    assert c.format_firmware(0x0A01) == "0A01 (10.01)"    # raw zero-padded to 4, upper
+    assert c.format_firmware(0x0000) == "0000 (0.00)"
+
+
+def test_format_firmware_masks_to_16_bits():
+    assert c.format_firmware(0x1641D) == "641D (100.29)"  # high bits ignored
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_codecs.py::test_format_firmware -q`
+Expected: FAIL with `AttributeError: module ... has no attribute 'format_firmware'`
+
+- [ ] **Step 3: Write minimal implementation**
+
+In `cim_suite/modules/iomodbus/protocol/codecs.py`, add this function at the end of the file (after `_to_float`):
+
+```python
+# --------------------------------------------------------------------------- #
+# Firmware version (read-only): one 16-bit word shown as raw hex + hi.lo decimals.
+# The catalog declares this row with inconsistent datatype/dp across devices, so we
+# format it directly from the raw word instead of via decode_value. The high byte is
+# the major version, the low byte the minor (e.g. 0x641D -> "641D (100.29)").
+# --------------------------------------------------------------------------- #
+
+
+def format_firmware(raw: int) -> str:
+    """Firmware register word -> '641D (100.29)': raw hex + hi.lo byte decimals."""
+    raw &= 0xFFFF
+    return f"{raw:04X} ({(raw >> 8) & 0xFF}.{raw & 0xFF:02d})"
+```
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_codecs.py -q`
+Expected: PASS (all codec tests, including the two new ones)
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add cim_suite/modules/iomodbus/protocol/codecs.py tests/iomodbus/test_codecs.py
+git commit -m "feat(iomodbus): add format_firmware codec (641D -> '641D (100.29)')"
+```
+
+---
+
+### Task 2: Tag the firmware cell in the device state
+
+**Files:**
+- Modify: `cim_suite/modules/iomodbus/domain/models.py`
+- Test: `tests/iomodbus/test_controller.py` (a model-level unit test; no controller needed)
+
+- [ ] **Step 1: Write the failing test**
+
+Add to the end of `tests/iomodbus/test_controller.py`:
+
+```python
+def test_device_state_tags_firmware_cell(catalog):
+    from cim_suite.modules.iomodbus.domain.models import DeviceState
+
+    cs05 = next(d for d in catalog.devices if d.description == "CS-05 (new)")
+    state = DeviceState(address=1, dev=cs05)
+    fw_row = next(r for r in state.settings if r.label == "Firmware Version")
+    assert fw_row.cell is not None
+    assert fw_row.cell.is_firmware is True
+    # a different setting must NOT be tagged
+    addr_row = next(r for r in state.settings if r.label == "Address")
+    assert addr_row.cell is not None
+    assert addr_row.cell.is_firmware is False
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_controller.py::test_device_state_tags_firmware_cell -q`
+Expected: FAIL with `AttributeError: 'RegisterCell' object has no attribute 'is_firmware'`
+
+- [ ] **Step 3: Write minimal implementation**
+
+In `cim_suite/modules/iomodbus/domain/models.py`:
+
+(a) Add the constant next to the grid constants (after `GRID_CHANNELS = 1`):
+
+```python
+FIRMWARE_LABEL = "Firmware Version"  # the one read-only row shown as hex + hi.lo decimals
+```
+
+(b) Add the field to `RegisterCell` (after the `force` field):
+
+```python
+    is_firmware: bool = False  # render via codecs.format_firmware, not the datatype decode
+```
+
+(c) In `DeviceState._build`, tag the cell when the setting label matches. Replace the
+settings loop:
+
+```python
+        for r, sdef in enumerate(self.dev.settings):
+            cell = None
+            if sdef.spec is not None:
+                cell = RegisterCell(sdef.spec, GRID_SETTINGS, r, 1)
+                self.cells.append(cell)
+            self.settings.append(SettingRow(sdef.label, cell))
+```
+
+with:
+
+```python
+        for r, sdef in enumerate(self.dev.settings):
+            cell = None
+            if sdef.spec is not None:
+                cell = RegisterCell(sdef.spec, GRID_SETTINGS, r, 1)
+                cell.is_firmware = sdef.label == FIRMWARE_LABEL
+                self.cells.append(cell)
+            self.settings.append(SettingRow(sdef.label, cell))
+```
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_controller.py::test_device_state_tags_firmware_cell -q`
+Expected: PASS
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add cim_suite/modules/iomodbus/domain/models.py tests/iomodbus/test_controller.py
+git commit -m "feat(iomodbus): tag the Firmware Version cell (is_firmware) at build time"
+```
+
+---
+
+### Task 3: Apply the firmware format in the controller
+
+**Files:**
+- Modify: `cim_suite/modules/iomodbus/domain/controller.py:286-299` (`_apply_read`)
+- Test: `tests/iomodbus/test_controller.py`
+
+- [ ] **Step 1: Write the failing test**
+
+Add to the end of `tests/iomodbus/test_controller.py`:
+
+```python
+def test_poll_formats_firmware_version(qtbot, catalog):
+    ctrl, _ = _wired(catalog)
+    _select_cs05(ctrl, catalog)
+    fw_row = next(r for r in ctrl.state.settings if r.label == "Firmware Version")
+    assert fw_row.cell is not None
+    # simulator seeds register 0 with the CS-05 header version 0x640F
+    assert fw_row.cell.value == "640F (100.15)"
+    # a non-firmware setting still decodes normally (regression guard)
+    addr_row = next(r for r in ctrl.state.settings if r.label == "Address")
+    assert addr_row.cell.value not in ("", "640F (100.15)")
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_controller.py::test_poll_formats_firmware_version -q`
+Expected: FAIL — `fw_row.cell.value` is the raw datatype decode (`"640F"`), not `"640F (100.15)"`
+
+- [ ] **Step 3: Write minimal implementation**
+
+In `cim_suite/modules/iomodbus/domain/controller.py`, edit `_apply_read`. Replace:
+
+```python
+        spec = cell.spec
+        value = cdc.decode_value(
+            resp.data, spec.datatype, dp=spec.dp, bits=spec.bits,
+            bstart=spec.bstart, chars=spec.chars, picklist=spec.picklist,
+        )
+        cell.value = value
+        cell.last_raw = cdc.reg_word(resp.data)
+        self.cellUpdated.emit(cell.grid, cell.row, cell.col, value)
+```
+
+with:
+
+```python
+        spec = cell.spec
+        cell.last_raw = cdc.reg_word(resp.data)
+        if cell.is_firmware:
+            value = cdc.format_firmware(cell.last_raw)
+        else:
+            value = cdc.decode_value(
+                resp.data, spec.datatype, dp=spec.dp, bits=spec.bits,
+                bstart=spec.bstart, chars=spec.chars, picklist=spec.picklist,
+            )
+        cell.value = value
+        self.cellUpdated.emit(cell.grid, cell.row, cell.col, value)
+```
+
+(Note: `cell.last_raw` is now computed before the branch so `format_firmware` can use it; the logging block below is unchanged.)
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `.venv\Scripts\python -m pytest tests/iomodbus/test_controller.py -q`
+Expected: PASS (all controller tests, including the new one)
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add cim_suite/modules/iomodbus/domain/controller.py tests/iomodbus/test_controller.py
+git commit -m "feat(iomodbus): show Firmware Version as '641D (100.29)' in Device Settings"
+```
+
+---
+
+### Task 4: Full verification
+
+**Files:** none (verification only)
+
+- [ ] **Step 1: Run the full test suite**
+
+Run: `.venv\Scripts\python -m pytest -q`
+Expected: PASS (no regressions)
+
+- [ ] **Step 2: Run the linter**
+
+Run: `.venv\Scripts\python -m ruff check cim_suite tests`
+Expected: clean (no errors)
+
+- [ ] **Step 3: Smoke-test in the simulator (manual, optional)**
+
+Run: `.venv\Scripts\python -m cim_suite.shell.app --module iomodbus --simulate`
+Then: scan addresses 1–2, select **CS-05 (new)**, open the **Device Settings** tab, and
+confirm the **Firmware Version** row reads `640F (100.15)`.
+
+- [ ] **Step 4: Commit (only if Steps 1–2 required any fixes)**
+
+```bash
+git add -A
+git commit -m "chore(iomodbus): firmware-version display — test/lint fixes"
+```
+
+---
+
+## Notes for the implementer
+
+- This is a **read-only** display field — there is no encode/write path. Do not add one.
+- The firmware word is always a single 16-bit register across every catalog device, so the hi/lo byte split is well-defined.
+- Do not touch `resources/IOModbus.txt` or `regdef.py`; the catalog format is unchanged.
+- Keep `pytest` green and `ruff` clean — both are part of "done" in this repo.