Fix debugger upload path for C/C++ function blocks#701
Conversation
When debugger MD5 verification failed and the user chose to upload the current project, the UI called runCompileProgram with the raw project data instead of the preprocessed project data. That bypassed preprocessPous() for the second compilation path, so C/C++ function block bodies were sent through the IEC ST/XML pipeline as plain source code. In practice this produced a different second source set and caused iec2c to fail on invalid IL/ST instructions such as C comments, #include directives, and setup()/loop() declarations. This change applies the same preprocessPous() step used by the normal compile and debug flows before invoking runCompileProgram from the debugger mismatch flow, and aborts early if C/C++ validation fails.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR refactors preprocessing logic in the workspace activity bar component by introducing a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx (1)
897-909: Good fix—consider sharing the compile-prep block.This upload path now repeats the same
preprocessPous(...)/validationFailedgate used at Lines 108-116 and 580-588. Since this PR exists because one compile path drifted, pulling that prep into a small helper would make therunCompileProgram/runDebugCompilationcall sites much harder to desync again.🤖 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 897 - 909, Extract the repeated preprocessing + validation gate into a small helper (e.g., prepareProjectForCompile) that calls preprocessPous(projectData, isSimulatorTarget(currentBoardInfo), (level, message) => consoleActions.addLog(...)) and returns { processedProjectData, validationFailed }; replace the duplicated blocks before window.bridge.runCompileProgram and before the runDebugCompilation call with this helper and ensure you still call setIsDebuggerProcessing(false) and return when validationFailed is true; keep the same arguments passed to runCompileProgram (processedProjectData, targetIpAddress, runtimeJwtToken) so call sites remain identical except for using the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Around line 897-909: Extract the repeated preprocessing + validation gate into
a small helper (e.g., prepareProjectForCompile) that calls
preprocessPous(projectData, isSimulatorTarget(currentBoardInfo), (level,
message) => consoleActions.addLog(...)) and returns { processedProjectData,
validationFailed }; replace the duplicated blocks before
window.bridge.runCompileProgram and before the runDebugCompilation call with
this helper and ensure you still call setIsDebuggerProcessing(false) and return
when validationFailed is true; keep the same arguments passed to
runCompileProgram (processedProjectData, targetIpAddress, runtimeJwtToken) so
call sites remain identical except for using the helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 854a4f37-a7a9-40b7-a83d-631dfdf84f63
📒 Files selected for processing (1)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx (1)
895-895: Consider fetching project data fresh from the store for consistency.Within
handleMd5Verification, other data is fetched fresh from the store (lines 655, 790, 894), butprepareProjectForCompile()usesprojectDatacaptured in the component's closure. In long-running async flows with potential delays (e.g., thesetTimeoutat line 939), the closure-captured data could become stale relative to the store.This is a pre-existing pattern and low risk, but for consistency you could consider passing project data explicitly:
♻️ Optional: Fetch fresh project data in async handlers
const prepareProjectForCompile = () => { - return preprocessPous(projectData, isCurrentBoardSimulator, (level, message) => + const { project } = useOpenPLCStore.getState() + return preprocessPous(project.data, isCurrentBoardSimulator, (level, message) => addLog({ id: crypto.randomUUID(), level, message }), ) }Alternatively, fetch project data at the call site and pass it:
const prepareProjectForCompile = (data: PLCProjectData) => { return preprocessPous(data, isCurrentBoardSimulator, (level, message) => addLog({ id: crypto.randomUUID(), level, message }), ) }🤖 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` at line 895, The closure-captured projectData used by prepareProjectForCompile can become stale in long-running async flows (e.g., handleMd5Verification); update prepareProjectForCompile to accept PLCProjectData as an explicit argument and change callers (including the call inside handleMd5Verification) to fetch the latest projectData from the store before invoking it so the function uses fresh data; reference prepareProjectForCompile and handleMd5Verification to locate and update the implementation and call sites, and preserve the existing addLog callback behavior when you refactor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/renderer/components/_organisms/workspace-activity-bar/default.tsx`:
- Line 895: The closure-captured projectData used by prepareProjectForCompile
can become stale in long-running async flows (e.g., handleMd5Verification);
update prepareProjectForCompile to accept PLCProjectData as an explicit argument
and change callers (including the call inside handleMd5Verification) to fetch
the latest projectData from the store before invoking it so the function uses
fresh data; reference prepareProjectForCompile and handleMd5Verification to
locate and update the implementation and call sites, and preserve the existing
addLog callback behavior when you refactor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 696005ce-c5cd-4275-8df1-10f743ba5616
📒 Files selected for processing (1)
src/renderer/components/_organisms/workspace-activity-bar/default.tsx
1 participant
Summary
Fix the debugger upload flow so C/C++ function blocks are preprocessed consistently.
Problem
When debugger MD5 verification failed and the user chose to upload the current project, the UI invoked
runCompileProgramwith raw project data instead of preprocessed project data.This caused C/C++ function block source to pass through the IEC ST/XML pipeline as plain text during the second compilation path, producing different generated sources and causing
iec2cto fail.Fix
Apply
preprocessPous()before callingrunCompileProgramfrom the debugger mismatch upload flow, and stop early if validation fails.Validation
Summary by CodeRabbit
Bug Fixes
Refactor