feat: add Stack-chan WASM build support - #402
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds WASM build tooling and manifests, many WASM-target stub modules (drivers, services, TTS, hardware shims), and a WASM startup branch that evaluates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Main as Main
participant Trace as Trace
participant DefaultMod as DefaultMod
participant Robot as Robot
Main->>Trace: log("main start")
Main->>Trace: log("install button bridge")
Main->>Trace: log("wifi stage")
Main->>DefaultMod: call onLaunch()
DefaultMod-->>Main: true
Main->>Trace: log("shouldRobotCreate = true")
Main->>Robot: createRobot()
Robot-->>Main: robotInstance
Main->>Trace: log("calling onRobotCreated")
Main->>DefaultMod: call onRobotCreated(robotInstance)
DefaultMod-->>Main: (done)
Main->>Trace: log("onRobotCreated completed")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52e849fa60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async getRotation(): Promise<null> { | ||
| return null |
There was a problem hiding this comment.
Return
Maybe shape from WASM getRotation
Robot.updatePose assumes driver.getRotation() returns an object with a success field and immediately dereferences result.success. In the WASM path, this stub returns null, so the default WASM config (driver.type: "none" in manifest_wasm.json) will hit a runtime error as soon as pose updates run. This prevents normal simulator behavior and should return a valid Maybe<Rotation> object like the non-WASM NoneDriver implementation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@firmware/stackchan/drivers/wasm/driver-stub.ts`:
- Around line 6-8: WasmDriver.getRotation() currently returns ZERO_ROTATION
directly which violates the driver result contract expected by callers to check
result.success; change getRotation to return a result object wrapping the
rotation, e.g. { success: true, rotation: ZERO_ROTATION } (or { success: false,
error: ... } on failure) so call sites that inspect result.success continue to
work; update the getRotation implementation and any related tests to return the
standardized result shape and reference ZERO_ROTATION inside that result.
In `@firmware/stackchan/drivers/wasm/dynamixel-driver.ts`:
- Around line 4-5: DynamixelDriver.getRotation currently returns null which
breaks callers expecting a Maybe object; change the function signature from
Promise<null> to Promise<Maybe<number>> (or the appropriate Maybe<T> for
rotation) and update the implementation to always return a Maybe shaped object
(e.g., { success: false } on error or { success: true, value: rotation } on
success) so callers can safely read result.success; ensure any internal paths
that previously returned null now return the correct Maybe form.
In `@firmware/stackchan/drivers/wasm/none-driver.ts`:
- Around line 4-5: getRotation currently returns null which breaks callers
expecting a Maybe-shaped object; change the implementation of getRotation to
return a Maybe-shaped object (e.g., an object containing the expected success
flag and value fields rather than null) so callers can safely read
result.success and result.value; update the returned Promise from getRotation to
resolve to that object shape (keeping the async signature) and ensure it matches
the project's Maybe type contract used elsewhere.
In `@firmware/stackchan/drivers/wasm/rs30x-driver.ts`:
- Around line 4-5: RS30XDriver.getRotation currently returns Promise<null>,
which breaks callers expecting a success-tagged result; update the method
signature and implementation in RS30XDriver.getRotation to return the expected
result shape (e.g., an object with a success boolean and either a rotation value
or an error message), ensure the Promise resolves to that object instead of
null, and adjust any internal logic to produce success: true with the rotation
data on success or success: false with an error on failure so downstream code
can reliably inspect the success tag.
In `@firmware/stackchan/drivers/wasm/scservo-driver.ts`:
- Around line 4-6: The getRotation() implementation currently returns null;
change it to return the zero-rotation object used by the existing WASM driver
baseline (not null). Update the method signature from Promise<null> to the
correct Rotation type (matching the other WASM driver code) and return the
zero-value rotation object (the same shape/fields the rest of the code expects)
so downstream callers no longer need null guards; adjust any imports/types if
necessary to reference the Rotation type and ensure the returned object matches
its shape.
In `@firmware/stackchan/drivers/wasm/sg90-driver.ts`:
- Around line 4-6: The getRotation method on PWMServoDriver currently returns
Promise<null> which violates the driver contract; change the signature to return
the correct rotation-shaped type (e.g., Promise<Rotation> or Promise<number> to
match other drivers) and return a real rotation value instead of null: implement
PWMServoDriver.getRotation to read the servo's current state (from an existing
field such as this.rotation/this.currentAngle or by converting pulse width /
calling the internal read method like getPulseWidth()/readPWMPosition()) and
return a Rotation-shaped object (or numeric angle) wrapped in a resolved Promise
so the method matches other driver implementations.
In `@firmware/stackchan/main.ts`:
- Around line 175-181: The onLaunch and onRobotCreated hooks are called
synchronously but may return Promises, so await their results to preserve
StackchanMod hook semantics and allow async errors to flow; change the call site
to await onLaunch() when computing shouldRobotCreate (e.g., const
shouldRobotCreate = await onLaunch?.() ?? true) and await
onRobotCreated?.(robot, globalEnv.device) after createRobot() (or make the
surrounding function async) so both hook results are handled correctly and any
thrown/rejected errors are propagated.
In `@firmware/stackchan/manifest_wasm.json`:
- Around line 15-19: The modules["*"] array in manifest_wasm.json is
mis-formatted for Biome; reformat the value of modules["*"] to match the
repository's JSON formatting style (one string per line, properly indented, with
commas placed/omitted as in the rest of the file) so the file passes the Biome
formatter; update the array entries for "./touch", "./robot", and "./main"
accordingly and run the Biome formatter (or commit the exact formatting used
elsewhere in the file) before pushing.
In `@firmware/stackchan/speeches/wasm/tts-stub.ts`:
- Around line 2-3: The constructor and stream method in this WASM TTS stub use
untyped parameters; change their signatures to match the explicit types used in
the other WASM TTS implementations (e.g., tts-voicevox.ts, tts-elevenlabs.ts,
tts-remote.ts, tts-openai.ts, tts-local.ts). Specifically, update
constructor(_options) to constructor(options: /* same options type used by the
other WASM TTS classes */) and update async stream(_text, _volume) to async
stream(text: string, volume?: number): /* same return type as other WASM TTS
stream methods */ so the parameter names, types, and return type match the rest
of the implementations.
In `@firmware/stackchan/wasm/microphone.ts`:
- Line 3: Restore the original optional parameter on the microphone API by
changing the record function signature back to record(durationMilliSec?:
number): Promise<ArrayBuffer> (i.e., accept an optional durationMilliSec
parameter), keep existing internal WASM logic unchanged but consume or ignore
durationMilliSec as appropriate (e.g., pass it into the WASM call or use it to
set a timeout/default), and ensure callers remain compatible with non-WASM code
paths; update the record method declaration in microphone.ts accordingly so
TypeScript call-sites don’t break.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a752a29-2d01-41ed-998e-8b788a080ea3
📒 Files selected for processing (24)
firmware/package.jsonfirmware/stackchan/default-mods/wasm/on-launch.tsfirmware/stackchan/drivers/wasm/driver-stub.tsfirmware/stackchan/drivers/wasm/dynamixel-driver.tsfirmware/stackchan/drivers/wasm/none-driver.tsfirmware/stackchan/drivers/wasm/rs30x-driver.tsfirmware/stackchan/drivers/wasm/scservo-driver.tsfirmware/stackchan/drivers/wasm/sg90-driver.tsfirmware/stackchan/main.tsfirmware/stackchan/manifest_wasm.jsonfirmware/stackchan/renderers-piu/manifest_wasm_renderer_piu.jsonfirmware/stackchan/services/wasm/network-service.tsfirmware/stackchan/services/wasm/preference-server.tsfirmware/stackchan/speeches/wasm/tts-elevenlabs.tsfirmware/stackchan/speeches/wasm/tts-local.tsfirmware/stackchan/speeches/wasm/tts-openai.tsfirmware/stackchan/speeches/wasm/tts-remote.tsfirmware/stackchan/speeches/wasm/tts-stub.tsfirmware/stackchan/speeches/wasm/tts-voicevox-web.tsfirmware/stackchan/speeches/wasm/tts-voicevox.tsfirmware/stackchan/utilities/manifest_wasm_utility.jsonfirmware/stackchan/wasm/led.tsfirmware/stackchan/wasm/microphone.tsfirmware/stackchan/wasm/tone.ts
This reverts commit 04bf80e.
…esolution # Conflicts: # .github/workflows/build.yml # firmware/package.json # firmware/tsconfig.test.json
Summary
Verification
/.local/share/moddable + emsdk 3.1.2 + FONTBM=/.local/bin/fontbm npm run build:wasmStacked follow-up will wire these artifacts into the deployed web page.
Do not merge yet; stacked CI deploy PR will target this branch.
Summary by CodeRabbit
New Features
Chores
Stability