Ev3 new support - #4025
Open
proto-aiken-13 wants to merge 24 commits into
Open
Ev3 new support#4025proto-aiken-13 wants to merge 24 commits into
proto-aiken-13 wants to merge 24 commits into
Conversation
Introduces a new remoteExecutionConductor feature that replaces the js-slang compilation step in the existing EV3 remote execution flow with a conductor-based pipeline using py-slang's EV3Engine. New files: - src/features/remoteExecutionConductor/flagConductorEv3Enable.ts: feature flag and selector to gate the new pipeline - src/features/remoteExecutionConductor/RemoteExecutionConductorActions.ts: conductor-specific redux actions for connect, disconnect, and run - src/features/remoteExecutionConductor/RemoteExecutionConductorSaga.ts: saga handling the conductor EV3 run flow, reusing existing SlingClient connection infrastructure from RemoteExecutionSaga tailored for the Conductor framework - src/features/remoteExecutionConductor/createEv3Conductor.ts: creates a conductor Conduit with a Web Worker loading ev3-pyslang.js, wires receiveResult to forward compiled SVML to the EV3 via SlingClient.sendRun() Modified files: - src/commons/utils/ActionsHelper.ts: registered RemoteExecutionConductorActions - src/commons/sagas/MainSaga.ts: forked RemoteExecutionConductorSaga alongside the legacy RemoteExecutionSaga - src/commons/sagas/WorkspaceSaga/helpers/evalEditorSaga.ts: added flag check to dispatch remoteExecConductorRun vs remoteExecRun based on conductor.ev3.enable feature flag
Added remoteExecutionConductor to support the new Ev3 remote runner under the conductor framework
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a conductor-based EV3 remote execution pipeline with a browser worker evaluator, typed messaging, device-event bridging, Redux actions and sagas, feature-flag routing, and a VS Code references setting. ChangesEV3 conductor execution
Editor configuration
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description summarizes the new Conductor-based EV3 execution feature and selects the new-feature type. The required testing section is blank, and the checklist does not confirm testing or documentation review. 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a Conductor-based EV3 execution pipeline using py-slang, adding the necessary actions, sagas, types, and a web plugin, integrated via a new feature flag. Key feedback points out a critical compilation error caused by an accidental import path replacement in evalEditor.ts, a potential runtime ReferenceError from using Node's Buffer in a browser environment, and a potential stale reference issue when reusing the active conductor across different client sessions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Modified the remote Conductor Tab to support peripheral usage
proto-aiken-13
marked this pull request as ready for review
July 21, 2026 01:57
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/features/remoteExecutionConductor/createEv3Conductor.ts (1)
33-36: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueWrap
client.sendRunagainst synchronous failures.
sendRunis an external device write invoked from an async plugin callback; if it throws, the error is unhandled in this callback. Consider atry/catchthat surfaces the failure viaactions.evalInterpreterError(consistent with theonErrorpath) so a send failure doesn't silently drop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/remoteExecutionConductor/createEv3Conductor.ts` around lines 33 - 36, Update the plugin.onResult callback to wrap client.sendRun in try/catch, and route any synchronous send failure through actions.evalInterpreterError consistently with the existing onError path. Preserve the base64 conversion and normal sendRun behavior when no exception occurs.src/features/remoteExecutionConductor/Ev3WebPlugin.ts (1)
3-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider more descriptive channel/plugin identifiers.
CHANNEL_ID = 'test'andWEB_ID = '__web_test'look like placeholders for what is a production EV3 execution channel. They must stay in sync with the worker (Du.channelAttach = ["test"]), so renaming both sides to something like'ev3'/'__web_ev3'would make the protocol contract clearer without behavior change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/remoteExecutionConductor/Ev3WebPlugin.ts` around lines 3 - 15, Replace the placeholder identifiers CHANNEL_ID and WEB_ID in Ev3WebPlugin with descriptive EV3-specific values, such as an “ev3” channel and matching “__web_ev3” plugin ID. Update the corresponding worker Du.channelAttach value to the same channel identifier, preserving the existing protocol behavior and synchronization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commons/sagas/WorkspaceSaga/helpers/evalEditor.ts`:
- Around line 58-59: Update evalEditor’s feature-flag selection to use the
EV3-specific flagConductorEv3Enable selector instead of flagConductorEnable, and
replace the corresponding import with the selector from the new flag module.
Keep the isConductorEv3 gating logic unchanged.
In `@src/features/remoteExecutionConductor/createEv3Conductor.ts`:
- Around line 46-49: Update the monitor handler in createEv3Conductor to return
when store.getState().session.remoteExecutionSession is absent instead of
asserting non-null, matching the onError and display handlers. Also validate
message[0] and the parsed port before calling substring or updating the session,
ignoring malformed monitor messages safely.
In `@src/features/remoteExecutionConductor/RemoteExecutionConductorSaga.ts`:
- Around line 28-30: Track the client associated with the active conductor and
update handleConductorRun to recreate the conductor when
session.connection.client changes, terminating the existing conduit before
replacement. Set the tracked client after creation, and clear both
activeConductor and the tracked client in handleConductorDisconnect.
---
Nitpick comments:
In `@src/features/remoteExecutionConductor/createEv3Conductor.ts`:
- Around line 33-36: Update the plugin.onResult callback to wrap client.sendRun
in try/catch, and route any synchronous send failure through
actions.evalInterpreterError consistently with the existing onError path.
Preserve the base64 conversion and normal sendRun behavior when no exception
occurs.
In `@src/features/remoteExecutionConductor/Ev3WebPlugin.ts`:
- Around line 3-15: Replace the placeholder identifiers CHANNEL_ID and WEB_ID in
Ev3WebPlugin with descriptive EV3-specific values, such as an “ev3” channel and
matching “__web_ev3” plugin ID. Update the corresponding worker Du.channelAttach
value to the same channel identifier, preserving the existing protocol behavior
and synchronization.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f3f9174b-d90c-4055-8c61-e3b22de5bece
📒 Files selected for processing (11)
.vscode/settings.jsonpublic/evaluators/ev3-remote-runner.jssrc/commons/sagas/MainSaga.tssrc/commons/sagas/WorkspaceSaga/helpers/evalEditor.tssrc/commons/utils/ActionsHelper.tssrc/features/remoteExecutionConductor/Ev3WebPlugin.tssrc/features/remoteExecutionConductor/RemoteExecutionConductorActions.tssrc/features/remoteExecutionConductor/RemoteExecutionConductorSaga.tssrc/features/remoteExecutionConductor/RemoteExecutionTypes.tssrc/features/remoteExecutionConductor/createEv3Conductor.tssrc/features/remoteExecutionConductor/flagConductorEv3Enable.ts
proto-aiken-13
marked this pull request as draft
July 21, 2026 10:00
proto-aiken-13
marked this pull request as ready for review
July 21, 2026 10:00
sayomaki
reviewed
Jul 21, 2026
Addresses review nit: keep saga naming consistent with the other sagas in MainSaga.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U8B3rG53g2Fqvxn7Co1c4G
… flag, not the general conductor.enable flag evalEditor.ts was checking flagConductorEnable (defaults to true) to decide whether a remote-execution run goes through the new Conductor/py-slang pipeline or the legacy js-slang -> SVML -> SlingClient path. Since flagConductorEnable is on by default for unrelated reasons (it gates Conductor evaluation generally, not EV3 specifically), this meant every Source EV3 session would silently be routed into the incomplete Conductor pipeline by default instead of the working legacy one. Switches this check to flagConductorEv3Enable, which already exists in this feature and defaults to false, so Source's legacy EV3 mechanism stays the default until the Conductor pipeline is actually finished end-to-end.
…uard monitor handler against a cleared session RemoteExecutionConductorSaga reused activeConductor across runs without checking whether session.connection.client had changed - reconnecting or switching to a different EV3 device left the conductor wired to a stale client instance. Now tracks the client the active conductor was built against and rebuilds it when that client changes, same as an explicit disconnect already did. createEv3Conductor's monitor handler asserted remoteExecutionSession non-null while its other two handlers (onError, display) both guard it - a monitor event delivered after the session is cleared would crash. Brought it in line with the other two.
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.
Coverage Report for CI Build 35505062719Coverage decreased (-0.2%) to 45.67%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
flagConductorEv3Enable was defined but never added to publicFlags, making it impossible to toggle from the /features UI. evalEditorSaga gates all EV3 device Runs on this flag, defaulting to the legacy js-slang -> SVML -> SlingClient path (compileFiles' module resolution) when it reads as false - which is what was happening here, unreachable.
… worker's CHANNEL_ID was still the placeholder 'test' left over from before the actual py-slang EV3 worker existed. The compiled worker (public/evaluators/ev3-remote-runner.js) registers its plugin on channelAttach=["ev3-execution"] - a real, different channel - so host and worker were never on the same channel. Conductor doesn't error when a send has no subscriber on the other end, so every run silently went nowhere: no error, no output, Run never completes.
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
5 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.
Description
Created a new feature directory remoteExecutionConductor, which serves the same purpose as the remoteExecution feature in a manner that supports the conductor framework
Type of change
How to test
Checklist