feat: simulator stubs for hardware-dependent blocks#630
Merged
Conversation
Allow the simulator to compile and run programs that use hardware-specific blocks (P1AM, MQTT, DS18B20, CAN, Sequent Microsystems, Arduino Cloud, TCP) by providing no-op stub implementations. Previously these blocks failed to compile for the simulator because their module files include platform-specific headers unavailable on AVR. - Add simulator_stubs.c with 51 no-op function stubs covering all hardware modules plus TCP communication functions - Route arduino_libs.h to stubs when SIMULATOR_MODE is defined, bypassing real hardware module includes - Bump matiec to v4.0.11 which allows communication.h (TCP block type definitions) to be included in simulator builds Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
WalkthroughMultiple changes introduce a new PLC preprocessing pipeline, make simulator-aware gating across UI components, and update a single dependency version in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActivityBar as WorkspaceActivityBar
participant Preproc as PreprocessService
participant Compiler
participant Debugger
participant Board as Board/Simulator
User->>ActivityBar: Start Compile / Start Debug
ActivityBar->>Preproc: preprocessPous(projectData, isSimulator, log)
Preproc-->>ActivityBar: { projectData: processedProjectData, validationFailed }
alt validationFailed == true
ActivityBar->>User: surface validation error / abort flow
else validationFailed == false
ActivityBar->>Compiler: compile(processedProjectData)
Compiler-->>ActivityBar: compilation result
alt Debug requested
ActivityBar->>Debugger: connectAndActivateDebugger(processedProjectData, boardContext)
Debugger->>Board: attach / start debug session
Board-->>Debugger: runtime responses
Debugger-->>ActivityBar: debug events/status
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
…device creation Extract duplicated Python + C++ POU preprocessing into a shared preprocessPous() utility. For the simulator target, Python function blocks compile as empty ST stubs instead of generating the full Python runtime wrapper (which requires POSIX shared memory unavailable on AVR). Unlock server and remote-device creation UI for the simulator target alongside OpenPLC Runtime v4, and suppress the board-switch warning modal when switching to the simulator with existing servers or remote devices. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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 (3)
src/renderer/components/_features/[workspace]/create-element/element-card/index.tsx (3)
411-419: User-facing message should mention simulator support.The gating logic now uses
allowServersAndRemoteDevices(which includes simulator), but the disabled message still only references "OpenPLC Runtime v4 targets". This could confuse users who selected a non-v4, non-simulator target.Consider updating the message to reflect that both Runtime v4 and Simulator support this feature:
📝 Suggested message update
{!allowServersAndRemoteDevices ? ( <div className='flex flex-col gap-2 py-2'> <p className='text-sm text-neutral-700 dark:text-neutral-300'> - Server configuration is only available for OpenPLC Runtime v4 targets. + Server configuration is only available for OpenPLC Runtime v4 or Simulator targets. </p> <p className='text-xs text-neutral-500 dark:text-neutral-400'> - Please select OpenPLC Runtime v4 in the Device Configuration to enable this feature. + Please select OpenPLC Runtime v4 or Simulator in the Device Configuration to enable this feature. </p> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_features/`[workspace]/create-element/element-card/index.tsx around lines 411 - 419, The user-facing message shown when allowServersAndRemoteDevices is false only references "OpenPLC Runtime v4 targets" and should also mention Simulator support; update the copy inside the JSX block that renders when !allowServersAndRemoteDevices (the div containing the two <p> elements) to say that Server configuration is available for OpenPLC Runtime v4 and Simulator targets and instruct the user to select Runtime v4 or Simulator in Device Configuration to enable the feature—locate the message text near the allowServersAndRemoteDevices conditional and replace the two paragraphs' strings accordingly.
746-748: Redundant condition—isSimulatorandisArduinoTargetare mutually exclusive.Based on the utility functions in
src/utils/device.ts,isArduinoTargetcheckscompiler === 'arduino-cli'andisSimulatorTargetcheckscompiler === 'simulator'. Since these are mutually exclusive,isArduinoTarget && !isSimulatoris logically equivalent to justisArduinoTarget.The code is functionally correct, but the
!isSimulatorcheck is unnecessary given the current implementation. If this is intentional defensive coding for potential future changes where a board could be both, consider adding a comment explaining the intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_features/`[workspace]/create-element/element-card/index.tsx around lines 746 - 748, The condition creating isPythonBlockedForArduino is using a redundant check: remove the unnecessary "!isSimulator" and set it to const isPythonBlockedForArduino = lang.value === 'Python' && isArduinoTarget; if the original extra check was intentional for future defensive reasons, instead add a short comment above the declaration explaining why both checks are present; refer to the isPythonBlockedForArduino, isArduinoTarget and isSimulator symbols to locate the change.
546-554: Same messaging inconsistency for remote device section.Similar to the server section, the disabled message should mention simulator support for consistency.
📝 Suggested message update
{!allowServersAndRemoteDevices ? ( <div className='flex flex-col gap-2 py-2'> <p className='text-sm text-neutral-700 dark:text-neutral-300'> - Remote device configuration is only available for OpenPLC Runtime v4 targets. + Remote device configuration is only available for OpenPLC Runtime v4 or Simulator targets. </p> <p className='text-xs text-neutral-500 dark:text-neutral-400'> - Please select OpenPLC Runtime v4 in the Device Configuration to enable this feature. + Please select OpenPLC Runtime v4 or Simulator in the Device Configuration to enable this feature. </p> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/_features/`[workspace]/create-element/element-card/index.tsx around lines 546 - 554, Update the disabled remote device message in element-card/index.tsx (the JSX block guarded by the allowServersAndRemoteDevices check) to match the server-section wording by explicitly mentioning simulator support; modify the two <p> strings inside the !allowServersAndRemoteDevices branch so they say that remote device configuration is only available for OpenPLC Runtime v4 targets (and the simulator) and instruct the user to select OpenPLC Runtime v4 in Device Configuration to enable the feature, mirroring the server messaging for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 580-592: The upload fallback path still uses raw projectData
causing preprocessing to be skipped; update the upload/MD5-mismatch compile call
to pass the already preprocessedProjectData from preprocessPous instead of
projectData so Python/C++ preprocessing runs consistently—locate where the MD5
mismatch/upload compile is invoked (the counterpart to
window.bridge.runDebugCompilation) and replace its projectData argument with
processedProjectData, preserving the same validationFailed handling and
consoleActions logging flow from preprocessPous.
In `@src/utils/PLC/preprocess-pous.ts`:
- Around line 63-77: The early preprocessing result from
applyEarlyCommentWrapping is being discarded when Python POUs exist because
addPythonLocalVariables is called with projectData instead of
processedProjectData; change the call to
addPythonLocalVariables(processedProjectData) (and ensure any downstream uses
refer to processedProjectData) so the wrapped ST/IL comments produced by
applyEarlyCommentWrapping are preserved when processing pythonPous.
- Around line 96-111: The map over processedProjectData.pous currently skips
conversion when processedPythonCodes[pythonIndex] is falsy (e.g. empty string),
causing misalignment; change the truthy check around
processedPythonCodes[pythonIndex] in the pou.data.body.language === 'python'
branch to explicitly test for undefined (e.g. processedPythonCodes[pythonIndex]
!== undefined or typeof processedPythonCodes[pythonIndex] !== 'undefined') so
empty/valid transformed code is still used by generateSTCode, and only increment
pythonIndex when the value is defined; references: processedProjectData.pous,
PLCPou, pou.data.body.language, processedPythonCodes[pythonIndex],
generateSTCode, pythonIndex.
---
Nitpick comments:
In
`@src/renderer/components/_features/`[workspace]/create-element/element-card/index.tsx:
- Around line 411-419: The user-facing message shown when
allowServersAndRemoteDevices is false only references "OpenPLC Runtime v4
targets" and should also mention Simulator support; update the copy inside the
JSX block that renders when !allowServersAndRemoteDevices (the div containing
the two <p> elements) to say that Server configuration is available for OpenPLC
Runtime v4 and Simulator targets and instruct the user to select Runtime v4 or
Simulator in Device Configuration to enable the feature—locate the message text
near the allowServersAndRemoteDevices conditional and replace the two
paragraphs' strings accordingly.
- Around line 746-748: The condition creating isPythonBlockedForArduino is using
a redundant check: remove the unnecessary "!isSimulator" and set it to const
isPythonBlockedForArduino = lang.value === 'Python' && isArduinoTarget; if the
original extra check was intentional for future defensive reasons, instead add a
short comment above the declaration explaining why both checks are present;
refer to the isPythonBlockedForArduino, isArduinoTarget and isSimulator symbols
to locate the change.
- Around line 546-554: Update the disabled remote device message in
element-card/index.tsx (the JSX block guarded by the
allowServersAndRemoteDevices check) to match the server-section wording by
explicitly mentioning simulator support; modify the two <p> strings inside the
!allowServersAndRemoteDevices branch so they say that remote device
configuration is only available for OpenPLC Runtime v4 targets (and the
simulator) and instruct the user to select OpenPLC Runtime v4 in Device
Configuration to enable the feature, mirroring the server messaging for
consistency.
ℹ️ 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 selected for processing (4)
src/renderer/components/_features/[workspace]/create-element/element-card/index.tsxsrc/renderer/components/_features/[workspace]/editor/device/configuration/board.tsxsrc/renderer/components/_organisms/workspace-activity-bar/default.tsxsrc/utils/PLC/preprocess-pous.ts
Comment on lines
+580
to
592
| const { projectData: processedProjectData, validationFailed } = preprocessPous( | ||
| projectData, | ||
| isSimulatorTarget(currentBoardInfo), | ||
| (level, message) => consoleActions.addLog({ id: crypto.randomUUID(), level, message }), | ||
| ) | ||
| if (validationFailed) { | ||
| setIsDebuggerProcessing(false) | ||
| return | ||
| } | ||
|
|
||
| window.bridge.runDebugCompilation( | ||
| [projectPath, boardTarget, processedProjectData as ProjectDataWithCpp], | ||
| [projectPath, boardTarget, processedProjectData], | ||
| (data: { logLevel?: 'info' | 'error' | 'warning'; message: string | Buffer; closePort?: boolean }) => { |
Contributor
There was a problem hiding this comment.
Use preprocessed project data in every compile path, including MD5 mismatch upload.
This new flow preprocesses for debug compilation, but the upload fallback still sends raw projectData (Line 899). That can bypass Python/C++ preprocessing in that path.
💡 Suggested fix (outside this hunk, at the upload compile call)
- window.bridge.runCompileProgram(
- [projectPath, boardTarget, boardCore, false, projectData, targetIpAddress ?? '', runtimeJwtToken],
+ const { projectData: uploadProjectData, validationFailed: uploadValidationFailed } = preprocessPous(
+ projectData,
+ isSimulatorTarget(availableBoards.get(boardTarget)),
+ (level, message) => consoleActions.addLog({ id: crypto.randomUUID(), level, message }),
+ )
+ if (uploadValidationFailed) {
+ setIsDebuggerProcessing(false)
+ return
+ }
+
+ window.bridge.runCompileProgram(
+ [projectPath, boardTarget, boardCore, false, uploadProjectData, targetIpAddress ?? '', runtimeJwtToken],
(data: {🤖 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 580 - 592, The upload fallback path still uses raw projectData causing
preprocessing to be skipped; update the upload/MD5-mismatch compile call to pass
the already preprocessedProjectData from preprocessPous instead of projectData
so Python/C++ preprocessing runs consistently—locate where the MD5
mismatch/upload compile is invoked (the counterpart to
window.bridge.runDebugCompilation) and replace its projectData argument with
processedProjectData, preserving the same validationFailed handling and
consoleActions logging flow from preprocessPous.
Comment on lines
+63
to
+77
| let processedProjectData: PLCProjectData = applyEarlyCommentWrapping(projectData) | ||
|
|
||
| // --- Python processing --- | ||
| const hasPythonCode = projectData.pous.some((pou: PLCPou) => pou.data.body.language === 'python') | ||
|
|
||
| if (hasPythonCode) { | ||
| const pythonPous = projectData.pous.filter((pou: PLCPou) => pou.data.body.language === 'python') | ||
|
|
||
| pythonPous.forEach((pou) => { | ||
| log('info', `Found Python POU: "${pou.data.name}" (${pou.type})`) | ||
| }) | ||
|
|
||
| log('info', `Processing ${pythonPous.length} Python POU(s)...`) | ||
|
|
||
| processedProjectData = addPythonLocalVariables(projectData) |
Contributor
There was a problem hiding this comment.
Preserve early preprocessing when Python POUs are present.
Line 77 rebuilds from projectData instead of processedProjectData, so wrapped ST/IL comments from Line 63 are lost whenever Python POUs exist.
💡 Suggested fix
- processedProjectData = addPythonLocalVariables(projectData)
+ processedProjectData = addPythonLocalVariables(processedProjectData)📝 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
| let processedProjectData: PLCProjectData = applyEarlyCommentWrapping(projectData) | |
| // --- Python processing --- | |
| const hasPythonCode = projectData.pous.some((pou: PLCPou) => pou.data.body.language === 'python') | |
| if (hasPythonCode) { | |
| const pythonPous = projectData.pous.filter((pou: PLCPou) => pou.data.body.language === 'python') | |
| pythonPous.forEach((pou) => { | |
| log('info', `Found Python POU: "${pou.data.name}" (${pou.type})`) | |
| }) | |
| log('info', `Processing ${pythonPous.length} Python POU(s)...`) | |
| processedProjectData = addPythonLocalVariables(projectData) | |
| let processedProjectData: PLCProjectData = applyEarlyCommentWrapping(projectData) | |
| // --- Python processing --- | |
| const hasPythonCode = projectData.pous.some((pou: PLCPou) => pou.data.body.language === 'python') | |
| if (hasPythonCode) { | |
| const pythonPous = projectData.pous.filter((pou: PLCPou) => pou.data.body.language === 'python') | |
| pythonPous.forEach((pou) => { | |
| log('info', `Found Python POU: "${pou.data.name}" (${pou.type})`) | |
| }) | |
| log('info', `Processing ${pythonPous.length} Python POU(s)...`) | |
| processedProjectData = addPythonLocalVariables(processedProjectData) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/PLC/preprocess-pous.ts` around lines 63 - 77, The early
preprocessing result from applyEarlyCommentWrapping is being discarded when
Python POUs exist because addPythonLocalVariables is called with projectData
instead of processedProjectData; change the call to
addPythonLocalVariables(processedProjectData) (and ensure any downstream uses
refer to processedProjectData) so the wrapped ST/IL comments produced by
applyEarlyCommentWrapping are preserved when processing pythonPous.
Comment on lines
+96
to
+111
| processedProjectData.pous = processedProjectData.pous.map((pou: PLCPou) => { | ||
| if (pou.data.body.language === 'python') { | ||
| if (processedPythonCodes[pythonIndex]) { | ||
| const stCode = generateSTCode({ | ||
| pouName: pou.data.name, | ||
| allVariables: pou.data.variables, | ||
| processedPythonCode: processedPythonCodes[pythonIndex], | ||
| }) | ||
|
|
||
| pou.data.body = { | ||
| language: 'st', | ||
| value: stCode, | ||
| } | ||
|
|
||
| pythonIndex++ | ||
| } |
Contributor
There was a problem hiding this comment.
Avoid truthy checks for indexed transformed Python code.
Line 98 treats '' as missing. That can skip conversion and leave later Python POUs misaligned/unprocessed.
💡 Suggested fix
let pythonIndex = 0
processedProjectData.pous = processedProjectData.pous.map((pou: PLCPou) => {
if (pou.data.body.language === 'python') {
- if (processedPythonCodes[pythonIndex]) {
+ const processedPythonCode = processedPythonCodes[pythonIndex]
+ pythonIndex++
+ if (processedPythonCode !== undefined) {
const stCode = generateSTCode({
pouName: pou.data.name,
allVariables: pou.data.variables,
- processedPythonCode: processedPythonCodes[pythonIndex],
+ processedPythonCode,
})
pou.data.body = {
language: 'st',
value: stCode,
}
-
- pythonIndex++
+ } else {
+ log('error', `Missing transformed Python code for "${pou.data.name}"`)
}
}
return pou
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/PLC/preprocess-pous.ts` around lines 96 - 111, The map over
processedProjectData.pous currently skips conversion when
processedPythonCodes[pythonIndex] is falsy (e.g. empty string), causing
misalignment; change the truthy check around processedPythonCodes[pythonIndex]
in the pou.data.body.language === 'python' branch to explicitly test for
undefined (e.g. processedPythonCodes[pythonIndex] !== undefined or typeof
processedPythonCodes[pythonIndex] !== 'undefined') so empty/valid transformed
code is still used by generateSTCode, and only increment pythonIndex when the
value is defined; references: processedProjectData.pous, PLCPou,
pou.data.body.language, processedPythonCodes[pythonIndex], generateSTCode,
pythonIndex.
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
arduino_libs.htosimulator_stubs.cwhenSIMULATOR_MODEis defined, bypassing real hardware module includes that require platform-specific headers#ifndef ARDUINOguard iniec_std_FB.hto#if !defined(ARDUINO) || defined(SIMULATOR_MODE), allowing TCP block type definitions to be included in simulator buildsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Chores