feat: built-in AVR ATmega2560 simulator#623
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (17)
WalkthroughAdds a built-in ATmega2560 simulator (avr8js) and threads a 'simulator' runtime through compiler, IPC, Modbus, and UI layers; introduces SimulatorModule, VirtualSerialPort, simulator-aware compile/upload flow, and UI/IPC controls to run and manage simulated firmware. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Renderer (UI)
participant IPC as Main Process (IPC Bridge)
participant Compiler as Compiler Module
participant Simulator as Simulator Module
participant VPort as VirtualSerialPort
participant RTU as ModbusRtuClient
UI->>IPC: runCompileProgram(boardRuntime='simulator')
IPC->>Compiler: compileProgram(boardRuntime='simulator')
Compiler->>Compiler: generate defines (SIMULATOR_MODE, MBSERIAL_*)
Compiler-->>IPC: return { hexPath, simulatorFirmwarePath }
IPC-->>UI: compile complete + simulatorFirmwarePath
UI->>IPC: simulatorLoadFirmware(simulatorFirmwarePath)
IPC->>Simulator: loadAndRun(hexPath)
Simulator->>Simulator: parse HEX, init CPU/SRAM, start loop
Simulator-->>IPC: running
UI->>IPC: debuggerConnect(connectionType='simulator')
IPC->>Simulator: create VirtualSerialPort
IPC->>RTU: new ModbusRtuClient({ serialPort: VPort })
RTU->>VPort: open() / write()
VPort->>Simulator: feedByte(byte)
Simulator->>VPort: emit UART TX bytes
RTU-->>UI: Modbus response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
![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: 9
🧹 Nitpick comments (5)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx (2)
319-343: Floating promise in synchronous callback.The
window.bridge.simulatorLoadFirmware(...)call chains.then().catch()inside a synchronous callback. While the error handling is present, the promise is technically floating. Prefixing withvoidmakes the intent explicit and satisfies linting rules.♻️ Prefix with `void`
// Load firmware into simulator when compilation finishes with a HEX path if (data.simulatorFirmwarePath) { - ;(window.bridge.simulatorLoadFirmware as (p: string) => Promise<{ success: boolean; error?: string }>)( + void (window.bridge.simulatorLoadFirmware as (p: string) => Promise<{ success: boolean; error?: string }>)( data.simulatorFirmwarePath, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx` around lines 319 - 343, The promise returned by window.bridge.simulatorLoadFirmware in the callback is floating; prefix the call with void to make the intent explicit and satisfy linting (e.g., change the invocation inside the compile-finish handler to "void window.bridge.simulatorLoadFirmware(...).then(...).catch(...)" while keeping the existing .then/.catch logic that updates setSimulatorRunning and addLog and handles errors).
321-323: Remove unnecessary type assertions onwindow.bridgesimulator methods.The preload bridge in
ipc/renderer.tsalready includes proper type definitions forsimulatorLoadFirmware,simulatorStop, andsimulatorIsRunning(lines 364–367). The explicitascasts at lines 321, 412, and 467 are redundant and should be removed.Refactoring
Line 321:
- ;(window.bridge.simulatorLoadFirmware as (p: string) => Promise<{ success: boolean; error?: string }>)( - data.simulatorFirmwarePath, - ) + void window.bridge.simulatorLoadFirmware(data.simulatorFirmwarePath)Line 412:
- await (window.bridge.simulatorStop as () => Promise<{ success: boolean }>)() + await window.bridge.simulatorStop()Line 467:
- const running = await (window.bridge.simulatorIsRunning as () => Promise<boolean>)() + const running = await window.bridge.simulatorIsRunning()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx` around lines 321 - 323, Remove the redundant type assertions on the preload bridge calls: delete the explicit "as ..." casts on window.bridge.simulatorLoadFirmware, window.bridge.simulatorStop, and window.bridge.simulatorIsRunning in default.tsx so the calls use the types already declared in ipc/renderer.ts; locate the call sites where simulatorLoadFirmware(...) (around the current call using data.simulatorFirmwarePath), simulatorStop(...) and simulatorIsRunning(...) are cast and simply call them without the "as (…)" type assertions so the compiler picks up the existing bridge definitions.src/renderer/components/_features/[workspace]/editor/device/configuration/board.tsx (1)
303-315: Consider extractingisSimulatorTarget(currentBoardInfo)to a localconst.The function is called 5+ times in the render body (lines 303, 346, 366, 474, 498, 501). The
communication.tsxfile already usesconst isSimulator = isSimulatorTarget(currentBoardInfo)— applying the same pattern here would improve readability and consistency.♻️ Extract to a local constant (like communication.tsx)
const currentBoardInfo = availableBoards.get(deviceBoard) + const isSimulator = isSimulatorTarget(currentBoardInfo)Then replace all
isSimulatorTarget(currentBoardInfo)occurrences in the render withisSimulator.Also applies to: 366-372, 474-474, 498-501
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_features/`[workspace]/editor/device/configuration/board.tsx around lines 303 - 315, Introduce a local const (e.g., const isSimulator = isSimulatorTarget(currentBoardInfo)) near the top of the component render/body and replace every inline call to isSimulatorTarget(currentBoardInfo) with isSimulator (affecting all occurrences inside this file); ensure currentBoardInfo and isSimulatorTarget are still imported/available and keep existing handlers (e.g., handleCompileOnly) and conditional logic unchanged—this improves readability and matches the pattern used in communication.tsx.src/main/modules/compiler/compiler-module.ts (2)
686-698:boardRuntimetyped asstringinstead of a literal union.Using a wide
stringtype loses exhaustiveness checking when new runtime values are added. The existing codebase already uses the discriminator values'simulator','openplc-compiler', and implicitly'arduino'.♻️ Proposed fix
- boardRuntime: string + boardRuntime: 'simulator' | 'openplc-compiler' | stringOr, if a shared literal type already exists in
compiler-types.ts, use that directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/modules/compiler/compiler-module.ts` around lines 686 - 698, The handleGenerateDefinitionsFile method declares parameter boardRuntime as a plain string which prevents exhaustiveness checking; change the boardRuntime parameter type in the handleGenerateDefinitionsFile signature to the specific literal union (e.g. 'simulator' | 'openplc-compiler' | 'arduino') or, if a shared type exists in compiler-types.ts, import and use that named type instead so switch/conditional logic over boardRuntime remains type-safe and will error when new runtimes are added.
1018-1024: Redundant identity.map()before.join().
boardHalsContent['ld_flags'].map((f: string) => f)is a no-op that just copies the array. Same pattern exists forc_flagsandcxx_flagson lines 1006 and 1014.♻️ Proposed simplification
- `compiler.c.elf.extra_flags=${boardHalsContent['ld_flags'].map((f: string) => f).join(' ')}`, + `compiler.c.elf.extra_flags=${boardHalsContent['ld_flags'].join(' ')}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/modules/compiler/compiler-module.ts` around lines 1018 - 1024, Remove the redundant identity .map(...) calls before .join() when building flags from boardHalsContent; directly join the arrays for ld_flags, c_flags and cxx_flags instead. Locate the block where buildProjectFlags is extended (references: buildProjectFlags and boardHalsContent['ld_flags']) and similarly the earlier blocks handling boardHalsContent['c_flags'] and boardHalsContent['cxx_flags'], and replace constructions like boardHalsContent['...'].map((f:string)=>f).join(' ') with boardHalsContent['...'].join(' ') so the arrays are used directly and the unnecessary map is removed. Ensure behavior and string interpolation (e.g., `compiler.c.elf.extra_flags=...`) remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/simulator-rp2040-plan.md`:
- Line 100: Add language identifiers to the fenced code blocks that currently
start and end with triple backticks (``` ... ```) so they satisfy markdownlint
MD040; update the blocks at the occurrences shown (the fenced blocks around
lines with ``` and the later block spanning the code sample) to include an
appropriate language tag such as ```tsx or ```bash depending on the sample
content, ensuring each opening fence includes the language token immediately
after the backticks.
- Around line 1-600: This doc currently documents an rp2040js-based design
(references: rp2040js, rp2040:rp2040:rpipico, rp2040pico.cpp, loadUF2,
firmware.uf2, bootrom.ts, mcu.uart[0].feedByte) but the project actually
implements an avr8js/ATmega2560 simulator; update the file to reflect the real
implementation by either (a) renaming the title and all occurrences of
RP2040/rp2040js/rpipico/UF2/bootrom to avr8js/ATmega2560/Intel HEX and adjust
the architecture-specific sections (build/upload flow, emulator module, virtual
serial port behavior, Modbus UART wiring, and asset names) to match the avr8js
design, or (b) clearly mark the document as historical/prototype (prepend a bold
“Historical: RP2040 prototype — superseded by avr8js/ATmega2560 implementation”
notice at the top) and add a short summary pointing to the current avr8js
design; ensure mentions of functions/classes (SimulatorModule,
VirtualSerialPort, ModbusRtuClient) remain accurate for the current
implementation or are removed if not applicable.
In `@src/main/modules/compiler/compiler-module.ts`:
- Around line 2088-2101: The simulator branch in compileProgram posts messages
but never closes the MessagePortMain (_mainProcessPort), leaving the port open;
fix it by calling _mainProcessPort.close() after sending the two postMessage
calls (or immediately before the return) in the boardRuntime === 'simulator'
branch so the main-process port is properly closed while still sending the
simulatorFirmwarePath (hexPath) payload; locate the simulator branch around
compileProgram where hexPath is defined and add the close call consistent with
other early-return branches.
- Line 2091: Replace the hardcoded "arduino.avr.mega" segment used to build
hexPath with a derived platform string from boardHalsContent['platform']
(transforming any ':' to '.'), so hexPath is constructed using
join(compilationPath, 'examples', 'Baremetal', 'build', derivedPlatform,
'Baremetal.ino.hex'); update the code that produces simulatorFirmwarePath to use
this same derivedPlatform variable; ensure the variable name (e.g.,
derivedPlatform or platformDir) is used consistently where hexPath and
simulatorFirmwarePath are formed so changes to boardHalsContent['platform'] are
reflected automatically.
In `@src/main/modules/ipc/main.ts`:
- Around line 894-911: Before creating a new ModbusRtuClient for the simulator
branch, check if this.debuggerModbusClient is non-null and if so call its
disconnect() (or await disconnect() if async) to tear down the prior session and
clear any callbacks; then assign the new client to this.debuggerModbusClient and
set this.debuggerConnectionType = 'simulator' as you already do. Locate the
simulator branch where VirtualSerialPort is constructed and ModbusRtuClient
instantiated, add the safe-disconnect of the existing this.debuggerModbusClient
before creating/assigning the new ModbusRtuClient to avoid leaving the previous
VirtualSerialPort onUartByte handler dangling.
- Around line 1357-1367: Validate the incoming hexPath in
handleSimulatorLoadFirmware before calling simulatorModule.loadAndRun: ensure
hexPath has a .hex extension, resolve and normalize it (e.g. via
path.resolve/path.normalize) and verify the resolved path is inside the expected
project build directory (compare with a resolved build dir like
this.projectBuildDir or a getProjectBuildDir() result using startsWith or
equivalent), and check the file exists and is a regular file (fs.stat). If any
check fails, return { success: false, error: '...' } rather than calling
simulatorModule.loadAndRun; only call simulatorModule.loadAndRun(hexPath) when
validation passes.
In `@src/main/modules/modbus/modbus-rtu-client.ts`:
- Around line 12-13: Replace the loose serialPort?: any with a concrete
interface describing the port contract used by ModbusRtuClient: declare an
interface (e.g., SerialPortLike) that includes on(event: 'open'|'error'|'data',
cb), once(event: 'error', cb), open(): Promise<void> | void, close():
Promise<void> | void, write(data: Buffer | string, cb?: (err?: Error) => void):
void, flush?(cb?: (err?: Error) => void): void, and an isOpen boolean property;
then change the property signature serialPort?: any to serialPort?:
SerialPortLike and ensure VirtualSerialPort implements this interface (or add a
type assertion) so all calls to .on, .once, .open, .close, .write, .flush, and
.isOpen are type-checked.
In `@src/main/modules/simulator/simulator-module.ts`:
- Around line 259-262: The code directly reads the private field nextClockEvent
via a double-cast in the SLEEP fast-forward logic; replace that private API
access by using avr8js public APIs instead: either fast-forward by repeatedly
calling cpu.tick() until the desired cycles are reached or, if you need to
schedule/inspect future events, track events when you call
cpu.addClockEvent(callback, cycles) and use cpu.updateClockEvent /
cpu.clearClockEvent to adjust them rather than reading nextClockEvent; update
the SLEEP handling code (the block that currently reads nextClockEvent and sets
cpu.cycles) to rely on cpu.tick() or your own event-tracking data structure and
remove the cast-based access.
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Line 88: Local simulatorRunning state in the component can become stale; on
mount and before toggling the simulator you should query the main process via
the same IPC used elsewhere (simulatorIsRunning) and update simulatorRunning
using setSimulatorRunning. Add a useEffect that invokes
ipcRenderer.invoke('simulatorIsRunning') on component mount to seed state, and
modify the play/stop button handler (the toggle handler that currently reads
simulatorRunning) to re-query ipcRenderer.invoke('simulatorIsRunning')
immediately before deciding to start/stop so the action and UI reflect the true
current state. Ensure any other places that read simulatorRunning (e.g., the
debugger path logic) are consistent by updating state from the IPC result.
---
Nitpick comments:
In `@src/main/modules/compiler/compiler-module.ts`:
- Around line 686-698: The handleGenerateDefinitionsFile method declares
parameter boardRuntime as a plain string which prevents exhaustiveness checking;
change the boardRuntime parameter type in the handleGenerateDefinitionsFile
signature to the specific literal union (e.g. 'simulator' | 'openplc-compiler' |
'arduino') or, if a shared type exists in compiler-types.ts, import and use that
named type instead so switch/conditional logic over boardRuntime remains
type-safe and will error when new runtimes are added.
- Around line 1018-1024: Remove the redundant identity .map(...) calls before
.join() when building flags from boardHalsContent; directly join the arrays for
ld_flags, c_flags and cxx_flags instead. Locate the block where
buildProjectFlags is extended (references: buildProjectFlags and
boardHalsContent['ld_flags']) and similarly the earlier blocks handling
boardHalsContent['c_flags'] and boardHalsContent['cxx_flags'], and replace
constructions like boardHalsContent['...'].map((f:string)=>f).join(' ') with
boardHalsContent['...'].join(' ') so the arrays are used directly and the
unnecessary map is removed. Ensure behavior and string interpolation (e.g.,
`compiler.c.elf.extra_flags=...`) remain unchanged.
In
`@src/renderer/components/_features/`[workspace]/editor/device/configuration/board.tsx:
- Around line 303-315: Introduce a local const (e.g., const isSimulator =
isSimulatorTarget(currentBoardInfo)) near the top of the component render/body
and replace every inline call to isSimulatorTarget(currentBoardInfo) with
isSimulator (affecting all occurrences inside this file); ensure
currentBoardInfo and isSimulatorTarget are still imported/available and keep
existing handlers (e.g., handleCompileOnly) and conditional logic unchanged—this
improves readability and matches the pattern used in communication.tsx.
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 319-343: The promise returned by
window.bridge.simulatorLoadFirmware in the callback is floating; prefix the call
with void to make the intent explicit and satisfy linting (e.g., change the
invocation inside the compile-finish handler to "void
window.bridge.simulatorLoadFirmware(...).then(...).catch(...)" while keeping the
existing .then/.catch logic that updates setSimulatorRunning and addLog and
handles errors).
- Around line 321-323: Remove the redundant type assertions on the preload
bridge calls: delete the explicit "as ..." casts on
window.bridge.simulatorLoadFirmware, window.bridge.simulatorStop, and
window.bridge.simulatorIsRunning in default.tsx so the calls use the types
already declared in ipc/renderer.ts; locate the call sites where
simulatorLoadFirmware(...) (around the current call using
data.simulatorFirmwarePath), simulatorStop(...) and simulatorIsRunning(...) are
cast and simply call them without the "as (…)" type assertions so the compiler
picks up the existing bridge definitions.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsonresources/sources/Baremetal/Baremetal.inois excluded by!resources/**resources/sources/boards/hals.jsonis excluded by!resources/**resources/sources/boards/previews/simulator.pngis excluded by!**/*.png,!resources/**
📒 Files selected for processing (18)
docs/simulator-rp2040-plan.mdpackage.jsonsrc/main/modules/compiler/compiler-module.tssrc/main/modules/compiler/compiler-types.tssrc/main/modules/hardware/hardware-module.tssrc/main/modules/hardware/hardware-types.tssrc/main/modules/ipc/main.tssrc/main/modules/ipc/renderer.tssrc/main/modules/modbus/modbus-rtu-client.tssrc/main/modules/simulator/simulator-module.tssrc/main/modules/simulator/virtual-serial-port.tssrc/renderer/components/_features/[workspace]/editor/device/configuration/board.tsxsrc/renderer/components/_features/[workspace]/editor/device/configuration/communication.tsxsrc/renderer/components/_organisms/workspace-activity-bar/default.tsxsrc/renderer/screens/workspace-screen.tsxsrc/renderer/store/slices/device/data/constants.tssrc/renderer/store/slices/device/types.tssrc/utils/device.ts
Comment on lines
+1
to
+600
| # Simulator Mode: rp2040js Integration Plan | ||
|
|
||
| ## Overview | ||
|
|
||
| Add a built-in simulator to OpenPLC Editor using [Wokwi's rp2040js](https://github.com/wokwi/rp2040js) (MIT, zero dependencies, ~200KB). The simulator appears as a device in the board list. When selected, "Build" compiles for RP2040, loads the UF2 firmware into the emulated CPU, and starts execution. "Debugger" connects via Modbus RTU over the emulated UART — identical to real hardware. | ||
|
|
||
| The user never sees any emulation details. They press Build, the code compiles and runs. They press Debugger, values appear. | ||
|
|
||
| --- | ||
|
|
||
| ## 1. New Device Entry in hals.json | ||
|
|
||
| **File:** `resources/sources/boards/hals.json` | ||
|
|
||
| Add a new entry at the **top** of the JSON (so it appears first in the device dropdown): | ||
|
|
||
| ```json | ||
| { | ||
| "OpenPLC Simulator": { | ||
| "compiler": "simulator", | ||
| "core": "rp2040:rp2040", | ||
| "board_manager_url": "https://github.com/earlephilhower/arduino-pico/releases/download/global/package_rp2040_index.json", | ||
| "c_flags": ["-MMD", "-c", "-Wno-incompatible-pointer-types"], | ||
| "extra_libraries": [], | ||
| "platform": "rp2040:rp2040:rpipico", | ||
| "source": "rp2040pico.cpp", | ||
| "preview": "simulator.png", | ||
| "specs": { | ||
| "CPU": "Emulated RP2040 ARM Cortex-M0+", | ||
| "RAM": "264 KB", | ||
| "Flash": "2 MB", | ||
| "Note": "Built-in simulator - no hardware required" | ||
| } | ||
| }, | ||
| ...existing entries... | ||
| } | ||
| ``` | ||
|
|
||
| Key decisions: | ||
| - **`"compiler": "simulator"`** — new compiler type, distinct from `"arduino-cli"` and `"openplc-compiler"`. This is the discriminator used throughout the codebase to determine behavior. | ||
| - Reuses the real Raspberry Pico `core`, `platform`, and `source` (rp2040pico.cpp) because compilation is identical. | ||
| - Pin mapping defaults match the Raspberry Pico HAL (DI: 6-13, DO: 14-21, AI: 26-28, AO: 4-5). | ||
|
|
||
| --- | ||
|
|
||
| ## 2. Device Type Utility Functions | ||
|
|
||
| **File:** `src/utils/device.ts` | ||
|
|
||
| Add a new utility function alongside the existing `isArduinoTarget` and `isOpenPLCRuntimeTarget`: | ||
|
|
||
| ```typescript | ||
| export function isSimulatorTarget(boardInfo: AvailableBoardInfo | undefined): boolean { | ||
| if (!boardInfo) return false | ||
| return boardInfo.compiler === 'simulator' | ||
| } | ||
| ``` | ||
|
|
||
| This function will be used across the UI to conditionally hide/show configuration fields. | ||
|
|
||
| --- | ||
|
|
||
| ## 3. Default Device for New Projects | ||
|
|
||
| **File:** `src/renderer/store/slices/device/data/constants.ts` | ||
|
|
||
| Change the default device board from `'OpenPLC Runtime v3'` to `'OpenPLC Simulator'`: | ||
|
|
||
| ```typescript | ||
| defaultDeviceConfiguration: DeviceConfiguration = { | ||
| deviceBoard: 'OpenPLC Simulator', // was 'OpenPLC Runtime v3' | ||
| ...rest unchanged... | ||
| } | ||
| ``` | ||
|
|
||
| This ensures every new project starts with the simulator selected, so users can build and debug immediately. | ||
|
|
||
| --- | ||
|
|
||
| ## 4. Device Editor UI — Hide Configuration for Simulator | ||
|
|
||
| **File:** `src/renderer/components/_features/[workspace]/editor/device/configuration/board.tsx` | ||
|
|
||
| The board.tsx component currently branches on `isOpenPLCRuntimeTarget(currentBoardInfo)`: | ||
| - If runtime target → shows IP address field, Connect button, scan cycle stats | ||
| - If arduino target → shows Communication Port dropdown, board specs, Pin Mapping table | ||
|
|
||
| For the simulator, **none of these should appear**. The board panel should show: | ||
| - The Device dropdown (so users can switch away from simulator) | ||
| - The board preview image (`simulator.png`) | ||
| - A simple message like "Built-in simulator — no configuration required" | ||
| - No communication port selector | ||
| - No IP address field | ||
| - No Connect button | ||
| - No pin mapping table (pins are fixed by the HAL) | ||
| - No Compile Only checkbox | ||
|
|
||
| Implementation: Add an `isSimulatorTarget(currentBoardInfo)` check at the top of the render logic: | ||
|
|
||
| ``` | ||
| Line 361: {isOpenPLCRuntimeTarget(currentBoardInfo) ? ( | ||
| ``` | ||
|
|
||
| Change to a three-way branch: | ||
|
|
||
| ``` | ||
| {isSimulatorTarget(currentBoardInfo) ? ( | ||
| <SimulatorInfo /> // Simple component showing "Built-in simulator" message | ||
| ) : isOpenPLCRuntimeTarget(currentBoardInfo) ? ( | ||
| ... existing runtime UI ... | ||
| ) : ( | ||
| ... existing arduino UI ... | ||
| )} | ||
| ``` | ||
|
|
||
| Similarly, the section after `<hr>` (line 488) that shows either scan-cycle stats or pin-mapping should show nothing (or a brief description) for the simulator. | ||
|
|
||
| **File:** `src/renderer/components/_features/[workspace]/editor/device/configuration/communication.tsx` | ||
|
|
||
| The Communication panel (Modbus RTU/TCP checkboxes and config) should be completely hidden when the simulator is selected. The simulator handles Modbus RTU internally and automatically. | ||
|
|
||
| Implementation: Early return or conditional render: | ||
| ```tsx | ||
| if (isSimulatorTarget(currentBoardInfo)) { | ||
| return ( | ||
| <DeviceEditorSlot heading='Communication'> | ||
| <p>Modbus RTU is automatically configured for the simulator.</p> | ||
| </DeviceEditorSlot> | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## 5. Compilation Flow — Handle Simulator Target | ||
|
|
||
| ### 5a. Compiler Module Changes (Build Button Only) | ||
|
|
||
| **File:** `src/main/modules/compiler/compiler-module.ts` | ||
|
|
||
| The `compileProgram()` method (line 1341) needs a new branch for `compiler === 'simulator'`. The compilation pipeline is: | ||
|
|
||
| 1. **Steps 1-10 are identical to Arduino/RP2040** — XML generation, xml2st, iec2c, debug.c, LOCATED_VARIABLES.h, C/C++ blocks. The simulator uses the exact same `rp2040pico.cpp` HAL and `rp2040:rp2040:rpipico` platform. | ||
|
|
||
| 2. **Step 11 (Arduino CLI compile)** — also identical. The simulator target still calls `arduino-cli compile` with the RP2040 core to produce a `.uf2` firmware binary. Arduino CLI must be installed. | ||
|
|
||
| 3. **Step 12 (Upload) — this is where the simulator diverges.** Instead of calling `arduino-cli upload` to a serial port, the compiled `.uf2` file path is sent back to the renderer via the MessageChannel port so the renderer can load it into rp2040js. | ||
|
|
||
| Implementation in `compileProgram()`: | ||
|
|
||
| ```typescript | ||
| // After successful Arduino CLI compilation... | ||
| if (boardRuntimeType === 'simulator') { | ||
| // Find the compiled .uf2 file | ||
| const uf2Path = path.join(buildDir, 'firmware.uf2') // or wherever arduino-cli outputs it | ||
| mainProcessPort.postMessage({ | ||
| logLevel: 'info', | ||
| message: 'Compilation successful. Loading firmware into simulator...', | ||
| }) | ||
| mainProcessPort.postMessage({ | ||
| simulatorFirmwarePath: uf2Path, | ||
| closePort: true, | ||
| }) | ||
| return | ||
| } | ||
| ``` | ||
|
|
||
| The `compileForDebugger()` method (line 2102) does **not** need a simulator-specific branch. This function only runs the first-stage compilation (XML → ST → C → debug metadata) and never invokes Arduino CLI or uploads anything. It works as-is for all targets, including the simulator. | ||
|
|
||
| ### 5b. IPC Handler Changes | ||
|
|
||
| **File:** `src/main/modules/ipc/main.ts` | ||
|
|
||
| The `handleRunCompileProgram` handler (line 777) passes args to `compilerModule.compileProgram()`. The args array includes `boardTarget` at index 1. The compiler module will read the board info from hals.json and detect `compiler === 'simulator'` to route appropriately. | ||
|
|
||
| No IPC handler changes needed — the existing MessageChannel pattern already supports sending arbitrary data back to the renderer. | ||
|
|
||
| ### 5c. Build Button Changes (Renderer) | ||
|
|
||
| **File:** `src/renderer/components/_organisms/workspace-activity-bar/default.tsx` | ||
|
|
||
| The build button handler (around line 267) calls `window.bridge.runCompileProgram(...)`. The callback receives messages from the compiler. | ||
|
|
||
| For the simulator target, the callback will receive a new message type: `{ simulatorFirmwarePath: string, closePort: true }`. | ||
|
|
||
| When this message arrives: | ||
| 1. Read the UF2 file from the path | ||
| 2. Load it into the rp2040js emulator instance (see Section 7) | ||
| 3. Start emulator execution | ||
| 4. Log "Simulator running" to the console | ||
|
|
||
| --- | ||
|
|
||
| ## 6. rp2040js Emulator Module | ||
|
|
||
| ### 6a. New Module | ||
|
|
||
| **File:** `src/main/modules/simulator/simulator-module.ts` (NEW) | ||
|
|
||
| This module manages the rp2040js emulator lifecycle in the **main process**. It lives in main because the Modbus RTU client (which the debugger uses) runs in main. | ||
|
|
||
| ```typescript | ||
| import { RP2040, loadUF2, USBCDC } from 'rp2040js' | ||
|
|
||
| class SimulatorModule { | ||
| private mcu: RP2040 | null = null | ||
| private running: boolean = false | ||
| private executionTimer: NodeJS.Timer | null = null | ||
|
|
||
| // Callbacks for UART bridge | ||
| onUartByte: ((byte: number) => void) | null = null | ||
|
|
||
| async loadAndRun(uf2Data: Buffer): Promise<void> { | ||
| this.stop() | ||
|
|
||
| this.mcu = new RP2040() | ||
| this.mcu.loadBootrom(bootromB1) // bundled bootrom | ||
| loadUF2(new Uint8Array(uf2Data), this.mcu) | ||
|
|
||
| // Wire UART0 output to the Modbus RTU bridge | ||
| this.mcu.uart[0].onByte = (byte: number) => { | ||
| this.onUartByte?.(byte) | ||
| } | ||
|
|
||
| // Start execution loop | ||
| this.mcu.PC = 0x10000000 | ||
| this.running = true | ||
| this.executeLoop() | ||
| } | ||
|
|
||
| feedByte(byte: number): void { | ||
| this.mcu?.uart[0].feedByte(byte) | ||
| } | ||
|
|
||
| stop(): void { | ||
| this.running = false | ||
| if (this.executionTimer) { | ||
| clearTimeout(this.executionTimer) | ||
| this.executionTimer = null | ||
| } | ||
| this.mcu = null | ||
| } | ||
|
|
||
| isRunning(): boolean { | ||
| return this.running | ||
| } | ||
|
|
||
| private executeLoop(): void { | ||
| if (!this.running || !this.mcu) return | ||
| // Execute a batch of cycles, then yield to event loop | ||
| this.mcu.execute() // runs until next yield point | ||
| this.executionTimer = setTimeout(() => this.executeLoop(), 0) | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### 6b. Bootrom | ||
|
|
||
| The RP2040 bootrom binary (~16KB) needs to be bundled. Options: | ||
| - Include as a `.ts` file with the binary data exported as a Uint8Array (same pattern as rp2040js's `demo/bootrom.ts`) | ||
| - Place in `resources/` and load at runtime | ||
|
|
||
| Recommended: Bundle as a TypeScript constant in `src/main/modules/simulator/bootrom.ts` for simplicity and zero filesystem dependency. | ||
|
|
||
| ### 6c. npm Dependency | ||
|
|
||
| ```bash | ||
| npm install rp2040js | ||
| ``` | ||
|
|
||
| Add to `package.json` dependencies. The package is ~200KB, zero transitive dependencies, supports both ESM and CJS. | ||
|
|
||
| --- | ||
|
|
||
| ## 7. Modbus RTU Bridge for Simulator — VirtualSerialPort Approach | ||
|
|
||
| ### Design Principle | ||
|
|
||
| Rather than duplicating the Modbus RTU protocol logic in a separate `SimulatorModbusClient` class, we create a `VirtualSerialPort` that mimics the `serialport` npm package's event-based API and adapts rp2040js's UART. The existing `ModbusRtuClient` is then reused unchanged — all CRC calculation, frame assembly, response parsing, retries, and timeout logic stays in one place. | ||
|
|
||
| This avoids code duplication and ensures any future bug fixes to the Modbus RTU protocol automatically apply to both physical hardware and the simulator. | ||
|
|
||
| ### 7a. VirtualSerialPort | ||
|
|
||
| **File:** `src/main/modules/simulator/virtual-serial-port.ts` (NEW) | ||
|
|
||
| `ModbusRtuClient.serialPort` is typed as `any` and uses these `serialport` APIs: | ||
| - `on('open', cb)` / `on('data', cb)` / `on('error', cb)` / `once('error', cb)` — events | ||
| - `write(data, callback)` — send bytes | ||
| - `flush(callback)` — flush input buffer | ||
| - `close()` — close port | ||
| - `isOpen` — boolean state | ||
| - `removeListener(event, fn)` — cleanup | ||
|
|
||
| `VirtualSerialPort` extends `EventEmitter` and implements all of these, routing bytes through `SimulatorModule.feedByte()` (TX to device) and `SimulatorModule.onUartByte` (RX from device): | ||
|
|
||
| ```typescript | ||
| import { EventEmitter } from 'events' | ||
| import { SimulatorModule } from './simulator-module' | ||
|
|
||
| export class VirtualSerialPort extends EventEmitter { | ||
| public isOpen = false | ||
| private simulator: SimulatorModule | ||
|
|
||
| constructor(simulator: SimulatorModule) { | ||
| super() | ||
| this.simulator = simulator | ||
| } | ||
|
|
||
| open(): void { | ||
| this.isOpen = true | ||
| // Wire UART RX: bytes from emulated device → ModbusRtuClient via 'data' events | ||
| this.simulator.onUartByte = (byte: number) => { | ||
| this.emit('data', Buffer.from([byte])) | ||
| } | ||
| // Emit 'open' asynchronously (matches real SerialPort behavior) | ||
| process.nextTick(() => this.emit('open')) | ||
| } | ||
|
|
||
| write(data: Uint8Array | Buffer, callback?: (err?: Error | null) => void): void { | ||
| // Send each byte to the emulated UART TX (host → device) | ||
| for (const byte of data) { | ||
| this.simulator.feedByte(byte) | ||
| } | ||
| callback?.(null) | ||
| } | ||
|
|
||
| flush(callback?: (err?: Error | null) => void): void { | ||
| // No hardware buffer to flush in virtual port | ||
| callback?.(null) | ||
| } | ||
|
|
||
| close(): void { | ||
| this.isOpen = false | ||
| this.simulator.onUartByte = null | ||
| this.removeAllListeners() | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| **Why byte-by-byte emission works:** `ModbusRtuClient.sendRequest()` already accumulates bytes into `responseBuffer` via `Buffer.concat` and uses a 50ms frame-completion timeout to detect end-of-frame. Each byte resets the timer. Since the emulated CPU processes response bytes in batches (within the same `executeLoop()` tick), they arrive nearly instantly and the 50ms gap correctly signals frame completion. | ||
|
|
||
| **No bootloader delay:** Physical serial ports have a 2.5s bootloader delay after opening. The `VirtualSerialPort` skips this entirely — the emulated UART is ready immediately. | ||
|
|
||
| ### 7b. ModbusRtuClient — Accept Injected Serial Port | ||
|
|
||
| **File:** `src/main/modules/modbus/modbus-rtu-client.ts` (MODIFIED — minimal change) | ||
|
|
||
| Add an optional `serialPort` field to the constructor options: | ||
|
|
||
| ```typescript | ||
| interface ModbusRtuClientOptions { | ||
| port: string | ||
| baudRate: number | ||
| slaveId: number | ||
| timeout: number | ||
| serialPort?: any // Pre-built serial port (e.g. VirtualSerialPort for simulator) | ||
| } | ||
| ``` | ||
|
|
||
| Store it in the constructor and add an early branch in `connect()`: | ||
|
|
||
| ```typescript | ||
| private injectedSerialPort: any = null | ||
|
|
||
| constructor(options: ModbusRtuClientOptions) { | ||
| this.port = options.port | ||
| this.baudRate = options.baudRate | ||
| this.slaveId = options.slaveId | ||
| this.timeout = options.timeout | ||
| this.injectedSerialPort = options.serialPort ?? null | ||
| } | ||
|
|
||
| async connect(): Promise<void> { | ||
| // If a pre-built serial port was provided (e.g. VirtualSerialPort), use it directly | ||
| if (this.injectedSerialPort) { | ||
| this.serialPort = this.injectedSerialPort | ||
| return new Promise((resolve, reject) => { | ||
| this.serialPort.on('open', () => resolve()) | ||
| this.serialPort.on('error', (err: Error) => reject(err)) | ||
| this.serialPort.open() | ||
| }) | ||
| } | ||
| // ...existing SerialPort creation code (unchanged)... | ||
| } | ||
| ``` | ||
|
|
||
| This is the **only change** to `ModbusRtuClient`. All protocol logic — `assembleRequest()`, `sendRequest()`, `calculateCrc()`, `getMd5Hash()`, `getVariablesList()`, `setVariable()` — remains untouched and is shared between physical hardware and the simulator. | ||
|
|
||
| ### 7c. No Separate SimulatorModbusClient or Shared Utils Needed | ||
|
|
||
| This approach eliminates: | ||
| - ~~`simulator-modbus-bridge.ts`~~ — not needed, `ModbusRtuClient` is reused directly | ||
| - ~~`modbus-rtu-utils.ts`~~ — not needed, no protocol code to extract/share | ||
|
|
||
| --- | ||
|
|
||
| ## 8. Debugger Connection for Simulator | ||
|
|
||
| ### 8a. IPC Handler — New Connection Type | ||
|
|
||
| **File:** `src/main/modules/ipc/main.ts` | ||
|
|
||
| The `handleDebuggerConnect` method (line 1114) currently supports `connectionType: 'tcp' | 'rtu' | 'websocket'`. Add `'simulator'`: | ||
|
|
||
| ```typescript | ||
| handleDebuggerConnect = async ( | ||
| _event: IpcMainInvokeEvent, | ||
| connectionType: 'tcp' | 'rtu' | 'websocket' | 'simulator', | ||
| connectionParams: { ... } | ||
| ): Promise<{ success: boolean; error?: string }> | ||
| ``` | ||
|
|
||
| New branch using `ModbusRtuClient` with `VirtualSerialPort` (no separate client class needed): | ||
|
|
||
| ```typescript | ||
| case 'simulator': | ||
| const virtualPort = new VirtualSerialPort(this.simulatorModule) | ||
| this.debuggerModbusClient = new ModbusRtuClient({ | ||
| port: 'simulator', // label only, not used for real I/O | ||
| baudRate: 115200, | ||
| slaveId: 1, | ||
| timeout: 5000, | ||
| serialPort: virtualPort, // injected virtual port | ||
| }) | ||
| await this.debuggerModbusClient.connect() | ||
| break | ||
| ``` | ||
|
|
||
| Since `ModbusRtuClient` is used directly, all existing debug polling logic (`handleDebuggerGetVariablesList`, `handleDebuggerVerifyMd5`, `handleDebuggerSetVariable`) works unchanged. | ||
|
|
||
| ### 8b. Debugger Button — Simulator Flow (Corrected) | ||
|
|
||
| **File:** `src/renderer/components/_organisms/workspace-activity-bar/default.tsx` | ||
|
|
||
| The debugger button does **not** compile the full firmware. It only runs the first-stage compilation (`compileForDebugger`) to generate debug metadata and extract the MD5 hash. This is already the existing behavior for all targets — `compileForDebugger()` never invokes Arduino CLI or uploads anything. | ||
|
|
||
| For the simulator, there is one extra check: whether the simulator has firmware loaded. If the user has never pressed Build, the simulator is "empty" and there's nothing to debug. | ||
|
|
||
| **Complete simulator debugger flow:** | ||
|
|
||
| 1. Detect `isSimulatorTarget` → skip all connection parameter checks (no IP, no port, no Modbus config) | ||
| 2. **Check if simulator is "empty"** via `window.bridge.simulatorIsRunning()` | ||
| - If empty (no firmware loaded) → show warning dialog: *"No firmware is running on the simulator. Would you like to build and upload the project first?"* | ||
| - If user agrees → trigger full build (`runCompileProgram`), which compiles and auto-loads firmware into emulator. After build completes, restart the debugger flow from step 3. | ||
| - If user declines → abort debugger | ||
| 3. Run `compileForDebugger()` (first-stage only: XML → ST → C → debug files). **No simulator-specific branch needed** — this function works as-is for all targets. | ||
| 4. Extract local MD5 from generated `program.st` (existing `debuggerReadProgramStMd5`) | ||
| 5. Connect to simulator and get its MD5 via `debuggerVerifyMd5('simulator', {}, expectedMd5)` — uses Modbus RTU over virtual UART | ||
| 6. **Compare MD5s** (existing logic): | ||
| - If match → proceed to parse debug.c, build variable tree, connect debugger, start polling | ||
| - If mismatch → show existing "Program Mismatch" dialog asking user to rebuild/upload. If user agrees, trigger full build and retry MD5 verification. (This is the same dialog that appears for real hardware when the running firmware doesn't match the current project.) | ||
|
|
||
| ```typescript | ||
| // In handleDebuggerClick(): | ||
| if (isSimulatorTarget(currentBoardInfo)) { | ||
| // Check if simulator has firmware loaded | ||
| const running = await window.bridge.simulatorIsRunning() | ||
| if (!running) { | ||
| const response = await window.bridge.showMessageDialog({ | ||
| type: 'warning', | ||
| title: 'Simulator Empty', | ||
| message: 'No firmware is running on the simulator. Would you like to build and upload the project first?', | ||
| buttons: ['Build & Upload', 'Cancel'], | ||
| }) | ||
| if (response === 0) { | ||
| // Trigger full build (same as build button), then restart debugger flow | ||
| // ... | ||
| } else { | ||
| setIsDebuggerProcessing(false) | ||
| return | ||
| } | ||
| } | ||
| connectionType = 'simulator' | ||
| // Fall through to normal debug compilation + MD5 verification | ||
| } | ||
| ``` | ||
|
|
||
| ### 8c. What Doesn't Change | ||
|
|
||
| - `compileForDebugger()` — works as-is, no simulator branch needed (first-stage only, no hardware) | ||
| - `handleMd5Verification()` — works as-is once `'simulator'` connection type is supported | ||
| - MD5 mismatch dialog — works as-is (triggers full build + retry) | ||
| - Debug file parsing, variable tree building, debug polling — all unchanged | ||
|
|
||
| --- | ||
|
|
||
| ## 9. New IPC Endpoints | ||
|
|
||
| **File:** `src/main/modules/ipc/main.ts` — add handlers | ||
| **File:** `src/main/modules/ipc/renderer.ts` — add renderer-side wrappers | ||
| **File:** `src/main/modules/preload/preload.ts` — expose via bridge | ||
|
|
||
| ### New IPC Calls | ||
|
|
||
| | Channel | Direction | Purpose | | ||
| |---------|-----------|---------| | ||
| | `simulator:load-firmware` | renderer → main | Load UF2 into emulator and start execution | | ||
| | `simulator:stop` | renderer → main | Stop the emulator | | ||
| | `simulator:is-running` | renderer → main | Check if emulator is currently running | | ||
|
|
||
| These are thin wrappers around `SimulatorModule` methods. | ||
|
|
||
| --- | ||
|
|
||
| ## 10. Firmware Modbus RTU Configuration | ||
|
|
||
| The existing RP2040 HAL (`resources/sources/hal/rp2040pico.cpp`) and the OpenPLC Arduino runtime already include Modbus RTU slave support. The compiler generates `defines.h` with Modbus configuration based on the device editor settings. | ||
|
|
||
| For the simulator, Modbus RTU must be **always enabled** with fixed defaults: | ||
| - Interface: `Serial` (UART0) | ||
| - Baud rate: `115200` | ||
| - Slave ID: `1` | ||
| - RS485 EN pin: none | ||
|
|
||
| **File:** `src/main/modules/compiler/compiler-module.ts` | ||
|
|
||
| In the code generation step that produces `defines.h`, when the target is `'simulator'`: | ||
|
|
||
| ```c | ||
| #define MODBUS_ENABLED | ||
| #define MODBUS_RTU | ||
| #define MODBUS_RTU_INTERFACE Serial | ||
| #define MODBUS_RTU_BAUD 115200 | ||
| #define MODBUS_RTU_SLAVE_ID 1 | ||
| ``` | ||
|
|
||
| These defines are hardcoded regardless of what the device editor shows (which for the simulator shows nothing). | ||
|
|
||
| --- | ||
|
|
||
| ## 11. Assets | ||
|
|
||
| ### Simulator Preview Image | ||
|
|
||
| **File:** `resources/sources/boards/images/simulator.png` (NEW) | ||
|
|
||
| Create a preview image for the simulator device. Suggestion: a stylized chip/CPU icon with a "play" symbol overlay to convey "virtual device." Should match the dimensions and style of existing board preview images. | ||
|
|
||
| --- | ||
|
|
||
| ## 12. File Summary | ||
|
|
||
| ### New Files | ||
|
|
||
| | File | Purpose | | ||
| |------|---------| | ||
| | `src/main/modules/simulator/simulator-module.ts` | rp2040js emulator lifecycle management | | ||
| | `src/main/modules/simulator/virtual-serial-port.ts` | EventEmitter-based serial port mock that routes bytes through emulated UART | | ||
| | `src/main/modules/simulator/bootrom.ts` | Bundled RP2040 bootrom binary | | ||
| | `resources/sources/boards/images/simulator.png` | Device preview image | | ||
|
|
||
| ### Modified Files | ||
|
|
||
| | File | Changes | | ||
| |------|---------| | ||
| | `resources/sources/boards/hals.json` | Add "OpenPLC Simulator" entry at top | | ||
| | `src/utils/device.ts` | Add `isSimulatorTarget()` function | | ||
| | `src/renderer/store/slices/device/data/constants.ts` | Change default device to "OpenPLC Simulator" | | ||
| | `src/renderer/components/_features/[workspace]/editor/device/configuration/board.tsx` | Hide comm port, pin mapping, IP address, Connect button for simulator | | ||
| | `src/renderer/components/_features/[workspace]/editor/device/configuration/communication.tsx` | Hide Modbus RTU/TCP config for simulator | | ||
| | `src/main/modules/compiler/compiler-module.ts` | Handle `compiler === 'simulator'` in compileProgram (skip upload, return UF2 path); force Modbus RTU defines | | ||
| | `src/main/modules/ipc/main.ts` | Add simulator IPC handlers; add `'simulator'` connection type to debugger | | ||
| | `src/main/modules/ipc/renderer.ts` | Add renderer wrappers for simulator IPC | | ||
| | `src/main/modules/preload/preload.ts` | Expose simulator bridge methods | | ||
| | `src/renderer/components/_organisms/workspace-activity-bar/default.tsx` | Handle simulator in build callback and debugger click (empty simulator check) | | ||
| | `src/main/modules/modbus/modbus-rtu-client.ts` | Accept optional injected serial port in constructor (for VirtualSerialPort) | | ||
| | `package.json` | Add `rp2040js` dependency | | ||
|
|
||
| --- | ||
|
|
||
| ## 13. Implementation Order | ||
|
|
||
| ### Phase 1: Foundation (Week 1) | ||
| 1. `npm install rp2040js` | ||
| 2. Add "OpenPLC Simulator" entry to `hals.json` | ||
| 3. Add `isSimulatorTarget()` to `src/utils/device.ts` | ||
| 4. Update default device to "OpenPLC Simulator" in constants.ts | ||
| 5. Create `SimulatorModule` with basic load/run/stop | ||
| 6. Bundle bootrom binary | ||
| 7. Add simulator IPC endpoints (load-firmware, stop, is-running) | ||
|
|
||
| ### Phase 2: Compilation (Week 2) | ||
| 8. Handle `compiler === 'simulator'` in `compileProgram()` — reuse Arduino CLI compilation for RP2040, skip upload step, return UF2 path | ||
| 9. Force Modbus RTU defines in generated `defines.h` for simulator | ||
| 10. Wire up build button callback to load UF2 into emulator on success | ||
|
|
||
| ### Phase 3: Debugger (Week 3) | ||
| 11. Create `VirtualSerialPort` (EventEmitter mock adapting rp2040js UART to `serialport` API) | ||
| 12. Modify `ModbusRtuClient` to accept optional injected serial port (single constructor change) | ||
| 13. Add `'simulator'` connection type to `handleDebuggerConnect` (creates `ModbusRtuClient` + `VirtualSerialPort`) | ||
| 14. Wire up debugger button flow for simulator: check if simulator is empty → first-stage compilation → MD5 verification → connect | ||
|
|
||
| ### Phase 4: UI Polish (Week 4) | ||
| 15. Update board.tsx to show simulator-specific UI (hide irrelevant fields) | ||
| 16. Update communication.tsx to show "auto-configured" message | ||
| 17. Create simulator preview image | ||
| 18. Testing: full flow (new project → build → debugger → see values) | ||
| 19. Edge cases: re-compile while running, stop simulator, switch device away from simulator | ||
|
|
Contributor
There was a problem hiding this comment.
The document describes the superseded RP2040/rp2040js design, not the implemented avr8js/ATmega2560 simulator.
Every technical detail — rp2040js, rp2040:rp2040:rpipico, rp2040pico.cpp, loadUF2, USBCDC, mcu.uart[0].feedByte, the bootrom binary (~16KB), firmware.uf2, and bootrom.ts — refers to the initial prototype that was replaced. The actual implementation uses avr8js, arduino.avr.mega, Intel HEX, and no bootrom. Leaving this document unrevised will mislead contributors into thinking the simulator targets RP2040.
At minimum, the filename and content should be updated to reflect the ATmega2560/avr8js architecture, or the document should be clearly marked as a historical design artifact.
🧰 Tools
🪛 LanguageTool
[style] ~143-~143: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...h, C/C++ blocks. The simulator uses the exact same rp2040pico.cpp HAL and `rp2040:rp2040...
(EN_WORDINESS_PREMIUM_EXACT_SAME)
[grammar] ~257-~257: Ensure spelling is correct
Context: ...is.executeLoop(), 0) } } ``` ### 6b. Bootrom The RP2040 bootrom binary (~16KB) needs ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.21.0)
[warning] 100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 106-106: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/simulator-rp2040-plan.md` around lines 1 - 600, This doc currently
documents an rp2040js-based design (references: rp2040js, rp2040:rp2040:rpipico,
rp2040pico.cpp, loadUF2, firmware.uf2, bootrom.ts, mcu.uart[0].feedByte) but the
project actually implements an avr8js/ATmega2560 simulator; update the file to
reflect the real implementation by either (a) renaming the title and all
occurrences of RP2040/rp2040js/rpipico/UF2/bootrom to avr8js/ATmega2560/Intel
HEX and adjust the architecture-specific sections (build/upload flow, emulator
module, virtual serial port behavior, Modbus UART wiring, and asset names) to
match the avr8js design, or (b) clearly mark the document as
historical/prototype (prepend a bold “Historical: RP2040 prototype — superseded
by avr8js/ATmega2560 implementation” notice at the top) and add a short summary
pointing to the current avr8js design; ensure mentions of functions/classes
(SimulatorModule, VirtualSerialPort, ModbusRtuClient) remain accurate for the
current implementation or are removed if not applicable.
|
|
||
| Implementation: Add an `isSimulatorTarget(currentBoardInfo)` check at the top of the render logic: | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Fenced code blocks are missing language identifiers (markdownlint MD040).
Lines 100–102 and 106–113 contain fenced code blocks without a language specifier. Add a language tag (e.g., ```tsx) to satisfy the markdownlint rule and enable syntax highlighting.
Also applies to: 106-106
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 100-100: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/simulator-rp2040-plan.md` at line 100, Add language identifiers to the
fenced code blocks that currently start and end with triple backticks (``` ...
```) so they satisfy markdownlint MD040; update the blocks at the occurrences
shown (the fenced blocks around lines with ``` and the later block spanning the
code sample) to include an appropriate language tag such as ```tsx or ```bash
depending on the sample content, ensuring each opening fence includes the
language token immediately after the backticks.
Comment on lines
+1357
to
+1367
| handleSimulatorLoadFirmware = async ( | ||
| _event: IpcMainInvokeEvent, | ||
| hexPath: string, | ||
| ): Promise<{ success: boolean; error?: string }> => { | ||
| try { | ||
| await this.simulatorModule.loadAndRun(hexPath) | ||
| return { success: true } | ||
| } catch (error) { | ||
| return { success: false, error: error instanceof Error ? error.message : String(error) } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
hexPath is used directly without any path validation.
This parameter arrives from the renderer process via IPC. Although the hex path originates from the compiler output, the renderer forwards it without sanitization. A compromised or malicious renderer could pass an arbitrary path to simulatorModule.loadAndRun. Consider validating that the path is within the expected project build directory and has a .hex extension.
🛡️ Proposed fix
handleSimulatorLoadFirmware = async (
_event: IpcMainInvokeEvent,
hexPath: string,
): Promise<{ success: boolean; error?: string }> => {
try {
+ // Validate path: must be within app userData/project directories and end with .hex
+ const resolvedPath = resolve(hexPath)
+ if (!resolvedPath.endsWith('.hex') || resolvedPath.includes('..')) {
+ return { success: false, error: 'Invalid firmware path' }
+ }
await this.simulatorModule.loadAndRun(hexPath)
return { success: true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/modules/ipc/main.ts` around lines 1357 - 1367, Validate the incoming
hexPath in handleSimulatorLoadFirmware before calling
simulatorModule.loadAndRun: ensure hexPath has a .hex extension, resolve and
normalize it (e.g. via path.resolve/path.normalize) and verify the resolved path
is inside the expected project build directory (compare with a resolved build
dir like this.projectBuildDir or a getProjectBuildDir() result using startsWith
or equivalent), and check the file exists and is a regular file (fs.stat). If
any check fails, return { success: false, error: '...' } rather than calling
simulatorModule.loadAndRun; only call simulatorModule.loadAndRun(hexPath) when
validation passes.
Comment on lines
+12
to
+13
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| serialPort?: any // Pre-built serial port (e.g. VirtualSerialPort for simulator) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, let's see the exact lines mentioned in the review
echo "=== Lines 12-13 and surrounding context in modbus-rtu-client.ts ==="
sed -n '8,20p' src/main/modules/modbus/modbus-rtu-client.ts
echo ""
echo "=== Lines 28-29 and surrounding context ==="
sed -n '24,35p' src/main/modules/modbus/modbus-rtu-client.ts
echo ""
echo "=== All any types in modbus-rtu-client.ts ==="
rg -n "any" src/main/modules/modbus/modbus-rtu-client.ts
echo ""
echo "=== Verify serialPort method calls ==="
rg -nP "serialPort\!?\.\w+|injectedSerialPort\!?\.\w+" src/main/modules/modbus/modbus-rtu-client.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 2595
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== VirtualSerialPort implementation ==="
if [ -f "src/main/modules/simulator/virtual-serial-port.ts" ]; then
cat -n src/main/modules/simulator/virtual-serial-port.ts
else
echo "File not found, searching for it..."
fd -t f "virtual-serial-port" src/
fiRepository: Autonomy-Logic/openplc-editor
Length of output: 1824
Replace any serial-port types with a typed port contract.
The any types at lines 12–13 and 28–29 remove compile-time guarantees for the serial port bridge surface (open, write, flush, listeners), violating the strict TypeScript requirement in the coding guidelines.
♻️ Proposed typing fix
interface ModbusRtuClientOptions {
port: string
baudRate: number
slaveId: number
timeout: number
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- serialPort?: any // Pre-built serial port (e.g. VirtualSerialPort for simulator)
+ serialPort?: SerialPortLike // Pre-built serial port (e.g. VirtualSerialPort for simulator)
}
+type SerialPortEvents = {
+ open: () => void
+ data: (data: Buffer) => void
+ error: (error: Error) => void
+}
+
+interface SerialPortLike {
+ isOpen: boolean
+ open(): void
+ close(): void
+ write(data: Uint8Array | Buffer, callback?: (err?: Error | null) => void): void
+ flush(callback?: (err?: Error | null) => void): void
+ on<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this
+ once<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this
+ removeListener<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this
+}
+
export class ModbusRtuClient {
private port: string
private baudRate: number
private slaveId: number
private timeout: number
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- private serialPort: any = null
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any`
- private injectedSerialPort: any = null
+ private serialPort: SerialPortLike | null = null
+ private injectedSerialPort: SerialPortLike | null = nullThe interface accurately captures all actual method calls throughout the file: .on('open'|'error'|'data', ...), .once('error', ...), .open(), .close(), .write(), .flush(), and .isOpen property access. VirtualSerialPort already implements this contract.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| serialPort?: any // Pre-built serial port (e.g. VirtualSerialPort for simulator) | |
| interface ModbusRtuClientOptions { | |
| port: string | |
| baudRate: number | |
| slaveId: number | |
| timeout: number | |
| serialPort?: SerialPortLike // Pre-built serial port (e.g. VirtualSerialPort for simulator) | |
| } | |
| type SerialPortEvents = { | |
| open: () => void | |
| data: (data: Buffer) => void | |
| error: (error: Error) => void | |
| } | |
| interface SerialPortLike { | |
| isOpen: boolean | |
| open(): void | |
| close(): void | |
| write(data: Uint8Array | Buffer, callback?: (err?: Error | null) => void): void | |
| flush(callback?: (err?: Error | null) => void): void | |
| on<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this | |
| once<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this | |
| removeListener<K extends keyof SerialPortEvents>(event: K, listener: SerialPortEvents[K]): this | |
| } | |
| export class ModbusRtuClient { | |
| private port: string | |
| private baudRate: number | |
| private slaveId: number | |
| private timeout: number | |
| private serialPort: SerialPortLike | null = null | |
| private injectedSerialPort: SerialPortLike | null = null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/modules/modbus/modbus-rtu-client.ts` around lines 12 - 13, Replace
the loose serialPort?: any with a concrete interface describing the port
contract used by ModbusRtuClient: declare an interface (e.g., SerialPortLike)
that includes on(event: 'open'|'error'|'data', cb), once(event: 'error', cb),
open(): Promise<void> | void, close(): Promise<void> | void, write(data: Buffer
| string, cb?: (err?: Error) => void): void, flush?(cb?: (err?: Error) => void):
void, and an isOpen boolean property; then change the property signature
serialPort?: any to serialPort?: SerialPortLike and ensure VirtualSerialPort
implements this interface (or add a type assertion) so all calls to .on, .once,
.open, .close, .write, .flush, and .isOpen are type-checked.
Comment on lines
+259
to
+262
| const nextEvent = (cpu as unknown as { nextClockEvent: { cycles: number } | null }).nextClockEvent | ||
| if (nextEvent && nextEvent.cycles > cpu.cycles) { | ||
| cpu.cycles = nextEvent.cycles | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
avr8js CPU nextClockEvent API public v0.20
💡 Result:
In avr8js v0.20.0, CPU.nextClockEvent is not a public API—it’s declared as a private field inside CPU:
private nextClockEvent: AVRClockEventEntry | null = null;[1]
So you’re not meant to read/modify it from outside the class. The supported/public way to interact with clock events is via:
cpu.addClockEvent(callback, cycles)cpu.updateClockEvent(callback, cycles)cpu.clearClockEvent(callback)- and
cpu.tick()(which internally checks/dispatchesnextClockEvent) [1]
If you really need visibility into “what’s the next scheduled clock event,” you’d have to maintain that yourself when calling addClockEvent, or fork/patch avr8js to expose it—accessing cpu['nextClockEvent'] would be relying on internal, non-stable implementation details. [1]
Sources: [1]
🏁 Script executed:
# Check package.json for avr8js version
find . -name "package.json" -type f | head -5 | xargs grep -l "avr8js" 2>/dev/null
# Get the avr8js version
cat package.json | grep -A 2 -B 2 "avr8js"Repository: Autonomy-Logic/openplc-editor
Length of output: 261
🏁 Script executed:
# Examine the actual code context around lines 259-262
sed -n '250,270p' src/main/modules/simulator/simulator-module.tsRepository: Autonomy-Logic/openplc-editor
Length of output: 782
🏁 Script executed:
# Check the CPU type import and interface definition in the file
head -50 src/main/modules/simulator/simulator-module.ts | grep -E "import|from|CPU"Repository: Autonomy-Logic/openplc-editor
Length of output: 475
🏁 Script executed:
# Look for all usages of nextClockEvent in the codebase
rg "nextClockEvent" --type ts -B 2 -A 2Repository: Autonomy-Logic/openplc-editor
Length of output: 594
Replace private API access with proper architectural redesign or public API usage.
CPU.nextClockEvent is a private field in avr8js v0.20.0 and is not a supported public API. Accessing it via double cast (as unknown as { ... }) violates the avr8js API contract and breaks encapsulation.
The documented public alternatives for clock event management are:
cpu.addClockEvent(callback, cycles)cpu.updateClockEvent(callback, cycles)cpu.clearClockEvent(callback)cpu.tick()(which internally handles next clock event dispatch)
Redesign the SLEEP fast-forward logic to either:
- Use the public
tick()method instead of manually accessing internal state, or - Maintain your own tracking of scheduled clock events when calling
addClockEvent()
This access pattern will break with any avr8js library update that changes the private field structure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/modules/simulator/simulator-module.ts` around lines 259 - 262, The
code directly reads the private field nextClockEvent via a double-cast in the
SLEEP fast-forward logic; replace that private API access by using avr8js public
APIs instead: either fast-forward by repeatedly calling cpu.tick() until the
desired cycles are reached or, if you need to schedule/inspect future events,
track events when you call cpu.addClockEvent(callback, cycles) and use
cpu.updateClockEvent / cpu.clearClockEvent to adjust them rather than reading
nextClockEvent; update the SLEEP handling code (the block that currently reads
nextClockEvent and sets cpu.cycles) to rely on cpu.tick() or your own
event-tracking data structure and remove the cast-based access.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx (1)
1176-1224:⚠️ Potential issue | 🟡 MinorSimulator firmware not reloaded after MD5-mismatch re-upload.
The
runCompileProgramcallback inside the mismatch-upload path (lines 1176–1224) does not handlesimulatorFirmwarePath. If the simulator reports an MD5 mismatch and the user chooses to re-upload, the firmware is recompiled but never reloaded into the emulator, leaving the simulator still running the stale firmware.🐛 Proposed fix
(data: { logLevel?: 'info' | 'error' | 'warning' message: string | Buffer plcStatus?: string closePort?: boolean + simulatorFirmwarePath?: string }) => { // …existing logging… + if (data.simulatorFirmwarePath) { + ;(window.bridge.simulatorLoadFirmware as (p: string) => Promise<{ success: boolean; error?: string }>)( + data.simulatorFirmwarePath, + ) + .then((result) => { + if (result.success) { + setSimulatorRunning(true) + } + }) + .catch((err: unknown) => { + consoleActions.addLog({ + id: crypto.randomUUID(), + level: 'error', + message: `Simulator reload error: ${err instanceof Error ? err.message : String(err)}`, + }) + }) + } if (data.closePort) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx` around lines 1176 - 1224, The closePort branch inside the runCompileProgram callback handles MD5 re-uploads but never reloads the emulator with simulatorFirmwarePath; update the data.closePort handler (the block that logs "Upload completed..." and calls handleMd5Verification) to, after the handleMd5Verification call completes, also invoke the existing routine that loads/restarts the simulator with simulatorFirmwarePath (i.e., call the bridge or helper function your codebase uses to load firmware into the emulator — locate where simulatorFirmwarePath is applied elsewhere and call that function here), ensuring the emulator is reloaded after a successful re-upload.
♻️ Duplicate comments (3)
src/main/modules/ipc/main.ts (2)
1365-1375:hexPathis still used without path validation.
handleSimulatorLoadFirmwarepasses the IPC-suppliedhexPathdirectly tosimulatorModule.loadAndRunwithout validating the extension, resolving against the expected build directory, or checking for traversal sequences. The fix from the previous review still applies.🛡️ Proposed fix
handleSimulatorLoadFirmware = async ( _event: IpcMainInvokeEvent, hexPath: string, ): Promise<{ success: boolean; error?: string }> => { try { + const resolvedPath = resolve(hexPath) + if (!resolvedPath.endsWith('.hex') || resolvedPath.includes('..')) { + return { success: false, error: 'Invalid firmware path' } + } await this.simulatorModule.loadAndRun(hexPath) return { success: true }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/modules/ipc/main.ts` around lines 1365 - 1375, The handler handleSimulatorLoadFirmware currently forwards the IPC-provided hexPath straight to simulatorModule.loadAndRun; validate and sanitize hexPath first by: ensuring the file extension is ".hex", resolving it against the expected build directory (use a canonical base like buildDir and path.resolve(path.join(buildDir, hexPath))), checking the resolved path begins with the buildDir prefix to prevent path traversal, verifying the file exists and is a regular file, and returning an error if any check fails before calling simulatorModule.loadAndRun; use the same symbol names (handleSimulatorLoadFirmware, simulatorModule.loadAndRun, hexPath) so the validation is colocated with the existing call.
894-911: Existing client not disconnected before overwrite inhandleDebuggerVerifyMd5simulator branch.
this.debuggerModbusClientis overwritten at line 908 without first checking and disconnecting an existing client, potentially leaving a danglingVirtualSerialPortonUartBytecallback. The fix from the previous review still applies.🐛 Proposed fix
if (connectionType === 'simulator') { + if (this.debuggerModbusClient) { + this.debuggerModbusClient.disconnect() + this.debuggerModbusClient = null + } const virtualPort = new VirtualSerialPort(this.simulatorModule)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/modules/ipc/main.ts` around lines 894 - 911, In handleDebuggerVerifyMd5's simulator branch, avoid overwriting this.debuggerModbusClient without first cleaning up any existing client: if this.debuggerModbusClient is set, await a proper teardown (e.g., call its disconnect/close method and remove any VirtualSerialPort callbacks) in a try/catch, then null out this.debuggerModbusClient and this.debuggerConnectionType before assigning the new client (the new client is created via new ModbusRtuClient and connected with connect()); ensure any errors during previous-client teardown are logged but do not prevent creating the new simulator client.src/renderer/components/_organisms/workspace-activity-bar/default.tsx (1)
116-121:simulatorRunningstill lacks mount-time sync — stale state on component mount.The
useEffectcorrectly resetssimulatorRunningtofalsewhen the main process stops the simulator, but it doesn't querysimulatorIsRunning()on mount. If the simulator was already running when this component first mounts (e.g., after a hot-reload or navigation), the toggle will show "Start Simulator" while the emulator is actually running.🛡️ Suggested addition
+ // Seed simulatorRunning from main process on mount + useEffect(() => { + if (isCurrentBoardSimulator) { + void (window.bridge.simulatorIsRunning as () => Promise<boolean>)().then(setSimulatorRunning) + } + }, [isCurrentBoardSimulator]) + // Sync simulatorRunning when the main process stops the simulator useEffect(() => { const cleanup = (window.bridge.onSimulatorStopped as (cb: () => void) => () => void)(() => { setSimulatorRunning(false) }) return cleanup }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx` around lines 116 - 121, On mount, sync simulatorRunning with the actual simulator state by calling the bridge query before subscribing: inside the useEffect in the default workspace-activity-bar component, invoke window.bridge.simulatorIsRunning() (or the correct async/sync API) and call setSimulatorRunning(result) to initialize state, then attach the existing onSimulatorStopped listener (currently created via (window.bridge.onSimulatorStopped as ...) => cleanup) so you still clear state when stopped; ensure you await or handle the promise if simulatorIsRunning is async and keep returning the same cleanup function from the useEffect.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 1176-1224: The closePort branch inside the runCompileProgram
callback handles MD5 re-uploads but never reloads the emulator with
simulatorFirmwarePath; update the data.closePort handler (the block that logs
"Upload completed..." and calls handleMd5Verification) to, after the
handleMd5Verification call completes, also invoke the existing routine that
loads/restarts the simulator with simulatorFirmwarePath (i.e., call the bridge
or helper function your codebase uses to load firmware into the emulator —
locate where simulatorFirmwarePath is applied elsewhere and call that function
here), ensuring the emulator is reloaded after a successful re-upload.
---
Duplicate comments:
In `@src/main/modules/ipc/main.ts`:
- Around line 1365-1375: The handler handleSimulatorLoadFirmware currently
forwards the IPC-provided hexPath straight to simulatorModule.loadAndRun;
validate and sanitize hexPath first by: ensuring the file extension is ".hex",
resolving it against the expected build directory (use a canonical base like
buildDir and path.resolve(path.join(buildDir, hexPath))), checking the resolved
path begins with the buildDir prefix to prevent path traversal, verifying the
file exists and is a regular file, and returning an error if any check fails
before calling simulatorModule.loadAndRun; use the same symbol names
(handleSimulatorLoadFirmware, simulatorModule.loadAndRun, hexPath) so the
validation is colocated with the existing call.
- Around line 894-911: In handleDebuggerVerifyMd5's simulator branch, avoid
overwriting this.debuggerModbusClient without first cleaning up any existing
client: if this.debuggerModbusClient is set, await a proper teardown (e.g., call
its disconnect/close method and remove any VirtualSerialPort callbacks) in a
try/catch, then null out this.debuggerModbusClient and
this.debuggerConnectionType before assigning the new client (the new client is
created via new ModbusRtuClient and connected with connect()); ensure any errors
during previous-client teardown are logged but do not prevent creating the new
simulator client.
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 116-121: On mount, sync simulatorRunning with the actual simulator
state by calling the bridge query before subscribing: inside the useEffect in
the default workspace-activity-bar component, invoke
window.bridge.simulatorIsRunning() (or the correct async/sync API) and call
setSimulatorRunning(result) to initialize state, then attach the existing
onSimulatorStopped listener (currently created via
(window.bridge.onSimulatorStopped as ...) => cleanup) so you still clear state
when stopped; ensure you await or handle the promise if simulatorIsRunning is
async and keep returning the same cleanup function from the useEffect.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
package.jsonsrc/main/modules/ipc/main.tssrc/main/modules/ipc/renderer.tssrc/main/modules/simulator/simulator-module.tssrc/renderer/components/_organisms/workspace-activity-bar/default.tsxsrc/renderer/screens/workspace-screen.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/renderer/screens/workspace-screen.tsx
- src/main/modules/simulator/simulator-module.ts
Detailed plan to integrate Wokwi's rp2040js emulator as a built-in simulator device. Covers device entry, compilation pipeline, Modbus RTU bridge over virtual UART, debugger integration, and UI changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Alphabetically closer to 'OpenPLC Runtime' in the device list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d debugger flow Two major revisions based on review feedback: 1. Modbus RTU bridge: Replace duplicated SimulatorModbusClient with a VirtualSerialPort that mimics the serialport event API and routes bytes through rp2040js UART. ModbusRtuClient gets a single constructor change to accept an optional injected serial port. All protocol logic (CRC, framing, retries, parsing) is reused as-is. Eliminates simulator-modbus-bridge.ts and modbus-rtu-utils.ts from the plan. 2. Debugger flow: The debugger button does NOT recompile the full firmware. It only runs first-stage compilation (compileForDebugger) to extract the MD5 hash. For the simulator, the extra check is whether the emulator is "empty" (no firmware loaded), which triggers a warning asking the user to build first. compileForDebugger() needs no simulator-specific branch - it works as-is for all targets. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ule, IPC Phase 1 of the OpenPLC Simulator implementation: - Install rp2040js npm dependency (Wokwi RP2040 emulator) - Add "OpenPLC Simulator" device entry to hals.json (first in list) - Add isSimulatorTarget() utility function to device.ts - Add 'simulator' to compiler type enums (Zod schemas + renderer types) - Change default device for new projects to "OpenPLC Simulator" - Create SimulatorModule with UF2 parsing, firmware loading, and execution loop (step-based with event loop yielding) - Bundle RP2040 bootrom binary (from rp2040js demo) - Add simulator IPC endpoints: load-firmware, stop, is-running - Add simulator methods to renderer bridge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Skip Arduino upload for simulator, send UF2 firmware path to renderer - Force Modbus RTU defines in defines.h when compiling for simulator - Handle simulatorFirmwarePath in build button callback to load firmware Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Create VirtualSerialPort that mocks serialport API over rp2040js UART - Modify ModbusRtuClient to accept injected serial port, avoiding code duplication - Add 'simulator' connection type to debugger connect and MD5 verify handlers - Add "empty simulator" check in debugger button flow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Hide Compile Only checkbox, comm port, specs, and pin mapping for simulator - Show simple "no configuration required" message in board settings - Show "auto-configured" message in communication panel for simulator - Add simulator.png preview image placeholder Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the simulator is selected, the user doesn't configure RTU settings (communication panel is hidden), so rtuSlaveId is null and MBSERIAL_SLAVE is missing from defines.h. Force Serial/115200/slave 1 for simulator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Arduino CLI outputs Baremetal.ino.uf2 (not Baremetal.uf2). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The raw RP2040.step() doesn't tick the simulation clock, so timer-based peripherals like UART never process bytes. The Simulator class properly advances the clock on each instruction, handles CPU wait states (WFI), and runs 1M iterations per batch for better throughput. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On RP2040, Serial maps to USB CDC while Serial1 maps to UART0. rp2040js bridges uart[0], so the firmware must use Serial1 for the simulator's Modbus RTU to communicate through the virtual UART. Also fix "undefined" display in debugger log for simulator target. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reconnection logic in handleDebuggerGetVariablesList only handled 'tcp' and 'rtu' connection types. When the first poll attempt failed for simulator, the client was nulled and subsequent reconnection attempts fell through to the error case returning "No connection type stored", causing all variable values to display as "-". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The polling setup in workspace-screen.tsx bailed out early when neither TCP nor RTU was enabled. For the simulator, both are disabled (Modbus RTU is auto-configured on the main process side), so the polling never started and all variable values displayed as "-". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wire start/stop button to control simulator state (start re-builds, stop calls simulatorStop). Button is enabled for simulator targets and shows the stop icon when the simulator is running. - Sort board list alphabetically so OpenPLC Simulator appears next to OpenPLC Runtime entries instead of first in the list. - Hide core version suffix (e.g. [5.4.3]) from the simulator entry in the board dropdown since it refers to the underlying rp2040 Arduino core, not the simulator itself. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The stock rp2040js Simulator class uses setTimeout(0) with 1M iterations per batch (~8ms simulated time). In Node.js, setTimeout(0) has a minimum ~1-4ms overhead, causing the simulation to run ~4x slower than real time. Replace the stock Simulator with a custom execution loop that: - Uses setImmediate instead of setTimeout(0) to minimize scheduling overhead between batches - Increases the batch size from 1M to 4M iterations per batch - Includes a local SimClock implementation to avoid deep-importing from rp2040js internals (not exposed in the package exports) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The OpenPLC firmware busy-waits in loop() checking micros(), so the CPU never enters WFI. A JavaScript ARM emulator can only execute ~30-40M instructions/sec, but real-time 125 MHz needs 125M/sec, causing the simulation to run ~4x slower than real time. Fix by dynamically measuring the sim-to-wall speed ratio every 10 batches and scaling the nanoseconds-per-cycle accordingly. If the simulation is running at 0.25x real time, cycleNanos is multiplied by 4 so timers and millis() advance at wall-clock rate. Clamped to 1-8x nominal to avoid instability. Also reverts batch size to stock 1M to keep event loop responsive for Modbus I/O. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The OpenPLC firmware busy-waits in loop() checking micros(), executing
millions of useless instructions per scan cycle. A JS ARM emulator
can only do ~30-40M instructions/sec vs the 125M/sec needed, causing
~4x slower than real-time execution.
Fix by adding __asm volatile("wfi") at the end of loop() when
SIMULATOR_MODE is defined. WFI puts the CPU to sleep until the next
interrupt (SysTick at 1ms, UART RX, etc.). The emulator's execution
loop already handles WFI by fast-forwarding the clock to the next
alarm, skipping idle time instantly instead of stepping through every
cycle. This makes the simulation run at near real-time speed.
Changes:
- defines.h generation adds #define SIMULATOR_MODE for simulator builds
- Baremetal.ino: WFI inserted at end of loop() under #ifdef
- simulator-module.ts: reverted dynamic calibration hack (no longer
needed), kept setImmediate and custom SimClock
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
With WFI in the firmware loop, the emulator fast-forwards idle time instantly, causing the simulation to run ~2.5x faster than real time. Fix by comparing simulated elapsed time to wall elapsed time after each batch. When the simulation is ahead, schedule the next batch with setTimeout(delay) to let wall time catch up. When behind or on time, schedule with setTimeout(0). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The simulator had no lifecycle cleanup — it would keep running in the background when opening a new project, creating a project, reloading the window, or quitting the app. This wasted CPU resources and could lead to stale simulator state. Add simulatorModule.stop() calls to: - handleProjectOpen / handleProjectOpenByPath / handleProjectCreate - handleWindowReload - handleAppQuit Note: loadAndRun() already calls stop() before starting, so re-building the same project is already safe. The single SimulatorModule instance prevents dual-simulator scenarios. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the rp2040js emulation core with avr8js targeting ATmega2560 (Arduino Mega). The new emulator loads Intel HEX firmware, emulates Timer0/1/2 + USART0 + AVRClock peripherals, and uses SLEEP-based fast-forwarding for near real-time execution speed. Key changes: - New simulator-module.ts using avr8js CPU with ATmega2560 configs (256KB flash, 8KB SRAM, correct interrupt vector addresses) - Intel HEX parser supporting extended segment/linear address records - UDR read-hook byte drain to prevent RX data loss during ISRs - Compiler targets arduino:avr:mega with SIMULATOR_MODE define - Board config (hals.json) updated for Arduino Mega pin mappings - Baremetal.ino uses AVR SLEEP instruction instead of ARM WFI - Old RP2040 simulator preserved as simulator-module-rp2040.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SLEEP fast-forwards no longer count against the per-batch iteration limit. Instead, use two separate limits: a real-instruction cap (100K) for actual CPU work and a sim-time cap (100ms) to prevent runaway batches during idle periods. This makes idle time between scan cycles essentially free, allowing each batch to cover more simulated time with the same wall-clock cost. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit ef78538.
…2040js - Fix rxQueue deadlock: feedByte() only attempted USART delivery on empty-to-non-empty transition; if rejected (rxBusy), the queue stalled permanently. Add batch-start retry to break the cycle. - Expand emulated SRAM from 8KB to ~63.5KB (full 16-bit address space) with linker flags (--defsym __DATA_REGION_LENGTH__/--stack) so avr-gcc uses the extra space. - Add ld_flags support to compiler module and board type schemas. - Remove rp2040js dependency and delete dead RP2040 emulator files (bootrom.ts, simulator-module-rp2040.ts). - Fix stale references: RP2040 -> ATmega2560, UF2 -> HEX. - Replace hardcoded UDR address 0xc6 with config reference. - Fix ModbusRtuClient listener leak in sendRequest error paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Pin avr8js to exact 0.20.0 (private nextClockEvent field used for SLEEP fast-forward could break on minor version bumps) - Add simulator:stopped IPC event so renderer UI stays in sync when main process stops the simulator on project open/create - Remove outdated docs/simulator-rp2040-plan.md (superseded by AVR ATmega2560 migration) - Fix ESLint warnings in workspace-screen.tsx by hoisting isSimulatorTarget call into a const Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add _mainProcessPort.close() after postMessage in the simulator compilation branch, matching all other early-return paths - Derive build sub-directory from platform FQBN instead of hardcoding 'arduino.avr.mega', so it stays in sync with hals.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
thiagoralves
force-pushed
the
feat/simulator
branch
from
6d0e585 to
9ed3b03
Compare
This was referenced Feb 25, 2026
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
1 participant
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.
Summary
Key changes
simulator-module.ts: AVR CPU emulation loop with SLEEP fast-forward, USART RX/TX bridging, wall-clock pacingvirtual-serial-port.ts: EventEmitter bridge between emulated USART and ModbusRtuClientcompiler-module.ts: Simulator compilation path +ld_flagssupport for expanded SRAMhals.json: OpenPLC Simulator board entry with ATmega2560 config and linker flagsmodbus-rtu-client.ts: Fixed listener leak in sendRequest error pathsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores