From 74365d4ae991bb933a2bf995f3cac0311fff8d4a Mon Sep 17 00:00:00 2001 From: David Montero Date: Fri, 12 Jun 2026 23:33:24 +0200 Subject: [PATCH] =?UTF-8?q?feat(cyw43):=20WiFi=20associates=20=E2=80=94=20?= =?UTF-8?q?join=20events=20drive=20link=20up=20(status=20NOIP)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Pico W now joins the virtual AP end to end: active(True) returns, connect() runs the full WPA/SET_SSID sequence, and the link comes up. Root causes fixed (each blocked the join): - mcast_list GET returned empty, so the driver read its own request bytes as the address count (ASCII 'mcas' ~1.9e9) and looped ~2e9 times, hanging wifi_on. GETs now return a zero-filled buffer of the asked-for length (count 0 / status 0), never empty. - Async event frames lacked the 4-byte BDC header the driver expects at SDPCM header_length, so it read the broadcast-MAC byte as data_offset and the payload pointed out of bounds (WRONG_PAYLOAD_TYPE). Prepend BDC. - WLC_E_LINK signalled link-up via the reason field, but the driver checks ev->flags & 1. encodeEventFrame now takes a flags arg; LINK uses flags=1. - Join needs WIFI_JOIN_STATE_KEYED, which only a WLC_E_PSK_SUP(status=6) event sets (connect(ssid, "") still configures the WPA supplicant). Emit it on a successful join. - Join events were raised synchronously during the SET_SSID ioctl, so the driver processed them before cyw43_wifi_join set wifi_join_state=ACTIVE, wiping the bits. Defer events until just after the ioctl reply. - Event-mask stored 4 bytes misaligned vs queueEvent's read offset. - SET/GET kind bit is 0x2 (SDPCM_SET), not 0x1. Remaining for status UP / isconnected: DHCP (needs the packet-transport bridge or an emulator-side DHCP responder). --- ...cow-cyw43-boot-harness.investigate.test.ts | 15 +++-- .../__tests__/picow-cyw43-integration.test.ts | 10 ++-- .../src/simulation/cyw43/Cyw43Emulator.ts | 59 ++++++++++++++++--- frontend/src/simulation/cyw43/constants.ts | 10 ++++ frontend/src/simulation/cyw43/sdpcm.ts | 34 +++++++---- 5 files changed, 100 insertions(+), 28 deletions(-) diff --git a/frontend/src/__tests__/picow-cyw43-boot-harness.investigate.test.ts b/frontend/src/__tests__/picow-cyw43-boot-harness.investigate.test.ts index 2a86c1d8..d6d4b834 100644 --- a/frontend/src/__tests__/picow-cyw43-boot-harness.investigate.test.ts +++ b/frontend/src/__tests__/picow-cyw43-boot-harness.investigate.test.ts @@ -283,14 +283,19 @@ describe.skipIf(!process.env.CYW43_HARNESS)('Pico W cyw43 boot harness (investig // once all TX FIFOs are drained to avoid spinning when idle. if (typeof pio.run === 'function') { pio.run = () => { - for (let i = 0; i < 1_000_000 && !pio.stopped; i++) { + // Cap sized for the largest real transfer (260-word F2 IOCTL write = + // ~16k steps); firmware data is dropped so it never needs deep drain. + // Break out fast once all TX FIFOs are empty (check every 64 steps, + // require a couple of empty checks so the PIO still raises TXSTALL). + let idleChecks = 0; + for (let i = 0; i < 64_000 && !pio.stopped; i++) { pio.step(); - if (i >= 2000 && (i & 1023) === 0) { + if ((i & 63) === 0) { let pending = false; for (const m of pio.machines) { if (m.txFIFO && !m.txFIFO.empty) { pending = true; break; } } - if (!pending) break; + if (pending) { idleChecks = 0; } else if (++idleChecks >= 2) break; } } if (!pio.stopped) pio.runTimer = setTimeout(() => pio.run(), 0); @@ -370,7 +375,7 @@ describe.skipIf(!process.env.CYW43_HARNESS)('Pico W cyw43 boot harness (investig const ipsr = (core?.IPSR ?? 0) & 0x3f; // 0=thread, else exception number ipsrHist.set(ipsr, (ipsrHist.get(ipsr) ?? 0) + 1); }, 20); - const deadline = setTimeout(finish, 70_000); + const deadline = setTimeout(finish, 165_000); function finish() { if (finished) return; finished = true; @@ -444,5 +449,5 @@ describe.skipIf(!process.env.CYW43_HARNESS)('Pico W cyw43 boot harness (investig console.log('\n===== reachedImport=' + result.reachedImport + ' reachedActive=' + result.reachedActive + ' ====='); expect(result.trace.length).toBeGreaterThan(0); - }, 90_000); + }, 185_000); }); diff --git a/frontend/src/__tests__/picow-cyw43-integration.test.ts b/frontend/src/__tests__/picow-cyw43-integration.test.ts index 566b0e7b..d7d24664 100644 --- a/frontend/src/__tests__/picow-cyw43-integration.test.ts +++ b/frontend/src/__tests__/picow-cyw43-integration.test.ts @@ -114,7 +114,9 @@ describe('Cyw43Emulator IOCTL — SET_SSID Velxio-GUEST', () => { const events = pushIoctl(chip, WLC.SET_SSID, payload, 1); expect(chip.getLinkState()).toBe('up'); - expect(events.some((e) => e.type === WLC_E.LINK && e.reason === 1)).toBe(true); + // Link-up is signalled by the LINK event's flags bit 0 (the driver checks + // ev->flags & 1, not the reason), plus a WLC_SUP_KEYED supplicant event. + expect(events.some((e) => e.type === WLC_E.LINK && (e.flags & 1) === 1)).toBe(true); expect(events.some((e) => e.type === WLC_E.SET_SSID && e.status === 0)).toBe(true); }); }); @@ -187,10 +189,10 @@ function pushIoctl( cmd: number, payload: Uint8Array, isSet: number, -): Array<{ type: number; status: number; reason: number; data: Uint8Array }> { +): Array<{ type: number; status: number; reason: number; flags: number; data: Uint8Array }> { const sdpcm = encodeIoctlRequest(0, cmd, isSet, 0, payload); chip.onCommand(makeHdr({ write: true, func: 2, addr: 0, length: sdpcm.length, inc: true }), sdpcm); - const events: Array<{ type: number; status: number; reason: number; data: Uint8Array }> = []; + const events: Array<{ type: number; status: number; reason: number; flags: number; data: Uint8Array }> = []; for (let i = 0; i < 32; i++) { const out = chip.onCommand(makeHdr({ write: false, func: 2, addr: 0, length: 1600, inc: true }), new Uint8Array(0))!; if (!out || out.every((b) => b === 0)) break; @@ -198,7 +200,7 @@ function pushIoctl( if (!f) break; if (f.channel === SdpcmChannel.EVENT) { const ev = decodeEventBody(f.payload); - if (ev) events.push({ type: ev.eventType, status: ev.status, reason: ev.reason, data: ev.data }); + if (ev) events.push({ type: ev.eventType, status: ev.status, reason: ev.reason, flags: ev.flags, data: ev.data }); } else if (f.channel === SdpcmChannel.CONTROL) { decodeCdc(f.payload); // ignore — we just care about events } diff --git a/frontend/src/simulation/cyw43/Cyw43Emulator.ts b/frontend/src/simulation/cyw43/Cyw43Emulator.ts index ad550068..4eb67702 100644 --- a/frontend/src/simulation/cyw43/Cyw43Emulator.ts +++ b/frontend/src/simulation/cyw43/Cyw43Emulator.ts @@ -38,6 +38,8 @@ import { WLC, WLC_E, WLC_E_STATUS, + WLC_E_LINK_UP_FLAG, + WLC_SUP, u32le, } from './constants'; import { @@ -130,6 +132,13 @@ export class Cyw43Emulator { private ap: VirtualAp; private eventMask = new Uint8Array(32); // up to 256 event types private inboundEvents: Uint8Array[] = []; + // Async events raised WHILE handling an IOCTL are deferred until just after + // the IOCTL response is queued, mirroring the real chip: it answers the + // ioctl, THEN emits async events. Critical for SET_SSID — the driver only + // sets wifi_join_state=ACTIVE after cyw43_ll_wifi_join returns, so join + // events delivered before that would be wiped out (link never comes up). + private deferEvents = false; + private pendingEvents: Uint8Array[] = []; // ── Debug counters (investigation harness only) ───────────────── private _dbgF2Writes = 0; // F2 write transfers received @@ -484,6 +493,8 @@ export class Cyw43Emulator { const cdc = decodeCdc(cdcBytes); if (!cdc) { this._dbgIoctlFail++; return; } this._dbgLastIoctl = cdc.cmd; + // Defer any async events the handler raises until after the ioctl reply. + this.deferEvents = true; // SDPCM_SET is 0x2 (bit 1) in the CDC flags; SDPCM_GET is 0. (cyw43_ll.c) const ioctlKind = cdc.flags & 0x2; const isGet = ioctlKind === 0; @@ -589,6 +600,15 @@ export class Cyw43Emulator { payload: replyCdc, }); this.pushFrame(sdpcm); + + // Now release any deferred async events AFTER the ioctl response, so the + // driver reads the reply (do_ioctl returns) before processing them. + this.deferEvents = false; + if (this.pendingEvents.length > 0) { + const evs = this.pendingEvents; + this.pendingEvents = []; + for (const f of evs) this.pushFrame(f); + } } // ── Concrete IOCTL handlers ───────────────────────────────────── @@ -611,7 +631,15 @@ export class Cyw43Emulator { this.queueEvent(WLC_E.ASSOC, WLC_E_STATUS.SUCCESS, 0); this.queueEvent(WLC_E.SET_SSID, WLC_E_STATUS.SUCCESS, 0, encodeSetSsidPayload(ssid)); - this.queueEvent(WLC_E.LINK, WLC_E_STATUS.SUCCESS, 1 /* link up flag */); + // LINK up: the driver checks ev->flags & 1 (NOT the reason) to mark the + // link up, and only sets WIFI_JOIN_STATE_LINK when that bit is set. + this.queueEvent(WLC_E.LINK, WLC_E_STATUS.SUCCESS, 0, new Uint8Array(0), + WLC_E_LINK_UP_FLAG); + // Supplicant "keyed": for a secured join the driver needs + // WIFI_JOIN_STATE_KEYED before it declares the link up (open networks + // preset it, but MicroPython's connect(ssid, "") still configures the + // WPA supplicant). Emit WLC_SUP_KEYED so the 4-bit join state completes. + this.queueEvent(WLC_E.PSK_SUP, WLC_SUP.KEYED, 0); this.linkState = 'up'; this.fireConnect(ssid); } else { @@ -638,16 +666,20 @@ export class Cyw43Emulator { const val = dv.getUint32(4, true); if (mask & 0x1) this.fireLed((val & 0x1) === 0x1); } - // bsscfg:event_msgs payload: 4-byte cfg index + 16-byte mask - if (name === 'bsscfg:event_msgs' && value.length >= 4 + 16) { - const mask = value.slice(4, 4 + 16); + // bsscfg:event_msgs (and plain event_msgs) payload: 4-byte bsscfg index + // followed by the event bitmask. Store the buffer VERBATIM (index included) + // so the bitmask lines up at offset 4 — exactly where queueEvent and + // hasAnyMaskBitsSet read it (eventMask[4 + (eventType>>3)]). Slicing the + // index off here mis-aligned the mask by 4 bytes, so the join events + // (AUTH/LINK/PSK_SUP) were dropped and the link never came up. + if ((name === 'bsscfg:event_msgs' || name === 'event_msgs') && value.length >= 4 + 4) { this.eventMask = new Uint8Array(32); - this.eventMask.set(mask); + this.eventMask.set(value.subarray(0, Math.min(value.length, 32))); } // sup_wpa_psk / wsec_pmk / passphrase — accept silently. } - private handleGetVar(name: string, _outlen: number): Uint8Array { + private handleGetVar(name: string, outlen: number): Uint8Array { if (name === 'cur_etheraddr') { return new Uint8Array(this.staMac); } @@ -655,7 +687,15 @@ export class Cyw43Emulator { // Synthetic firmware version banner; the driver only uses the prefix. return new TextEncoder().encode('velxio-cyw43-emu 1.0\0'); } - return new Uint8Array(0); + // Every other GET must return a ZERO-FILLED buffer of the size the driver + // asked for — NEVER an empty response. cyw43_do_ioctl only memmoves the + // bytes we return over the driver's request buffer, so an empty reply + // leaves the request bytes in place. Several GETs then parse a leading u32 + // as a count/length and run away: e.g. cyw43_ll_wifi_update_multicast_filter + // reads mcast_list's first word as the address count and loops that many + // times (the ASCII "mcas" => ~1.9e9 iterations, which read out of bounds + // and hang wifi_on). Zeros => count 0 / status 0 (success) everywhere. + return new Uint8Array(Math.max(4, Math.min(outlen, 1536))); } // ── Event queueing ───────────────────────────────────────────── @@ -665,6 +705,7 @@ export class Cyw43Emulator { status: number, reason: number, payload: Uint8Array = new Uint8Array(0), + flags = 0, ): void { // Honour the host's event mask if it's been set; events outside the // mask are dropped on the floor (the chip wouldn't deliver them). @@ -684,8 +725,10 @@ export class Cyw43Emulator { reason, payload, this.staMac, + flags, ); - this.pushFrame(frame); + if (this.deferEvents) this.pendingEvents.push(frame); + else this.pushFrame(frame); } private hasAnyMaskBitsSet(): boolean { diff --git a/frontend/src/simulation/cyw43/constants.ts b/frontend/src/simulation/cyw43/constants.ts index d6bf55e3..7d27eef6 100644 --- a/frontend/src/simulation/cyw43/constants.ts +++ b/frontend/src/simulation/cyw43/constants.ts @@ -101,9 +101,19 @@ export const WLC_E = { SCAN_COMPLETE: 26, JOIN_START: 36, ASSOC_START: 38, + PSK_SUP: 46, // WPA supplicant state; status 6 (WLC_SUP_KEYED) => key exchange done ESCAN_RESULT: 69, } as const; +// WLC_E_PSK_SUP status values (the WPA supplicant state machine). The driver +// only marks WIFI_JOIN_STATE_KEYED when it sees status == KEYED. +export const WLC_SUP = { + KEYED: 6, // WLC_SUP_KEYED — 4-way handshake complete, link may come up +} as const; + +// LINK event flag: bit 0 set means "link up" (cyw43_ll.c checks ev->flags & 1). +export const WLC_E_LINK_UP_FLAG = 1; + // ── Status codes used in event payloads ─────────────────────────── export const WLC_E_STATUS = { SUCCESS: 0, diff --git a/frontend/src/simulation/cyw43/sdpcm.ts b/frontend/src/simulation/cyw43/sdpcm.ts index 069e546d..eab4df51 100644 --- a/frontend/src/simulation/cyw43/sdpcm.ts +++ b/frontend/src/simulation/cyw43/sdpcm.ts @@ -141,28 +141,37 @@ export function encodeEventFrame( reason: number, payload: Uint8Array = new Uint8Array(0), srcMac: Uint8Array = new Uint8Array([0, 0, 0, 0, 0, 0]), + flags = 0, ): Uint8Array { // Event SDPCM payload layout used by Broadcom firmware: + // BDC_HDR (4 bytes) flags, priority, interface, data_offset // ETHER_HDR (14 bytes) dest+src+ethertype // BCMETH_HDR (10 bytes) Broadcom OUI tag // EVENT_HDR (48 bytes) event_type, status, … // + // The driver reads the BDC header at the SDPCM header_length, then takes the + // ethernet payload at bdc + 4 + (data_offset<<2). Without the BDC it reads + // the broadcast-MAC byte as data_offset and the payload points out of bounds. const ETHERTYPE_BRCM = 0x886c; + const BDC_LEN = 4; const ETH_LEN = 14; const BCMETH_LEN = 10; const EVENT_LEN = 48; - const total = ETH_LEN + BCMETH_LEN + EVENT_LEN + payload.length; + const total = BDC_LEN + ETH_LEN + BCMETH_LEN + EVENT_LEN + payload.length; const buf = new Uint8Array(total); const dv = new DataView(buf.buffer); + // BDC header: data_offset = 0 (ethernet immediately follows), interface 0. + // buf[0..3] stay zero. + const eth = BDC_LEN; // Ethernet header: dst = broadcast, src = chip MAC, ethertype = BRCM - for (let i = 0; i < 6; i++) buf[i] = 0xff; - buf.set(srcMac, 6); - dv.setUint16(12, ETHERTYPE_BRCM, false); + for (let i = 0; i < 6; i++) buf[eth + i] = 0xff; + buf.set(srcMac, eth + 6); + dv.setUint16(eth + 12, ETHERTYPE_BRCM, false); // Broadcom Ethernet header (subtype/len/ver/oui/usr_subtype) - let off = ETH_LEN; + let off = eth + ETH_LEN; dv.setUint16(off + 0, 0, false); - dv.setUint16(off + 2, total - ETH_LEN, false); + dv.setUint16(off + 2, total - BDC_LEN - ETH_LEN, false); buf[off + 4] = 0x02; // ver buf[off + 5] = 0x00; // oui[0] buf[off + 6] = 0x10; // oui[1] @@ -172,7 +181,7 @@ export function encodeEventFrame( // EVENT_HDR (big-endian — Broadcom firmware writes this BE) off += BCMETH_LEN; dv.setUint16(off + 0, 1, false); // ver - dv.setUint16(off + 2, 0, false); // flags + dv.setUint16(off + 2, flags & 0xffff, false); // flags (bit 0 = link up for WLC_E_LINK) dv.setUint32(off + 4, eventType >>> 0, false); dv.setUint32(off + 8, status >>> 0, false); dv.setUint32(off + 12, reason >>> 0, false); @@ -191,24 +200,27 @@ export function encodeEventFrame( }); } -/** Decode an event payload — returns the event_type / status / reason. */ +/** Decode an event payload — returns the flags / event_type / status / reason. */ export function decodeEventBody(payload: Uint8Array): { + flags: number; eventType: number; status: number; reason: number; datalen: number; data: Uint8Array; } | null { + const BDC_LEN = 4; const ETH_LEN = 14; const BCMETH_LEN = 10; const EVENT_LEN = 48; - if (payload.length < ETH_LEN + BCMETH_LEN + EVENT_LEN) return null; + if (payload.length < BDC_LEN + ETH_LEN + BCMETH_LEN + EVENT_LEN) return null; const dv = new DataView(payload.buffer, payload.byteOffset, payload.byteLength); - const off = ETH_LEN + BCMETH_LEN; + const off = BDC_LEN + ETH_LEN + BCMETH_LEN; + const flags = dv.getUint16(off + 2, false); const eventType = dv.getUint32(off + 4, false); const status = dv.getUint32(off + 8, false); const reason = dv.getUint32(off + 12, false); const datalen = dv.getUint32(off + 20, false); const data = payload.slice(off + EVENT_LEN, off + EVENT_LEN + datalen); - return { eventType, status, reason, datalen, data }; + return { flags, eventType, status, reason, datalen, data }; }