-
Notifications
You must be signed in to change notification settings - Fork 62
fix: Global variables support - types and debugger mapping #520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,195 @@ | ||||||||||
| import { DimensionsModal } from '@root/renderer/components/_atoms/dimensions-modal' | ||||||||||
| import { toast } from '@root/renderer/components/_features/[app]/toast/use-toast' | ||||||||||
| import { useOpenPLCStore } from '@root/renderer/store' | ||||||||||
| import { arrayValidation } from '@root/renderer/store/slices/workspace/utils/variables' | ||||||||||
| import { BaseType, baseTypeSchema } from '@root/types/PLC/open-plc' | ||||||||||
| import { useEffect, useState } from 'react' | ||||||||||
|
|
||||||||||
| type ArrayModalProps = { | ||||||||||
| variableName: string | ||||||||||
| variableRow?: number | ||||||||||
| arrayModalIsOpen: boolean | ||||||||||
| setArrayModalIsOpen: (value: boolean) => void | ||||||||||
| closeContainer: () => void | ||||||||||
| } | ||||||||||
|
|
||||||||||
| type Pou = { type: string; name: string } | ||||||||||
| type UserLibWithPous = { pous: Pou[] } | ||||||||||
| type UserLibFunctionBlock = { type: string; name: string } | ||||||||||
|
|
||||||||||
| export const GlobalArrayModal = ({ | ||||||||||
| arrayModalIsOpen, | ||||||||||
| closeContainer, | ||||||||||
| setArrayModalIsOpen, | ||||||||||
| variableName, | ||||||||||
| variableRow, | ||||||||||
| }: ArrayModalProps) => { | ||||||||||
| const { | ||||||||||
| project: { | ||||||||||
| data: { | ||||||||||
| dataTypes, | ||||||||||
| configuration: { | ||||||||||
| resource: { globalVariables }, | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| }, | ||||||||||
| projectActions: { updateVariable }, | ||||||||||
| libraries: sliceLibraries, | ||||||||||
| } = useOpenPLCStore() | ||||||||||
|
|
||||||||||
| const baseTypes = baseTypeSchema.options.filter((type) => type.toUpperCase() !== 'ARRAY') | ||||||||||
|
|
||||||||||
| const userDataTypes = dataTypes.map((type) => type.name).filter((typeName) => typeName.toUpperCase() !== 'ARRAY') | ||||||||||
|
|
||||||||||
| const systemFunctionBlocks = sliceLibraries.system.flatMap((lib) => | ||||||||||
| lib.pous.filter((pou) => pou.type === 'function-block').map((pou) => pou.name.toUpperCase()), | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| const userFunctionBlocks = sliceLibraries.user.flatMap((userLib: UserLibWithPous | UserLibFunctionBlock) => | ||||||||||
| 'pous' in userLib && Array.isArray(userLib.pous) | ||||||||||
| ? userLib.pous.filter((pou) => pou.type === 'function-block').map((pou) => pou.name.toUpperCase()) | ||||||||||
| : (userLib as UserLibFunctionBlock).type === 'function-block' | ||||||||||
| ? [(userLib as UserLibFunctionBlock).name.toUpperCase()] | ||||||||||
| : [], | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| const VariableTypes = [ | ||||||||||
| { definition: 'base-type', values: baseTypes }, | ||||||||||
| { definition: 'user-data-type', values: userDataTypes }, | ||||||||||
| ] | ||||||||||
|
|
||||||||||
| const LibraryTypes = [ | ||||||||||
| { definition: 'system', values: systemFunctionBlocks }, | ||||||||||
| { definition: 'user', values: userFunctionBlocks }, | ||||||||||
| ] | ||||||||||
|
|
||||||||||
| const [selectedInput, setSelectedInput] = useState<string>('') | ||||||||||
| const [dimensions, setDimensions] = useState<string[]>([]) | ||||||||||
| const [typeValue, setTypeValue] = useState<string>('dint') | ||||||||||
|
|
||||||||||
| useEffect(() => { | ||||||||||
| const variable = globalVariables.find((variable) => variable.name === variableName) | ||||||||||
| if (!variable) return | ||||||||||
|
|
||||||||||
| if (variable.type.definition === 'array') { | ||||||||||
| setDimensions(variable.type.data.dimensions.map((dimension) => dimension.dimension)) | ||||||||||
| setTypeValue(variable.type.data.baseType.value) | ||||||||||
| } else { | ||||||||||
| setDimensions([]) | ||||||||||
| setTypeValue('dint') | ||||||||||
| } | ||||||||||
| }, [variableName, globalVariables]) | ||||||||||
|
|
||||||||||
| const handleAddDimension = () => { | ||||||||||
| setDimensions((prev) => [...prev, '']) | ||||||||||
| setSelectedInput(dimensions.length.toString()) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const handleRemoveDimension = (index: string) => { | ||||||||||
| setDimensions((prev) => [...prev.slice(0, Number(index)), ...prev.slice(Number(index) + 1)]) | ||||||||||
| setSelectedInput('') | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const handleRearrangeDimensions = (index: number, direction: 'up' | 'down') => { | ||||||||||
| if (direction === 'up') { | ||||||||||
| if (index === 0) return | ||||||||||
| const newDimensions = [...dimensions] | ||||||||||
| const [removed] = newDimensions.splice(index, 1) | ||||||||||
| newDimensions.splice(index - 1, 0, removed) | ||||||||||
| setDimensions(newDimensions) | ||||||||||
| setSelectedInput((index - 1).toString()) | ||||||||||
| return | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (index === dimensions.length - 1) return | ||||||||||
| const newDimensions = [...dimensions] | ||||||||||
| const [removed] = newDimensions.splice(index, 1) | ||||||||||
| newDimensions.splice(index + 1, 0, removed) | ||||||||||
| setDimensions(newDimensions) | ||||||||||
| setSelectedInput((index + 1).toString()) | ||||||||||
| } | ||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||
|
|
||||||||||
| const handleUpdateType = (value: BaseType) => { | ||||||||||
| setTypeValue(value) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const handleUpdateDimension = (index: number, value: string): { ok: boolean } => { | ||||||||||
| const res = arrayValidation({ value: value }) | ||||||||||
| if (!res.ok) { | ||||||||||
| toast({ | ||||||||||
| title: res.title, | ||||||||||
| description: res.message, | ||||||||||
| variant: 'fail', | ||||||||||
| }) | ||||||||||
| return { ok: false } | ||||||||||
| } | ||||||||||
| setDimensions((prev) => [...prev.slice(0, index), value, ...prev.slice(index + 1)]) | ||||||||||
| return { ok: true } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const handleInputClick = (value: string) => { | ||||||||||
| setSelectedInput(value) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const handleSave = () => { | ||||||||||
| const dimensionToSave = dimensions.filter((value) => value !== '') | ||||||||||
| if (dimensionToSave.length === 0) { | ||||||||||
| toast({ | ||||||||||
| title: 'Invalid array', | ||||||||||
| description: 'Array must have at least one not empty dimension', | ||||||||||
| variant: 'fail', | ||||||||||
| }) | ||||||||||
| return | ||||||||||
| } | ||||||||||
| const formatArrayName = `ARRAY [${dimensionToSave.join(', ')}] OF ${typeValue?.toUpperCase()}` | ||||||||||
|
|
||||||||||
| const isBaseType = baseTypes.includes(typeValue) | ||||||||||
|
|
||||||||||
| updateVariable({ | ||||||||||
| scope: 'global', | ||||||||||
| rowId: variableRow, | ||||||||||
| data: { | ||||||||||
| type: { | ||||||||||
| definition: 'array', | ||||||||||
| value: formatArrayName, | ||||||||||
| data: { | ||||||||||
| // @ts-expect-error - This is a valid operation. This is being fixed. | ||||||||||
|
||||||||||
| // @ts-expect-error - This is a valid operation. This is being fixed. | |
| // @ts-expect-error - `type.data` in the variable schema does not currently declare a `baseType` | |
| // field, but PLC array types expect this shape at runtime. This suppression can be removed once | |
| // the TypeScript definitions for global variable `type.data` are updated to include `baseType`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π§© Analysis chain
π Script executed:
# Search for baseType definition patterns to understand how function blocks are handled
rg -n "definition.*base-type|definition.*user-data-type|definition.*derived" --type ts -B2 -A2Repository: Autonomy-Logic/openplc-editor
Length of output: 50386
π Script executed:
# Look for how LibraryTypes or function block types are used in array handling
rg -n "LibraryTypes|userFunctionBlocks|systemFunctionBlocks" --type ts src/renderer/components/_molecules/global-variables-table/ -A3 -B3Repository: Autonomy-Logic/openplc-editor
Length of output: 6674
π Script executed:
# Find the baseTypes definition to understand what types are considered base types
rg -n "baseTypes|baseTypeSchema" --type ts src/renderer/components/_molecules/global-variables-table/elements/array-modal.tsx -B5 -A5Repository: Autonomy-Logic/openplc-editor
Length of output: 1765
π Script executed:
# Check for array variable type handling in POU context (if it exists)
fd "array" --type f src/renderer/components/_molecules/ | head -20Repository: Autonomy-Logic/openplc-editor
Length of output: 318
π Script executed:
# Search for how variable types are classified in the broader codebase
rg -n "typeValue.*definition|isBaseType" --type ts -C4Repository: Autonomy-Logic/openplc-editor
Length of output: 8610
π Script executed:
# Check if there are tests or other usages that show function blocks in arrays
rg -n "derived.*function-block|function-block.*derived" --type ts -B2 -A2 | head -30Repository: Autonomy-Logic/openplc-editor
Length of output: 55
π Script executed:
# Verify the structure of LibraryTypes selection in array-modal
sed -n '180,220p' src/renderer/components/_molecules/global-variables-table/elements/array-modal.tsxRepository: Autonomy-Logic/openplc-editor
Length of output: 563
Handle function block type classification separately.
The logic classifies typeValue as either 'base-type' or 'user-data-type' based solely on whether it exists in baseTypes. However, function blocks selected from LibraryTypes (which are uppercase) will never match baseTypes (lowercase), causing them to be incorrectly assigned definition: 'user-data-type'.
Function block types should use definition: 'derived' instead. Add a check: if typeValue matches any function block from systemFunctionBlocks or userFunctionBlocks, assign 'derived'. Only unmatched non-base types should fall back to 'user-data-type' (for user-defined structs).
π€ Prompt for AI Agents
In
@src/renderer/components/_molecules/global-variables-table/elements/array-modal.tsx
around lines 146 - 160, The code currently sets the inner type definition based
only on baseTypes (isBaseType) which misclassifies function block types; update
the logic where updateVariable is called (the type.data.baseType block using
typeValue and isBaseType) to first check membership in systemFunctionBlocks or
userFunctionBlocks and, if matched, set definition to 'derived', else if
isBaseType set 'base-type', otherwise set 'user-data-type' (preserving value:
typeValue); keep existing surrounding structure (formatArrayName, variableRow,
scope: 'global') unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale closure bug in
handleAddDimension.dimensions.lengthcaptures the value at the time the handler was created, not after the state update. SincesetDimensionsis asynchronous,selectedInputwill be set to the old length rather than the new dimension's index.π Proposed fix using functional update
const handleAddDimension = () => { - setDimensions((prev) => [...prev, '']) - setSelectedInput(dimensions.length.toString()) + setDimensions((prev) => { + setSelectedInput(prev.length.toString()) + return [...prev, ''] + }) }Alternatively, use a ref or compute the new length explicitly:
const handleAddDimension = () => { + const newIndex = dimensions.length setDimensions((prev) => [...prev, '']) - setSelectedInput(dimensions.length.toString()) + setSelectedInput(newIndex.toString()) }π Committable suggestion
π€ Prompt for AI Agents