refactor: Unify debugger and OPC-UA variable index resolution by thiagoralves · Pull Request #545 · Autonomy-Logic/openplc-editor

thiagoralves · 2026-01-19T21:48:16Z

Summary

Consolidate fallback index resolution logic into shared utilities in debug-variable-finder.ts
Ensure both debugger and OPC-UA systems use identical strategies for finding variable indexes
Add canonical debug parsing module (debug-parser.ts) and tree traversal utilities (debug-tree-traversal.ts)
Remove duplicate code from renderer-specific utilities

Test plan

Verify debugger can resolve indexes for base types, FBs, structs, arrays
Verify OPC-UA can resolve indexes for same variable types
Test nested FB/struct scenarios where type could be either FB or struct
Confirm force variable functionality works for all variable types

🤖 Generated with Claude Code

The autocomplete component in the Ladder editor was not properly handling the Enter key when navigating to "Add variable" option with arrow keys. Root cause: A race condition occurred where the autocomplete component would unmount (due to onFocusOutside triggering closeModal) before the async useEffect chain could process the Enter key press. Solution: - Added triggerSubmit() method to autocomplete ref for synchronous submission - Updated Ladder components (variable, contact, coil) to call triggerSubmit() directly on Enter key press, bypassing the async keyPressed prop flow - This ensures the submit action completes before the blur/unmount occurs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Comprehensive documentation explaining: - Problem statement and current code duplication - Analysis of debug path formats in debug.c - Current architecture of debugger and OPC-UA systems - Detailed 6-phase refactoring plan: 1. Consolidate debug file parsing 2. Unify path building 3. Unify variable index lookup 4. Unify tree building with visitor pattern 5. Update debugger polling 6. Remove deprecated code after verification - Migration strategy with backwards compatibility - Testing and rollback plans Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

fix: Resolve keyboard navigation for Add Variable in Ladder editor

Add Slave ID (Unit ID) configuration for Modbus TCP devices to support communication through Modbus TCP gateways that forward messages to RS485 serial networks. The Slave ID is included in the Modbus TCP frame and used by gateways to address specific devices on the RS485 bus. Changes: - Add slaveId field to ModbusTcpConfigSchema (0-255, optional, default 1) - Add Slave ID input field in Remote Device editor UI - Update store slice to handle slaveId configuration - Include slave_id in generated Modbus master JSON configuration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

When a project is loaded, deviceDefinitions.configuration.runtimeIpAddress was restored from the saved project, but runtimeConnection.ipAddress (ephemeral state) remained null. This caused the debugger and PLC logs polling to fail with "No runtime IP address configured" error. The fix syncs runtimeIpAddress to runtimeConnection.ipAddress when setDeviceDefinitions is called during project load. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

feat: Add Slave ID support for Modbus TCP remote devices

…shared-utilities

Consolidate the fallback index resolution logic used by both the debugger and OPC-UA systems into shared utilities. This ensures consistent behavior when resolving variable indexes for user-defined types (which can be either function blocks or structures). Key changes: - Add findDebugVariableWithFallback() to debug-variable-finder.ts - Add getIndexFromMapWithFallback() for polling map lookups - Update resolve-indices.ts to use shared fallback functions - Update debug-tree-traversal.ts to use shared fallback for FB fields - Update workspace-activity-bar to use findVariableIndexWithFallback() - Update workspace-screen.tsx polling to use shared fallback lookups - Extract debug-parser.ts with canonical parsing logic - Remove duplicate parsing logic from parse-debug-file.ts - Remove duplicate logic from debug-tree-builder.ts The unified fallback strategy (FB-style path first, then struct-style path) ensures that if one system can find a variable's index, the other will too. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

coderabbitai · 2026-01-19T21:48:21Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

development

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copilot

Pull request overview

This PR consolidates debug variable index resolution logic to eliminate duplication between the debugger and OPC-UA subsystems, ensuring consistent behavior through shared utilities.

Changes:

Extracted debug.c parsing and path resolution into canonical shared modules (debug-parser.ts, debug-tree-traversal.ts)
Unified fallback resolution strategy for FB vs. struct paths in both debugger and OPC-UA systems
Added Modbus TCP slave ID configuration support
Fixed race condition in ladder editor autocomplete submission

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/utils/debug-parser.ts	New canonical debug.c parser module
src/utils/debug-tree-traversal.ts	New visitor-pattern tree traversal for complex PLC types
src/utils/debug-variable-finder.ts	Added fallback resolution functions and path utilities
src/utils/opcua/resolve-indices.ts	Updated to use shared fallback resolution
src/renderer/utils/parse-debug-file.ts	Simplified to re-export from canonical parser
src/renderer/utils/debug-tree-builder.ts	Refactored to use shared traversal utilities
src/renderer/screens/workspace-screen.tsx	Updated polling code to use shared path resolution
src/renderer/components/_organisms/workspace-activity-bar/default.tsx	Updated to use shared index lookup functions
src/types/PLC/open-plc.ts	Added slaveId field to Modbus TCP config schema
src/utils/modbus/generate-modbus-master-config.ts	Added slave_id to generated Modbus config
src/renderer/store/slices/project/types.ts	Added slaveId to remote device config schema
src/renderer/store/slices/project/slice.ts	Added slaveId handling in updateRemoteDeviceConfig
src/renderer/store/slices/device/slice.ts	Added runtimeIpAddress sync to runtimeConnection
src/renderer/components/_features/[workspace]/editor/device/remote-device/index.tsx	Added Slave ID input field
src/renderer/components/_atoms/graphical-editor/ladder/variable.tsx	Fixed autocomplete Enter key race condition
src/renderer/components/_atoms/graphical-editor/ladder/contact.tsx	Fixed autocomplete Enter key race condition
src/renderer/components/_atoms/graphical-editor/ladder/coil.tsx	Fixed autocomplete Enter key race condition
src/renderer/components/_atoms/graphical-editor/autocomplete/index.tsx	Added triggerSubmit method to prevent race condition
src/main/modules/ipc/main.ts	Removed debug logging statement
docs/debugger-opcua-shared-utilities.md	Added comprehensive refactoring documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T21:50:10Z

+          if (selectedVariable.positionInArray === -1) {
+            const addVariableOption = selectableValues.find((item) => item.type === 'add')
+            if (addVariableOption) {
+              submitAutocompletion({ variable: addVariableOption.variable })
+            } else {
+              closeModal()
+            }
+          } else {
+            submitAutocompletion({ variable: selectedVariable.variable })


The triggerSubmit function duplicates submission logic that exists elsewhere in the component. When selectedVariable.positionInArray !== -1, line 109 calls submitAutocompletion directly with selectedVariable.variable, but this logic should be verified against the existing submission handler to ensure consistent behavior (e.g., handling of edge cases like empty variable names).

Suggested change

if (selectedVariable.positionInArray === -1) {

const addVariableOption = selectableValues.find((item) => item.type === 'add')

if (addVariableOption) {

submitAutocompletion({ variable: addVariableOption.variable })

} else {

closeModal()

}

} else {

submitAutocompletion({ variable: selectedVariable.variable })

let variableToSubmit: { id: string; name: string } | null = null

if (selectedVariable.positionInArray === -1) {

const addVariableOption = selectableValues.find((item) => item.type === 'add')

if (addVariableOption && addVariableOption.variable.name.trim() !== '') {

variableToSubmit = addVariableOption.variable

}

} else if (selectedVariable.variable.name.trim() !== '') {

variableToSubmit = selectedVariable.variable

}

if (variableToSubmit) {

submitAutocompletion({ variable: variableToSubmit })

} else {

closeModal()

Copilot · 2026-01-19T21:50:10Z

+        },
      }
-    }, [selectedVariable, popoverRef, autocompleteFocus])
+    }, [selectedVariable, selectableValues, popoverRef, autocompleteFocus])


The useMemo dependency array includes submitAutocompletion and closeModal indirectly (they're called inside triggerSubmit), but these functions aren't listed as dependencies. If submitAutocompletion or closeModal are recreated on every render, the triggerSubmit closure will use stale references. Add submitAutocompletion and closeModal to the dependency array.

Suggested change

}, [selectedVariable, selectableValues, popoverRef, autocompleteFocus])

}, [selectedVariable, selectableValues, popoverRef, autocompleteFocus, submitAutocompletion, closeModal])

Copilot · 2026-01-19T21:50:11Z

+    const module = require('@root/renderer/data/library/standard-function-blocks') as {
+      StandardFunctionBlocks: { pous: Array<{ name: string; type: string }> }
+    }
+    const isStandard = module.StandardFunctionBlocks.pous.some(
+      (pou) => pou.name.toUpperCase() === typeNameUpper && normalizeTypeString(pou.type) === 'functionblock',
+    )
+    if (isStandard) return true


Using require() for a dynamic import in a try-catch block is intended to handle main process vs. renderer process differences, but the type assertion assumes a specific module structure. If the module structure changes, this will fail silently. Consider adding runtime validation of the required properties before using them.

