Add testing framework, fix issues - #83
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
- ci: least-privilege permissions, non-persisted checkout credentials, and a pinned cargo-binstall SHA. - ci/qemu: clone rmk once and point both the firmware and build:wasm at it via RMK_REPO, so the two artifacts cannot land on different protocol commits. qemu/run.mjs now resolves rmk the same way build-rynk-wasm.py does. - qemu: the smoke test allocates a free port and hands it to run.mjs instead of sharing a hardcoded 7965. - rynk: probeVersion takes a deadline, so a device that opens the port and never answers surfaces a transport error instead of parking the store in connecting. - store: endSession awaits the teardown and runs it on the failure path too; a not-connected reboot no longer relabels a link-lost phase. - ui: the picker distinguishes dismissal from a real failure and surfaces both it and a failed handshake; StateBar matches phases exhaustively and shows the error cause. - tests: cover the session-ending, storage-reset and BLE commands, the probe deadline, and derive the rynk code list from its source of truth.
|
Addressed the review in 14d0051. All nine points were valid; each is fixed below. Actionable1. 2. This also closes the same hole locally: a sibling 3. Nitpicks
|
This comment was marked as outdated.
This comment was marked as outdated.
|
|
||
| /// Web mode cannot enumerate: `navigator.serial.requestPort()` needs a user | ||
| /// gesture and opens the browser's own picker, so callers use connectWebSerial. | ||
| export function canDiscover(): boolean { |
cobsEncode never closed a full 0xFF block, so `code` overflowed 255 and was truncated by Uint8Array — any payload with a zero-free run longer than 254 bytes encoded to garbage. cobsDecode already handled the 0xFF case. Latent today (the only encode site is the 3-byte GetVersion probe), but the helpers are now exported as protocol primitives for tests. connectClient takes an optional wasm source: the browser default resolves the module next to the JS glue, which Node fetch cannot do for a file: URL.
A dead link used to leave the store looking connected: the topic loop swallowed
its exception and nothing called resetStore, so the UI kept an editable keymap
until the next write failed. Both death paths — a parked next_topic() rejecting
and any request returning a transport error — now converge on teardown, which
clears device/config/status and reports { phase: error, cause }.
teardown takes fromTopicLoop because awaiting session.topicLoop from inside that
loop deadlocks on its own promise.
Also fixes connecting to a locked device: GetMatrixState is unlock-gated
upstream, and fetchStatus asked for it unconditionally, failing the whole
handshake. It is now skipped while locked and read back after unlockPoll.
New commands: refreshStatus, refreshLockStatus, lock, unlockPoll,
refreshBleStatus, switchBleProfile, clearBleProfile, reboot, bootloaderJump,
storageReset. Reboot and bootloader jump are fire-and-forget upstream, so both
the ack and a transport failure end the session.
KeyboardError missed the Busy variant; RYNK_ERROR_CODES is now a Record so a new
upstream variant fails the build.
web.ts was dead code: index.ts imported WebByteLink with `import type`, so it never existed at runtime, and discover() returns [] outside Tauri. Web mode could not reach a device at all. connectWebSerial/connectWebHid are now real exports, and canDiscover() names the Tauri-only enumeration so App can offer a click-driven picker instead — the browser requires a user gesture for requestPort().
CI only ran lint and build, and the qemu fixture had no consumer. 26 unit tests cover the hand-rolled wire code (COBS round-trips including runs past 254 bytes, chunk reassembly, interleaved topic pushes), the rynk error mapping, and the store: optimistic rollback, request serialization, teardown on link death, connecting while locked, and the unlock ceremony. 6 smoke tests drive the real riscv fixture firmware through the real rynk-wasm client over the UART-to-TCP bridge. They live behind pnpm test:qemu because they need qemu-system-riscv32 and a riscv target, and run as a Linux-only CI job. deploy now waits on it — a protocol regression should not ship. CI also runs pnpm check, which was never wired up.
The npm-scripts block is a fragment of object properties, not a JSON document, so the jsonc parser eslint runs over fenced blocks failed on the first colon. Retagged as text — wrapping it in braces parses but then trips style/no-multi-spaces on the aligned values, and --fix would drop the alignment.
Every command in the Development section was inverted — web:dev/tauri:dev/ web:build/tauri:build, none of which exist; the scripts are dev:web, dev:tauri, build:web, build:tauri. Following the README got you four failures in a row. It also omitted build:wasm entirely. src/rynk/wasm/ is gitignored, so a fresh clone has no protocol client and dev:web fails on the first import. Notes that dev:tauri and build:tauri run their web counterparts through tauri.conf beforeDevCommand/beforeBuildCommand, so starting both duplicates the vite server. Adds a Testing section. test:qemu spawns the fixture itself, so it conflicts with a hand-started pnpm qemu — the serial port takes one client. Also flags CI=true for lint: the eslint config relaxes rules when it detects an editor.
The header, features and acknowledgements still described Nuxt and PrimeVue. Neither is a dependency — nor is Vue at all. The UI is Svelte 5 with Tailwind 4 and Ark UI. Also fixes the icon alt text (left over from Clash), an unclosed <h3>, and a missing period in Install.
- ci: least-privilege permissions, non-persisted checkout credentials, and a pinned cargo-binstall SHA. - ci/qemu: clone rmk once and point both the firmware and build:wasm at it via RMK_REPO, so the two artifacts cannot land on different protocol commits. qemu/run.mjs now resolves rmk the same way build-rynk-wasm.py does. - qemu: the smoke test allocates a free port and hands it to run.mjs instead of sharing a hardcoded 7965. - rynk: probeVersion takes a deadline, so a device that opens the port and never answers surfaces a transport error instead of parking the store in connecting. - store: endSession awaits the teardown and runs it on the failure path too; a not-connected reboot no longer relabels a link-lost phase. - ui: the picker distinguishes dismissal from a real failure and surfaces both it and a failed handshake; StateBar matches phases exhaustively and shows the error cause. - tests: cover the session-ending, storage-reset and BLE commands, the probe deadline, and derive the rynk code list from its source of truth.
Web Serial's getInfo() exposes no string descriptors, so connectWebSerial can only offer the constant 'WebSerial' and the status bar showed that instead of the device. The handshake already reads product_name via get_device_info; use it for the connected phase and keep the transport label as the fallback and for the pre-handshake phases.
qemu binds the serial socket during machine init, so the harness could connect and send GET_VERSION before the guest booted — and Uart::new() resets the 16550 FIFO, discarding it. The fixture never answered and the probe burned its whole deadline; CI failed this way once out of several runs. The fixture now prints a marker once its UART is up and the harness waits for it before dialling. Kept out of probeVersion on purpose: a real keyboard is long past boot by the time a host connects. Also hold run.mjs's output and dump it when the handshake fails — the previous run was undiagnosable because cargo and semihosting output went to /dev/null.
fc98c56 to
5c912a0
Compare
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation