Skip to content

fix: show online debug values for structured data types and FB instances#614

Merged
thiagoralves merged 3 commits into
developmentfrom
fix/591-debug-online-values-structured-types
Feb 18, 2026
Merged

fix: show online debug values for structured data types and FB instances#614
thiagoralves merged 3 commits into
developmentfrom
fix/591-debug-online-values-structured-types

Conversation

@thiagoralves
Copy link
Copy Markdown
Contributor

@thiagoralves thiagoralves commented Feb 18, 2026

Summary

Fixes #591 — Online values not visible for structured data types and FB instances in textual (ST/IL) programs.

  • Extend variable reclassification to all POUs on project load. Previously only graphical (LD/FBD) POUs had their variables re-parsed with full project context, so FB instances in ST/IL programs stayed misclassified as user-data-type instead of derived.
  • Expand top-level user-data-type variables in debug polling so structs and any unresolved FB instances get their leaf fields registered for online value polling.
  • Use fallback path resolution for struct fields in the debug tree builder. The xml2st compiler converts structs to FBs, so debug.c may use FB-style paths (no .value.) instead of struct-style paths (.value.). The tree builder now tries both.

Test plan

  • Open the Debug project, compile, and start debugging
  • Verify Structure.Parameter1, .Parameter2, .Parameter3 show online values
  • Verify fbAddition.Par1, .Par2, .Par3, .RetVal, .LocalVar show online values
  • Open the Irrigation Full Test project and verify it still works correctly (regression check for graphical POUs)
  • Run npm run test — all 76 tests pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved debugging of user-defined type variables to expose inner fields and nested variables.
    • More accurate workspace-wide variable classification so visualizations reflect reclassified variables.
    • Enhanced debug-tree traversal to resolve variable paths across different data-structure formats.
    • Minor fix to variable-panel lookup logic to ensure consistent presence checks.

…ces (#591)

Fix three issues preventing debug values from displaying for struct and
FB instance variables in textual (ST/IL) programs:

1. Extend variable reclassification to all POUs on project load, not just
   graphical (LD/FBD). The text parser lacks project context to correctly
   classify FB instances as 'derived', so re-parsing with full context is
   needed for ST/IL programs too.

2. Expand top-level user-data-type variables in debug polling so structs
   and any unresolved FB instances get their leaf fields registered for
   value polling.

3. Use fallback path resolution for struct fields in the debug tree builder.
   The xml2st compiler converts structs to FBs, so debug.c may use FB-style
   paths (no .value.) instead of struct-style paths (.value.). Try both.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 18, 2026

Warning

Rate limit exceeded

@thiagoralves has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 39 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

Walkthrough

Adds top-level user-data-type (UDT) variable processing in the workspace screen, performs full-project variable reclassification before graphical-node sync in the store, and implements dual-path debug-field resolution (FB-style vs struct-style) in debug-tree traversal to improve variable debug mapping and online visibility. (<=50 words)

Changes

Cohort / File(s) Summary
UDT Variable Processing
src/renderer/screens/workspace-screen.tsx
Adds handling for top‑level UDT variables: detects FB vs struct types, derives inner variables (ensures ENO), preserves struct field types, and recursively processes nested variables with debug prefixes.
Variable Reclassification & Sync
src/renderer/store/slices/shared/index.ts
Performs full‑context reclassification of all POUs (re-parsing with pous, dataTypes, libraries), stores reclassDataTypes/reclassLibraries, runs this step before graphical POU node synchronization so nodes use reclassified variables.
Debug Path Resolution
src/utils/debug-tree-traversal.ts
Implements helper-based debug-variable lookup and dual-path resolution: prefers FB-style path when a debug variable exists, otherwise falls back to struct-style path; applies to UDT fields and arrays and passes matched path/index to leaf visitor.
UI Variable Panel Minor
src/renderer/components/_molecules/variables-panel/index.tsx
Replaces nullish coalescing check with logical OR for debugVariableIndexes membership checks (behavior preserved).

Sequence Diagram(s)

sequenceDiagram
    participant Workspace as WorkspaceScreen
    participant Store as SharedStore
    participant Parser as IECParser
    participant Debug as DebugTraversal
    participant UI as GraphicalPOUNodes

    Workspace->>Store: Trigger full-context reclassification
    Store->>Parser: Re-parse POUs with pous, dataTypes, libraries
    Parser-->>Store: Reclassified variables, dataTypes, libraries
    Store->>UI: Sync graphical POU nodes using reclassified variables
    Workspace->>Debug: Process top-level UDTs (derive inner vars, ensure ENO)
    Debug->>Debug: Resolve field paths (FB-style preferred, else struct-style)
    Debug-->>Workspace: Mapped debug-variable entries for UI monitoring
    UI-->>Workspace: Visualize updated variable tree and online values
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • MarcosGAC
  • JoaoGSP
  • Gustavohsdp

Poem

🐰 I dug through structs and FB nests,
Chased ENO lights and debug quests,
Paths now choose the route that's true,
FB or struct — I'll find you! 🥕✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main objective: fixing online debug values for structured data types and FB instances.
Description check ✅ Passed The description clearly explains the issue, the solution approach, and includes a comprehensive test plan, though it lacks some DOD checklist items.
Linked Issues check ✅ Passed The PR directly addresses issue #591 by implementing all three required fixes: variable reclassification for ST/IL POUs, expansion of top-level user-data-type variables in debug polling, and fallback path resolution for struct fields.
Out of Scope Changes check ✅ Passed All changes in the four modified files are directly aligned with the stated objectives to fix online debug values for structured data types and FB instances.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/591-debug-online-values-structured-types

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/utils/debug-tree-traversal.ts (1)

227-258: Consider extracting the duplicated FB-vs-struct path resolution into a helper.

Lines 228–235 (for user-data-type fields) and 251–258 (for array fields) contain identical path-resolution logic: build both paths, scan debugVariables for an FB-style prefix match, and pick the winner. A small helper (similar to findDebugVariableForField for base-type fields) would reduce duplication and make future path-style changes a single-point edit.

♻️ Example helper
+function resolveFieldPath(
+  debugVariables: DebugVariableEntry[],
+  parentPath: string,
+  fieldName: string,
+): string {
+  const fbStylePath = `${parentPath}.${fieldName.toUpperCase()}`
+  const structStylePath = `${parentPath}.value.${fieldName.toUpperCase()}`
+  const hasFbMatch = debugVariables.some(
+    (dv) =>
+      dv.name.toUpperCase().startsWith(fbStylePath.toUpperCase() + '.') ||
+      dv.name.toUpperCase() === fbStylePath.toUpperCase(),
+  )
+  return hasFbMatch ? fbStylePath : structStylePath
+}

Then both sites become:

-        const fbStylePath = `${fullPath}.${field.name.toUpperCase()}`
-        const structStylePath = `${fullPath}.value.${field.name.toUpperCase()}`
-        const hasFbMatch = debugVariables.some(...)
-        const fieldFullPath = hasFbMatch ? fbStylePath : structStylePath
+        const fieldFullPath = resolveFieldPath(debugVariables, fullPath, field.name)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/debug-tree-traversal.ts` around lines 227 - 258, Extract the
duplicated FB-vs-struct path resolution (building fbStylePath and
structStylePath, scanning debugVariables for a startsWith or exact match, and
choosing the winner) into a single helper (e.g., resolveFbOrStructPath or
findDebugVariablePath) and call it from both the user-data-type branch that
invokes traverseNestedNode and the array branch; follow the existing pattern
used by findDebugVariableForField for base-type fields so the helper accepts
(fullPath, field.name, debugVariables) and returns the selected fieldFullPath to
replace the duplicated blocks.
🤖 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/store/slices/shared/index.ts`:
- Around line 1223-1250: Wrap the per-POU reclassification block in a try/catch
so a single failing POU won't abort handleOpenProjectRequest: inside the
pous.forEach callback around calls to generateIecVariablesToString and
parseIecStringToVariables, catch any error, log it (including pou.data.name and
error.message) and continue to the next POU; on success call
getState().projectActions.setPouVariables with the reparsedVariables as before,
using reclassDataTypes and reclassLibraries from the outer scope.

---

Nitpick comments:
In `@src/utils/debug-tree-traversal.ts`:
- Around line 227-258: Extract the duplicated FB-vs-struct path resolution
(building fbStylePath and structStylePath, scanning debugVariables for a
startsWith or exact match, and choosing the winner) into a single helper (e.g.,
resolveFbOrStructPath or findDebugVariablePath) and call it from both the
user-data-type branch that invokes traverseNestedNode and the array branch;
follow the existing pattern used by findDebugVariableForField for base-type
fields so the helper accepts (fullPath, field.name, debugVariables) and returns
the selected fieldFullPath to replace the duplicated blocks.

Comment thread src/renderer/store/slices/shared/index.ts
thiagoralves and others added 2 commits February 18, 2026 09:54
…tree

The `canForceVariable` function used nullish coalescing (`??`) instead of
logical OR (`||`) for its fallback checks. Since `Map.has()` returns a
boolean (not null/undefined), `false ?? nextCheck` returns `false` without
evaluating `nextCheck`. This prevented the compositeKey lookup from ever
running when the fullPath lookup failed.

For struct field nodes in the debug tree, `debugIndex` may not be set by
the tree builder, so the function needs the compositeKey fallback to find
the variable in `debugVariableIndexes` (populated by the polling setup).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wrap the reclassification loop in a try/catch so that a single malformed
POU doesn't abort the entire project load. If reclassification fails for
one POU, it keeps its original type classifications and the rest of the
project loads normally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Online values are not visible for structured data types and FB instances

1 participant