From 2a4d19e9c20978e056e59c82d2096f499e0d377a Mon Sep 17 00:00:00 2001 From: David Montero Crespo Date: Wed, 19 Aug 2026 22:19:27 +0200 Subject: [PATCH] =?UTF-8?q?fix(esp32):=20matrix=20keypad=20=E2=80=94=20act?= =?UTF-8?q?ive-row=20model,=20idempotent=20install,=20gap-free=20listeners?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated live against the ESP32 alarm-keypad project. Three fixes over the initial implementation: - Active-row model: the write-pin callback only fires on OUTPUT-latch changes (after the first pass re-driving the row LOW is a no-change write), and dir alone misses the first pass. A matrix scan drives one row at a time with the latch LOW, so 'the row that most recently went active (dir=OUTPUT, or latch write LOW)' IS the scanned row. - Idempotent install: the canvas effect re-attaches on every boards[] mutation (each serial line); reinstalling wiped the pressed-key set mid-scan. Same wiring now keeps the existing state dict. - Gap-free listeners: the keypad's button-press listeners were attached on a 300ms setTimeout per effect run, opening a listener-less window on every re-run in which a press was lost entirely. Attach synchronously and drop the sendSensorDetach cleanup (detach+reattach on unrelated re-renders is exactly the wipe the idempotent install guards against). --- backend/app/services/esp32_worker.py | 74 +++++++++++++------ .../components/simulator/SimulatorCanvas.tsx | 23 +++--- 2 files changed, 63 insertions(+), 34 deletions(-) diff --git a/backend/app/services/esp32_worker.py b/backend/app/services/esp32_worker.py index 18412f46..07088197 100644 --- a/backend/app/services/esp32_worker.py +++ b/backend/app/services/esp32_worker.py @@ -643,17 +643,36 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) # _on_pin_change recompute the column levels synchronously (both run on # the QEMU thread, so qemu_picsimlab_set_pin lands before the firmware's # digitalRead() that follows its digitalWrite(row, LOW)). + # + # Active-row model (validated live against the alarm-keypad project): + # neither the write-pin nor the dir callback alone is reliable across + # passes — the write callback only fires when the OUTPUT LATCH changes + # (after the first pass the latch stays LOW, so re-driving the row is a + # no-change write), and the row's level while INPUT is not modelled. What + # IS delivered every pass is the ENABLE toggle: pinMode(OUTPUT) → dir=1, + # pinMode(INPUT) → dir=0. Since a matrix scan drives exactly one row at a + # time and the latch is LOW during the scan window, "the row whose dir + # went 1 most recently" IS the active row. The write hook stays as a + # belt-and-braces trigger for the first pass (latch 1→0). _keypad_by_row: dict[int, dict] = {} # row gpio → keypad state dict _keypad_cols_owned: set[int] = set() # col gpios driven by the worker def _keypad_install(gpio: int, sensor_data: dict) -> None: rows = [int(r) for r in sensor_data.get('rows', [])] cols = [int(cg) for cg in sensor_data.get('cols', [])] + # Idempotent: the canvas effect re-attaches on unrelated re-renders + # (boards[] mutates with serial output). Same wiring => keep the + # existing state dict — wiping it would drop pressed keys mid-scan. + existing = _keypad_by_row.get(rows[0]) if rows else None + if (existing is not None and existing['rows'] == rows + and existing['cols'] == cols): + sensor_data['keypad'] = existing + return kp = { 'rows': rows, 'cols': cols, 'pressed': set(), # {(row_idx, col_idx)} - 'row_out': [False] * len(rows), # row currently OUTPUT - 'col_level': [1] * len(cols), # last level applied per col + 'active_row': -1, # index of the row being scanned + 'col_level': [1] * len(cols), # last level applied per col } sensor_data['keypad'] = kp for r in rows: @@ -676,15 +695,10 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) lib.qemu_picsimlab_set_pin(cg + 1, 1) def _keypad_recompute(kp: dict) -> None: - rows, cols, pressed = kp['rows'], kp['cols'], kp['pressed'] + cols, pressed = kp['cols'], kp['pressed'] + active = kp.get('active_row', -1) for j, cg in enumerate(cols): - low = any( - kp['row_out'][i] - and _pin_state.get(rows[i], 1) == 0 - and (i, j) in pressed - for i in range(len(rows)) - ) - lvl = 0 if low else 1 + lvl = 0 if (active >= 0 and (active, j) in pressed) else 1 if kp['col_level'][j] != lvl: kp['col_level'][j] = lvl lib.qemu_picsimlab_set_pin(cg + 1, lvl) @@ -887,12 +901,21 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) return gpio = int(_PINMAP[slot]) if 1 <= slot <= _GPIO_COUNT else slot _pin_state[gpio] = value & 1 - # Matrix keypad: a row level changed — refresh column inputs NOW, - # synchronously on the QEMU thread, so the digitalRead()s that follow - # the firmware's digitalWrite(row, LOW) see the pressed key. + # Matrix keypad: a row LATCH changed — only fires on actual latch + # transitions (first pass 1→0), so it complements the per-pass dir + # trigger below rather than replacing it. _kp = _keypad_by_row.get(gpio) if _kp is not None: - _keypad_recompute(_kp) + try: + _idx = _kp['rows'].index(gpio) + except ValueError: + _idx = -1 + if _idx >= 0: + if (value & 1) == 0: + _kp['active_row'] = _idx + elif _kp.get('active_row', -1) == _idx: + _kp['active_row'] = -1 + _keypad_recompute(_kp) # Flush pending SPI bytes BEFORE announcing this pin change so the # frontend processes them under the pin state (e.g. the ILI9341 DC line) # that was in effect when they were sent. With CS events gated off for @@ -1008,19 +1031,22 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) # ── DHT22: track direction changes + trigger sync response ─────── if slot >= 1: gpio = int(_PINMAP[slot]) if slot <= _GPIO_COUNT else slot - # Matrix keypad: rows toggle OUTPUT(scan)/INPUT(idle) every pass. - # Track the direction and refresh the columns synchronously — - # on later passes the row's output latch is already LOW, so the - # pinMode(OUTPUT) transition is the only signal that the row - # became active (the digitalWrite(LOW) that follows is a - # no-change write and fires no pin callback). + # Matrix keypad: rows toggle OUTPUT(scan)/INPUT(idle) every pass, + # and the ENABLE toggle is the one signal delivered EVERY pass + # (the latch write is a no-change after the first pass). dir=1 + # marks this row as the actively scanned one; dir=0 releases it. _kp = _keypad_by_row.get(gpio) - if _kp is not None: + if _kp is not None and direction in (0, 1): try: - _kp['row_out'][_kp['rows'].index(gpio)] = direction == 1 + _idx = _kp['rows'].index(gpio) except ValueError: - pass - _keypad_recompute(_kp) + _idx = -1 + if _idx >= 0: + if direction == 1: + _kp['active_row'] = _idx + elif _kp.get('active_row', -1) == _idx: + _kp['active_row'] = -1 + _keypad_recompute(_kp) with _sensors_lock: sensor = _sensors.get(gpio) if sensor is not None and sensor.get('type') == 'dht22': diff --git a/frontend/src/components/simulator/SimulatorCanvas.tsx b/frontend/src/components/simulator/SimulatorCanvas.tsx index 28622fde..2ff6b6ba 100644 --- a/frontend/src/components/simulator/SimulatorCanvas.tsx +++ b/frontend/src/components/simulator/SimulatorCanvas.tsx @@ -1430,9 +1430,14 @@ export const SimulatorCanvas = ({ headerSlot }: SimulatorCanvasProps = {}) => { kpBridge.sendSensorUpdate(anchorPin, { pressed: [...pressed].map((s) => s.split(',').map(Number)), }); - const timeout = setTimeout(() => { - const el = document.getElementById(component.id); - if (!el) return; + // Attach synchronously — this effect re-runs on every boards[] + // mutation (serial output!), and a delayed attach would open a + // listener-less window every re-run in which a key press is lost + // entirely. No sendSensorDetach on cleanup for the same reason: + // the worker-side install is idempotent, and detach+reattach on + // unrelated re-renders would wipe the pressed-key state mid-scan. + const kpEl = document.getElementById(component.id); + if (kpEl) { const onPress = (e: Event) => { const { row, column } = (e as CustomEvent).detail; pressed.add(`${row},${column}`); @@ -1443,15 +1448,13 @@ export const SimulatorCanvas = ({ headerSlot }: SimulatorCanvasProps = {}) => { pressed.delete(`${row},${column}`); sendPressed(); }; - el.addEventListener('button-press', onPress); - el.addEventListener('button-release', onRelease); + kpEl.addEventListener('button-press', onPress); + kpEl.addEventListener('button-release', onRelease); cleanups.push(() => { - el.removeEventListener('button-press', onPress); - el.removeEventListener('button-release', onRelease); + kpEl.removeEventListener('button-press', onPress); + kpEl.removeEventListener('button-release', onRelease); }); - }, 300); - cleanups.push(() => clearTimeout(timeout)); - cleanups.push(() => kpBridge.sendSensorDetach(anchorPin)); + } } }