EV3 hardware support: port Python->PVML compilation and stdlib - #461
Open
Akshay-2007-1 wants to merge 7 commits into
Open
Akshay-2007-1 wants to merge 7 commits into
Akshay-2007-1 wants to merge 7 commits into
Conversation
Ports the real, substantial EV3 work from Miguel Tarcena's closed, unmerged PRs (#156, #238-241 — all closed by martin-henz with no review comments, most likely stale-branch cleanup rather than a rejection on merit: #241's own base branch, pvml-browser-revival, no longer exists) onto today's main, reconciling against the compiler API drift #241's own description flags. - stdlib/ev3.ts: full ev3_* builtin group (motors, color/ultrasonic/ gyro/touch sensors, LEDs, speak/playSequence) as stub no-ops, ported to the current @Validate-decorated static-class idiom (see misc.ts) with real per-function arities instead of the old draft's placeholder 0/0s. - engines/ev3/{EV3Engine,types,index}.ts: compiles Python to PVML bytecode for on-device transmission. Fixed two real API mismatches the old PR didn't have: PVMLCompiler.fromProgram no longer takes an internalFunctions map (that mechanism doesn't exist in the compiler today - see below), and targetsPynter must be explicitly true or int literals compile to LGCBI, which can't be serialised to PVML's fixed-width binary format at all. - conductor/plugins/Ev3ExecutionPlugin.ts: the conduit-level plugin that owns EV3Engine, matching plugins#54's description of where this logic should live post-split. - conductor/Ev3Evaluator.ts: local/dev evaluator running compiled programs against a native pynter binary. RemoteExecutionPlugin (@sourceacademy/runner-remote-execution, from plugins#54, itself still unmerged) isn't published yet, so this uses a clearly-marked local stub in its place - see the TODO in the file. - tests/ev3-engine.test.ts: compilation smoke tests, plus two tests that pin down a real, honest gap rather than hiding it. Known gap, deliberately not worked around: PVMLCompiler has no device-call annotation/CALLV emission. CALLV/CALLTV exist as reserved opcodes (opcodes.ts, assembler) but nothing in the compiler ever emits them - any reference to an ev3_* name (call or not) fails to compile today with a clean "Primitive function ... not implemented" error rather than producing silently-wrong bytecode. Wiring real CALLV emission, and agreeing its index table with whatever the on-device VM/firmware (ev3-source) expects, is separate follow-up work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add an internalFunctions table (name -> device-function index) threaded through PVMLCompiler.fromProgram/fromFunctionNode. A root-level name found in this table now compiles to CALLV/CALLTV (call) or NEWCV (referenced as a value), the same argument-taken-directly-off-the-stack convention as CALLP/CALLTP/NEWCP, instead of throwing "Primitive function not implemented". stdlib/ev3.ts gains EV3_FUNCTIONS/EV3_INTERNAL_FUNCTIONS, an explicit 0-based index table matching pynter's own devices/ev3/src/ev3_functions.c internals[] array position-for-position. EV3Engine and Ev3Evaluator now pass this table into PVMLCompiler.fromProgram. ev3-engine.test.ts replaces the old "not yet compilable" placeholder tests with real assertions on the compiled entry function's opcode/operand arrays: each ev3_* call resolves to the correct CALLV opcode and device index (e.g. ev3_pause -> 0, ev3_motorA -> 2, ev3_speak -> 34), covering multiple calls in one program and a non-literal call argument.
Ev3ExecutionPlugin (compiles Python -> PVML bytecode on request, then hands the bytecode back over its channel) is a bare IPlugin, not a BasicEvaluator, so it doesn't fit the generic evaluator-worker system (src/conductor/initialise.ts + scripts/build.ts's allTargets) that every other language target uses - it needs its own dedicated worker, same as the browser main-thread side already expects (createEv3Conductor.ts's `new Conduit(worker, true)` in frontend). entry.ts / rollup.ev3-worker.mjs build src/engines/ev3/entry.ts into dist/ev3-remote-runner.js, matching the exact path frontend's EV3_EVALUATOR_PATH loads as a Worker. Unlike the main evaluator bundles this never touches pynter-wasm/WASM loading at all - EV3Engine only compiles bytecode, actual execution happens on the real device via a native pynter-ev3 binary - so it doesn't need rollup.config.mjs's browser shims for that. Verified: builds cleanly, and the resulting bundle loads and executes without throwing when given the two standard Worker globals it needs (MessageChannel/MessagePort) that a real browser Worker provides natively - confirmed via a Node vm sandbox with those two stubbed in, since a real browser wasn't available to test in directly. Wired into `yarn build` via a new build:ev3-worker script, alongside build:repl.
Akshay-2007-1
added a commit
to proto-aiken-13/frontend
that referenced
this pull request
Sep 7, 2026
The committed public/evaluators/ev3-remote-runner.js predated all of py-slang's EV3 work landing (source-academy/py-slang#461) - no CALLV emission, an older stdlib. Rebuilt from py-slang's new src/engines/ev3/entry.ts / rollup.ev3-worker.mjs (`yarn build:ev3-worker`) so the browser actually loads a worker that can compile ev3_* calls. No sourcemap committed, matching the other bundles already in this directory (debug/, pie-slang/) - none of them ship one either.
Akshay-2007-1
marked this pull request as ready for review
September 7, 2026 08:19
Member
|
Yes, these PRs must have been closed automatically when we merged the migration branch into main. I didn't deliberately close the PRs to dismiss them. @Akshay-2007-1 great that you are reviving the work. |
Member
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22719d74c2
ℹ️ 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".
| // arbitrary-precision bigint pool — see PVMLCompiler's `targetsPynter` doc comment. | ||
| const compiler = PVMLCompiler.fromProgram( | ||
| ast, | ||
| 0, |
There was a problem hiding this comment.
For any EV3 program containing a comparison such as distance < 10, variant 0 satisfies variant <= 2 in getCompareOpCode, causing the compiler to emit the §1/§2 opcodes such as LTG12 or EQG12. Those opcodes are above PYNTER_OPCODE_MAX and assemble(program, PYNTER_OPCODE_MAX) rejects them, so ordinary sensor conditions and loops fail to compile before reaching the robot. The native Pynter pathway elsewhere explicitly targets Python §3; pass 3 here and in the matching Ev3Evaluator call.
Useful? React with 👍 / 👎.
Akshay-2007-1
added a commit
to proto-aiken-13/frontend
that referenced
this pull request
Sep 20, 2026
Previous bundle predated py-slang's real hardware-support work landing in full (source-academy/py-slang#461) - stdlib ev3_* functions, the EV3Engine Python->PVML compiler, and the PVML compiler's CALLV/CALLTV emission for ev3_* calls, without which any ev3_* reference failed to compile. Rebuilt from py-slang@b258917 (feature/ev3-hardware-support) via `yarn build:ev3-worker`. The entry point (src/engines/ev3/entry.ts) only registers Ev3ExecutionPlugin - no dependency on plugins#54's RemoteExecutionPlugin (that's only used by Ev3Evaluator.ts, a separate local/CLI dev tool against physical pynter, not this browser worker). Ev3ExecutionPlugin's channel ("ev3-execution") and message shapes match what Ev3WebPlugin.ts already expects, so no frontend-side protocol changes were needed alongside this rebuild.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports the real EV3 hardware-support work from Miguel Tarcena's closed, unmerged PRs (#156, #238, #239, #240, #241) onto current
main, reconciling against the compiler API drift #241's own description flags. All five were closed by martin-henz with no review comments - most likely stale-branch cleanup once #241's base branch (pvml-browser-revival, a temporary rename-migration branch) was deleted, not a rejection on merit.What's here
src/stdlib/ev3.ts(+GroupName.EV3insrc/stdlib/utils.ts): the fullev3_*builtin group - motors, color/ultrasonic/gyro/touch sensors, LEDs,ev3_speak/ev3_playSequence- ported to the current@Validate-decorated static-class idiom, with real per-function arities (the original draft left every function at a placeholderminArgs: 0).src/engines/ev3/{EV3Engine,types,index}.ts: compiles Python source to PVML bytecode for on-device transmission.src/conductor/plugins/Ev3ExecutionPlugin.ts: the conduit-level plugin owningEV3Engine, matching whatsource-academy/plugins#54expects to live here after its own split of transport vs. execution concerns.src/conductor/Ev3Evaluator.ts: a local/dev evaluator that runs compiled programs against a nativepynterbinary (seesource-academy/pynter, which already has a complete, workingdevices/ev3native runtime - motors, sensors, LEDs,ev3_speakviaespeak+aplay, all real implementations, not stubs).src/engines/pvml/pvml-compiler.tsnow actually emitsCALLV/CALLTVforev3_*calls. This was the real remaining gap when this PR was first opened - the compiler had no code path producing these opcodes at all, so anyev3_*reference failed to compile. Fixed by threading aninternalFunctions: ReadonlyMap<string, number>throughfromProgram, resolvingev3_*names against it before falling through to "not implemented", and emittingNEWCV/CALLV/CALLTVvia the existingemitPrimitiveCallpath. The opcode/operand contract (op_call_v = 0x44,op_call_t_v = 0x45, a{id, num_args}operand struct, arguments read straight off the value stack) was confirmed againstpynter's actual VM source (vm/include/pynter/opcode.h,vm.c'sdo_internal_function), not assumed - it turned out to already share the same calling convention asCALLP/CALLTP, just dispatching throughsivmfn_vminternalsinstead ofsivmfn_primitives. The function-index table (EV3_INTERNAL_FUNCTIONS) matchespynter's owninternals[]array order exactly (ev3_pause→0,ev3_motorA→2,ev3_speak→34, etc.) - verified byte-for-byte againstpynter/devices/ev3/src/ev3_functions.c.src/tests/ev3-engine.test.ts: compiles real programs callingev3_*functions and asserts the exact emitted opcode + resolved device index for each, not just "compiles without throwing".Two real API-drift bugs fixed vs. the original closed PR (not just import-path changes):
PVMLCompiler.fromProgramno longer took aninternalFunctionsmap the way #241's version assumed (that parameter is nowuseGlobalMap: boolean, so this table is threaded through separately), andtargetsPyntermust be explicitly passedtrueor integer literals compile to a form (LGCBI) that can't be serialized into PVML's fixed-width binary format at all.Ev3Evaluator.tscurrently stubsRemoteExecutionPluginlocally with a// TODOcomment, since@sourceacademy/runner-remote-execution(fromplugins#54, now green and mergeable) isn't published to npm yet - swap that block for the real import once it is.Status
Typecheck clean, lint clean, full suite passing (3 pre-existing, unrelated failures in
wasm-modules.test.ts, confirmed present onmainindependent of this branch). Still marked draft since it hasn't been exercised against real hardware yet - theev3-sourcedevice image also needs to build and bundlepynter-ev3alongside the existingsinter_host, which this PR doesn't touch.