Suggested change

const module = require('@root/renderer/data/library/standard-function-blocks') as {

StandardFunctionBlocks: { pous: Array<{ name: string; type: string }> }

}

const isStandard = module.StandardFunctionBlocks.pous.some(

(pou) => pou.name.toUpperCase() === typeNameUpper && normalizeTypeString(pou.type) === 'functionblock',

)

if (isStandard) return true

const module: unknown = require('@root/renderer/data/library/standard-function-blocks')

// Runtime validation of module structure before use

if (

module &&

typeof module === 'object' &&

'StandardFunctionBlocks' in module &&

module.StandardFunctionBlocks &&

typeof module.StandardFunctionBlocks === 'object' &&

'pous' in module.StandardFunctionBlocks &&

Array.isArray(module.StandardFunctionBlocks.pous)

) {

const isStandard = module.StandardFunctionBlocks.pous.some(

(pou) =>

pou &&

typeof pou === 'object' &&

typeof pou.name === 'string' &&

typeof pou.type === 'string' &&

pou.name.toUpperCase() === typeNameUpper &&

normalizeTypeString(pou.type) === 'functionblock',

)

if (isStandard) return true

}

Move closeModal and submitAutocompletion declarations before useImperativeHandle and add them to the dependency array to ensure the triggerSubmit closure always captures the latest function references. Addresses Copilot review feedback on PR #545. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

JoaoGSP and others added 9 commits January 15, 2026 15:59

Merge pull request #533 from Autonomy-Logic/fix-keyboard-functionality

6d46255

fix: Resolve keyboard navigation for Add Variable in Ladder editor

Merge pull request #543 from Autonomy-Logic/feat/modbus-tcp-slave-id

a1c18e1

feat: Add Slave ID support for Modbus TCP remote devices

Merge branch 'development' into feat/debugger-shared-utilities

0199f69

Merge remote-tracking branch 'origin/development' into feat/debugger-…

44b44e7

…shared-utilities

thiagoralves requested a review from Copilot January 19, 2026 21:48

Copilot AI reviewed Jan 19, 2026

View reviewed changes

thiagoralves merged commit 8f375ad into feat/opcua-server-configuration Jan 19, 2026
1 check passed

thiagoralves deleted the feat/debugger-shared-utilities branch January 19, 2026 21:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Unify debugger and OPC-UA variable index resolution#545

refactor: Unify debugger and OPC-UA variable index resolution#545
thiagoralves merged 10 commits into
feat/opcua-server-configurationfrom
feat/debugger-shared-utilities

thiagoralves commented Jan 19, 2026

Uh oh!

coderabbitai Bot commented Jan 19, 2026 •

edited

Loading

Review skipped

Other AI code review bot(s) detected

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	}, [selectedVariable, selectableValues, popoverRef, autocompleteFocus])
	}, [selectedVariable, selectableValues, popoverRef, autocompleteFocus, submitAutocompletion, closeModal])

-    const module = require('@root/renderer/data/library/standard-function-blocks') as {
-      StandardFunctionBlocks: { pous: Array<{ name: string; type: string }> }
-    }
-    const isStandard = module.StandardFunctionBlocks.pous.some(
-      (pou) => pou.name.toUpperCase() === typeNameUpper && normalizeTypeString(pou.type) === 'functionblock',
-    )
-    if (isStandard) return true
+    const module: unknown = require('@root/renderer/data/library/standard-function-blocks')
+    // Runtime validation of module structure before use
+    if (
+      module &&
+      typeof module === 'object' &&
+      'StandardFunctionBlocks' in module &&
+      module.StandardFunctionBlocks &&
+      typeof module.StandardFunctionBlocks === 'object' &&
+      'pous' in module.StandardFunctionBlocks &&
+      Array.isArray(module.StandardFunctionBlocks.pous)
+    ) {
+      const isStandard = module.StandardFunctionBlocks.pous.some(
+        (pou) =>
+          pou &&
+          typeof pou === 'object' &&
+          typeof pou.name === 'string' &&
+          typeof pou.type === 'string' &&
+          pou.name.toUpperCase() === typeNameUpper &&
+          normalizeTypeString(pou.type) === 'functionblock',
+      )
+      if (isStandard) return true
+    }

Conversation

thiagoralves commented Jan 19, 2026

Summary

Test plan

Uh oh!

coderabbitai Bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Other AI code review bot(s) detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Jan 19, 2026 •

edited

Loading