diff --git a/backend/app/services/esp32_worker.py b/backend/app/services/esp32_worker.py index 07088197..deb9c1fc 100644 --- a/backend/app/services/esp32_worker.py +++ b/backend/app/services/esp32_worker.py @@ -40,6 +40,7 @@ import ctypes import json import os import queue +import re import sys import tempfile import threading @@ -172,6 +173,75 @@ def _log(msg: str) -> None: # ESP32 has 40 GPIOs (0-39), ESP32-C3 only has 22 (0-21). # The pinmap is rebuilt after reading config (see main()), defaulting to ESP32. +# QEMU machines whose SoC model instantiates a WiFi MAC. The S3 machine does +# not (hw/xtensa/esp32s3.c never calls qemu_find_nic_info), so it must not be +# handed a NIC — see the radio block in start(). +_WIFI_MACHINES = {'esp32-picsimlab'} + + +# The classic ESP32's WiFi MAC, at its DPORT address and at the APB alias the +# IDF driver actually uses (0x3ff73000 - DR_REG_DPORT_APB_BASE + APB_REG_BASE). +# One page each, matching the region hw/misc/esp32_wifi.c maps. +_WIFI_MAC_RANGES = ((0x3ff73000, 0x3ff74000), (0x60033000, 0x60034000)) + + +def wifi_nic_arg(machine: str, wifi_enabled: bool, + hostfwd_port: int = 0) -> str | None: + """The `-nic` value for this machine, or None if it models no radio. + + The radio is attached whether or not the sketch appears to use it, because + a real ESP32 has one either way. It used to be conditional on wifi_enabled, + which made a source-scanning GUESS load-bearing: the fork only instantiates + the MAC when a NIC is present (`qemu_find_nic_info(TYPE_ESP32_WIFI)` in + hw/xtensa/esp32.c), so a sketch the scanner misread as WiFi-less ran on a + machine with nothing mapped at DR_REG_WIFI_BASE. The firmware's first + register touch then took an unmapped-peripheral fault — `Guru Meditation + Error (LoadStorePIFAddrError)`, EXCVADDR 0x60033c00 — which reads as a + Velxio crash and says nothing about WiFi. That is issue #260, and one bad + guess was all it took. + + Measured before making it unconditional, on a sketch that never touches + WiFi: boot time, guest-clock-vs-real-time and container CPU all unchanged + over 4 paired runs. The AP's beacon timer runs on QEMU_CLOCK_REALTIME — + the reason -icount is off for Xtensa — but idle beacons cost nothing here. + + `wifi_enabled` still gates the host forward, which exposes the GUEST's + server to the host and so belongs to a sketch that actually serves. + """ + if 'c3' in machine: + model = 'esp32c3_wifi' + elif machine in _WIFI_MACHINES: + model = 'esp32_wifi' + else: + # The S3 machine models no radio (hw/xtensa/esp32s3.c never looks for + # the NIC), so handing it one would leave an unconsumed netdev. + return None + arg = f'user,model={model},net=192.168.4.0/24' + if wifi_enabled and hostfwd_port: + arg += f',hostfwd=tcp::{hostfwd_port}-192.168.4.15:80' + return arg + + +def _explain_pif_fault(chunk: bytes) -> str | None: + """Turn a LoadStorePIFAddrError dump into a sentence about what it means. + + The panic reports an address and nothing else, so a user reading it has no + way to know the chip was reaching for a peripheral this machine does not + model. A week of debugging went into issue #260 for exactly that reason. + Returns the line to print once EXCVADDR has been seen, or None if this + chunk does not carry it yet. + """ + m = re.search(rb'EXCVADDR\s*:\s*0x([0-9a-fA-F]+)', chunk) + if not m: + return None + addr = int(m.group(1), 16) + where = ('the WiFi MAC' if any(lo <= addr < hi for lo, hi in _WIFI_MAC_RANGES) + else 'a peripheral') + return (f'\r\n[velxio] That panic is not a bug in your sketch: the firmware ' + f'read {where} at 0x{addr:08x}, which this emulated machine does not ' + f'provide. Nothing is mapped there, so the CPU faulted.\r\n') + + _GPIO_COUNT = 40 _PINMAP = (ctypes.c_int16 * (_GPIO_COUNT + 1))( _GPIO_COUNT, @@ -453,13 +523,14 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) args_list.extend([b'-icount', b'3']) # ── WiFi NIC (slirp user-mode networking) ────────────────────────────── - if wifi_enabled: - nic_model = 'esp32c3_wifi' if 'c3' in machine else 'esp32_wifi' - nic_arg = f'user,model={nic_model},net=192.168.4.0/24' - if wifi_hostfwd_port: - nic_arg += f',hostfwd=tcp::{wifi_hostfwd_port}-192.168.4.15:80' + # Always present on machines that model a radio — see wifi_nic_arg(). + nic_arg = wifi_nic_arg(machine, wifi_enabled, wifi_hostfwd_port) + if nic_arg: args_list.extend([b'-nic', nic_arg.encode()]) - _log(f'WiFi enabled: -nic {nic_arg}') + _log(f'WiFi radio attached (sketch wants WiFi: {wifi_enabled}): -nic {nic_arg}') + elif wifi_enabled: + _log(f'WiFi requested but machine {machine} models no radio — ' + 'the sketch will fault if it touches the MAC') argc = len(args_list) argv = (ctypes.c_char_p * argc)(*args_list) @@ -499,6 +570,12 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) _camera_frame_count = [0] # ESP32-CAM frame trace counter _CRASH_STR = b'Cache disabled but cached memory region accessed' _REBOOT_STR = b'Rebooting...' + # A LoadStorePIFAddrError is always the guest touching a peripheral this + # machine does not model, but the panic dump says only "memory". The + # address arrives a few lines after the cause, so the cause is latched + # here until EXCVADDR shows up. See _explain_pif_fault. + _PIF_STR = b'LoadStorePIFAddrError' + _pif_pending = [False] # ── Signal routing (GPIO Matrix mirror) ─────────────────────────────── # The SignalRouter owns the per-GPIO routing table that the firmware @@ -1103,6 +1180,13 @@ def main() -> None: # noqa: C901 (complexity OK for inline worker) _reboot_count[0] += 1 _emit({'type': 'system', 'event': 'reboot', 'count': _reboot_count[0]}) + if _PIF_STR in chunk: + _pif_pending[0] = True + elif _pif_pending[0]: + note = _explain_pif_fault(chunk) + if note: + _pif_pending[0] = False + _emit({'type': 'serial_output', 'data': note, 'uart': 0}) # WiFi progress logging (only in debug — helps diagnose prod issues) if wifi_enabled: line = chunk.decode('utf-8', errors='replace').strip() diff --git a/backend/app/services/espidf_compiler.py b/backend/app/services/espidf_compiler.py index 2f4f9c8f..175915bc 100644 --- a/backend/app/services/espidf_compiler.py +++ b/backend/app/services/espidf_compiler.py @@ -3740,11 +3740,21 @@ class ESPIDFCompiler: # it inside that branch broke pure ESP-IDF compiles with # UnboundLocalError at the result step (2026-08-05 regression). merged_libs_report: dict[str, str] = {} + _all_text = '\n'.join(f.get('content', '') for f in files) if pure_idf: - _all_text = '\n'.join(f.get('content', '') for f in files) has_wifi = self._detect_idf_wifi_usage(_all_text) else: - has_wifi = self._detect_wifi_usage(main_content) + # Scan the WHOLE project, not just the entry sketch. A header that + # includes WiFi.h is as much a WiFi project as an .ino that does, + # and judging it WiFi-less used to be fatal: the QEMU worker then + # left the radio out of the machine and the firmware's first touch + # of the MAC panicked with LoadStorePIFAddrError (issue #260). + # The pure-IDF branch above always read every file; this one had + # been reading only main_content. + has_wifi = self._detect_wifi_usage(_all_text) + # Normalization still rewrites only the entry sketch: it edits + # string literals in place, and the SSID that matters is the one + # WiFi.begin() is called with. main_content = self._normalize_wifi_for_qemu(main_content) # Arduino-as-component only when compile() decided so: the core must diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index 0c1d5b9a..ca5881e3 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -2043,8 +2043,25 @@ export const useSimulatorStore = create((set, get) => { const bridgeMpyWifi = ( esp32Bridge as { micropythonWifiSupported?: boolean } ).micropythonWifiSupported; - const wifiOn = - (board.hasWifi ?? sketchUsesWifi(files)) && bridgeMpyWifi !== false; + // Does this run get the REAL network driver? Only one thing decides + // it now: whether the backend behind this board models the radio. + // + // It used to also require the sketch to look like a WiFi sketch, + // because without a NIC `network.WLAN(STA_IF)` hangs forever on + // status bits nothing sets, and the stub was the lesser evil. The + // QEMU worker attaches the radio unconditionally since #260 — the + // chip has one whether or not the sketch uses it — so that condition + // no longer describes anything real, and keeping it would put the + // stub in front of a working device whenever the scan misread the + // sketch. That is the #262 failure, from the other direction. + // + // Known gap, not introduced here: QEMU's S3 machine models no radio + // at all, so a MicroPython sketch that calls WLAN() on an S3 through + // the backend waits on a device that is not there. That was already + // true when this was gated on the sketch — such a sketch set hasWifi + // and took the real driver either way. A sketch that never imports + // network is unaffected, which is why this widening is safe. + const wifiOn = bridgeMpyWifi !== false; // WiFi compat shim: replace `network`, `ntptime`, `urequests` // with smart stubs BEFORE user main.py imports them. @@ -2464,17 +2481,18 @@ export const useSimulatorStore = create((set, get) => { } esp32Bridge.setSensors(sensors); - // Use WiFi flag set by the compiler (most reliable — avoids stale file group issues). - // Fall back to scanning the active file group if the flag hasn't been set yet. - let hasWifi = board.hasWifi; - if (hasWifi === undefined) { - const editorState = useEditorStore.getState(); - const rawFiles = editorState.fileGroups[board.activeFileGroupId]; - const boardFiles = rawFiles && rawFiles.length > 0 ? rawFiles : editorState.files; - // Shared with loadMicroPythonProgram's stub decision — the two - // must not disagree about whether a NIC exists. - hasWifi = sketchUsesWifi(boardFiles); - } + // Either signal saying "WiFi" is enough. The compiler's flag comes + // from the build that actually ran, which is why it is consulted at + // all; the source scan reads every file in the group, which the + // compiler's Arduino path did not until #260. Neither is authoritative + // alone, and an OR cannot be talked out of a true by a stale or + // narrower false — which is the direction that used to hurt. + const editorState = useEditorStore.getState(); + const rawFiles = editorState.fileGroups[board.activeFileGroupId]; + const boardFiles = rawFiles && rawFiles.length > 0 ? rawFiles : editorState.files; + // Shared with loadMicroPythonProgram's stub decision so the two + // cannot disagree about whether this sketch wants WiFi. + const hasWifi = (board.hasWifi ?? false) || sketchUsesWifi(boardFiles); esp32Bridge.wifiEnabled = hasWifi; // microSD — if a card is on the canvas, build a FAT16 image (project diff --git a/test/backend/unit/test_esp32_wifi_args.py b/test/backend/unit/test_esp32_wifi_args.py index 82256209..d2a0abf9 100644 --- a/test/backend/unit/test_esp32_wifi_args.py +++ b/test/backend/unit/test_esp32_wifi_args.py @@ -1,97 +1,61 @@ """ -Tests for WiFi NIC argument injection in ESP32 QEMU launch. +Tests for the WiFi NIC argument the ESP32 QEMU launch builds. -Verifies that: - - `-nic user,model=esp32_wifi,...` is added when wifi_enabled=True - - NIC args are absent when wifi_enabled=False - - esp32c3_wifi model is used for C3 boards - - hostfwd port is included when specified +These call the SHIPPED `wifi_nic_arg`. The previous version of this file +re-implemented the logic inline, so it only ever proved that copy +self-consistent — it would have passed unchanged through issue #260, whose +whole content is that this decision was wrong. """ -import json import unittest from unittest.mock import patch, MagicMock +from app.services.esp32_worker import wifi_nic_arg -class TestEsp32WorkerWifiArgs(unittest.TestCase): - """Test the QEMU arg construction in esp32_worker when WiFi is enabled.""" - def _build_config(self, wifi_enabled=False, wifi_hostfwd_port=0, - machine='esp32-picsimlab'): - return json.dumps({ - 'lib_path': '/fake/libqemu-xtensa.so', - 'firmware_b64': 'AAAA', # minimal base64 - 'machine': machine, - 'sensors': [], - 'wifi_enabled': wifi_enabled, - 'wifi_hostfwd_port': wifi_hostfwd_port, - }) +class TestWifiNicArg(unittest.TestCase): + """What the machine gets, and why.""" - def test_wifi_disabled_no_nic_arg(self): - """When wifi_enabled=False, -nic should NOT appear in args.""" - cfg = json.loads(self._build_config(wifi_enabled=False)) - args = self._simulate_args(cfg) - self.assertNotIn(b'-nic', args) + def test_radio_attached_even_when_the_sketch_looks_wifi_less(self): + """The regression #260 is about. - def test_wifi_enabled_adds_nic_arg(self): - """When wifi_enabled=True, -nic should appear with esp32_wifi model.""" - cfg = json.loads(self._build_config(wifi_enabled=True)) - args = self._simulate_args(cfg) - self.assertIn(b'-nic', args) - nic_idx = args.index(b'-nic') - nic_val = args[nic_idx + 1].decode() - self.assertIn('model=esp32_wifi', nic_val) - self.assertIn('net=192.168.4.0/24', nic_val) + A real ESP32 has a radio whether or not the sketch uses it. When the + NIC was conditional, a sketch misread as WiFi-less ran on a machine + with nothing mapped at the MAC, and the firmware's first register + touch panicked with LoadStorePIFAddrError instead of failing to + connect. + """ + arg = wifi_nic_arg('esp32-picsimlab', wifi_enabled=False) + self.assertIsNotNone(arg) + self.assertIn('model=esp32_wifi', arg) + self.assertIn('net=192.168.4.0/24', arg) - def test_wifi_enabled_c3_uses_c3_model(self): - """ESP32-C3 machines should use esp32c3_wifi model.""" - cfg = json.loads(self._build_config( - wifi_enabled=True, machine='esp32c3-picsimlab')) - args = self._simulate_args(cfg) - nic_idx = args.index(b'-nic') - nic_val = args[nic_idx + 1].decode() - self.assertIn('model=esp32c3_wifi', nic_val) + def test_classic_esp32_model(self): + arg = wifi_nic_arg('esp32-picsimlab', wifi_enabled=True) + self.assertIn('model=esp32_wifi', arg) + + def test_c3_uses_its_own_model(self): + arg = wifi_nic_arg('esp32c3-picsimlab', wifi_enabled=True) + self.assertIn('model=esp32c3_wifi', arg) + + def test_s3_gets_no_nic_because_it_models_no_radio(self): + """hw/xtensa/esp32s3.c never looks for the NIC, so handing it one + would leave an unconsumed netdev.""" + self.assertIsNone(wifi_nic_arg('esp32s3-picsimlab', wifi_enabled=True)) + self.assertIsNone(wifi_nic_arg('esp32s3-picsimlab', wifi_enabled=False)) def test_hostfwd_included_when_port_set(self): - """When wifi_hostfwd_port is set, hostfwd should appear in NIC arg.""" - cfg = json.loads(self._build_config( - wifi_enabled=True, wifi_hostfwd_port=12345)) - args = self._simulate_args(cfg) - nic_idx = args.index(b'-nic') - nic_val = args[nic_idx + 1].decode() - self.assertIn('hostfwd=tcp::12345-192.168.4.15:80', nic_val) + arg = wifi_nic_arg('esp32-picsimlab', wifi_enabled=True, hostfwd_port=12345) + self.assertIn('hostfwd=tcp::12345-192.168.4.15:80', arg) def test_hostfwd_absent_when_port_zero(self): - """When wifi_hostfwd_port is 0, hostfwd should NOT appear.""" - cfg = json.loads(self._build_config( - wifi_enabled=True, wifi_hostfwd_port=0)) - args = self._simulate_args(cfg) - nic_idx = args.index(b'-nic') - nic_val = args[nic_idx + 1].decode() - self.assertNotIn('hostfwd', nic_val) + arg = wifi_nic_arg('esp32-picsimlab', wifi_enabled=True, hostfwd_port=0) + self.assertNotIn('hostfwd', arg) - @staticmethod - def _simulate_args(cfg: dict) -> list[bytes]: - """Simulate the arg-building logic from esp32_worker.main().""" - machine = cfg.get('machine', 'esp32-picsimlab') - wifi_enabled = cfg.get('wifi_enabled', False) - wifi_hostfwd_port = cfg.get('wifi_hostfwd_port', 0) - - args_list = [ - b'qemu', - b'-M', machine.encode(), - b'-nographic', - b'-L', b'/fake/rom', - b'-drive', b'file=/tmp/fw.bin,if=mtd,format=raw', - ] - - if wifi_enabled: - nic_model = 'esp32c3_wifi' if 'c3' in machine else 'esp32_wifi' - nic_arg = f'user,model={nic_model},net=192.168.4.0/24' - if wifi_hostfwd_port: - nic_arg += f',hostfwd=tcp::{wifi_hostfwd_port}-192.168.4.15:80' - args_list.extend([b'-nic', nic_arg.encode()]) - - return args_list + def test_hostfwd_needs_a_sketch_that_serves(self): + """The forward exposes the guest's server to the host. The radio is + unconditional; opening a port is not.""" + arg = wifi_nic_arg('esp32-picsimlab', wifi_enabled=False, hostfwd_port=12345) + self.assertNotIn('hostfwd', arg) class TestEspQemuManagerWifiArgs(unittest.TestCase): @@ -134,3 +98,30 @@ class TestSimulationWifiPort(unittest.TestCase): if __name__ == '__main__': unittest.main() + + +class TestExplainPifFault(unittest.TestCase): + """The panic dump says "memory"; the user needs to hear "peripheral".""" + + def test_names_the_wifi_mac_at_the_apb_alias(self): + from app.services.esp32_worker import _explain_pif_fault + note = _explain_pif_fault(b'EXCVADDR: 0x60033c00\n') + self.assertIsNotNone(note) + self.assertIn('WiFi MAC', note) + self.assertIn('0x60033c00', note) + + def test_names_the_wifi_mac_at_the_dport_address(self): + from app.services.esp32_worker import _explain_pif_fault + note = _explain_pif_fault(b'EXCVADDR: 0x3ff73c00\n') + self.assertIn('WiFi MAC', note) + + def test_other_addresses_are_still_explained_generically(self): + from app.services.esp32_worker import _explain_pif_fault + note = _explain_pif_fault(b'EXCVADDR: 0x3ff40000\n') + self.assertIn('a peripheral', note) + self.assertNotIn('WiFi MAC', note) + + def test_waits_for_the_address(self): + """The cause and EXCVADDR arrive in different chunks.""" + from app.services.esp32_worker import _explain_pif_fault + self.assertIsNone(_explain_pif_fault(b'Core 0 register dump:\n')) diff --git a/test/backend/unit/test_wifi_detection_scope.py b/test/backend/unit/test_wifi_detection_scope.py new file mode 100644 index 00000000..aac4cf9a --- /dev/null +++ b/test/backend/unit/test_wifi_detection_scope.py @@ -0,0 +1,55 @@ +""" +Which files count when deciding a sketch uses WiFi (issue #260). + +The Arduino path used to read only the entry sketch while the pure-IDF path +read every file, and that asymmetry was not cosmetic: a "no WiFi" verdict made +the QEMU worker leave the radio out of the machine, so the firmware's first +touch of the MAC panicked with LoadStorePIFAddrError instead of merely failing +to connect. A header that includes WiFi.h is as much a WiFi project as an .ino +that does. +""" +import unittest + +from app.services.espidf_compiler import ESPIDFCompiler + + +class TestWifiDetectionScope(unittest.TestCase): + def setUp(self): + self.c = ESPIDFCompiler.__new__(ESPIDFCompiler) # no toolchain needed + + def _joined(self, files): + """What the compiler scans: every file's content, as one blob.""" + return '\n'.join(f.get('content', '') for f in files) + + def test_include_in_the_entry_sketch(self): + files = [{'name': 'main.ino', 'content': '#include \nvoid setup(){}'}] + self.assertTrue(self.c._detect_wifi_usage(self._joined(files))) + + def test_include_in_a_secondary_header(self): + """The case that reached #260 as a Guru Meditation.""" + files = [ + {'name': 'main.ino', 'content': '#include "net.h"\nvoid setup(){ net_start(); }'}, + {'name': 'net.h', 'content': '#include \ninline void net_start(){ WiFi.begin("x",""); }'}, + ] + self.assertTrue(self.c._detect_wifi_usage(self._joined(files))) + + def test_begin_call_in_a_secondary_source(self): + files = [ + {'name': 'main.ino', 'content': 'void setup(){ net_start(); }'}, + {'name': 'net.cpp', 'content': 'void net_start(){ WiFi.begin("x",""); }'}, + ] + self.assertTrue(self.c._detect_wifi_usage(self._joined(files))) + + def test_a_project_with_no_wifi_anywhere(self): + files = [ + {'name': 'main.ino', 'content': 'void setup(){ Serial.begin(115200); }'}, + {'name': 'helper.h', 'content': 'inline int twice(int n){ return n * 2; }'}, + ] + self.assertFalse(self.c._detect_wifi_usage(self._joined(files))) + + def test_idf_projects_already_read_everything(self): + files = [ + {'name': 'main.c', 'content': '#include "net.h"\nvoid app_main(){}'}, + {'name': 'net.h', 'content': '#include '}, + ] + self.assertTrue(self.c._detect_idf_wifi_usage(self._joined(files)))