diff --git a/src/__architecture__/validate.ts b/src/__architecture__/validate.ts index 333812800..f06070d1a 100644 --- a/src/__architecture__/validate.ts +++ b/src/__architecture__/validate.ts @@ -274,6 +274,12 @@ const KNOWN_EXCEPTIONS: Record = { 'frontend/store/slices/ladder/utils/index.ts': ['components'], // Ladder slice — needs nodesBuilder + defaultCustomNodesStyles for rung creation 'frontend/store/slices/ladder/slice.ts': ['components'], + // FBD slice — the same case as the ladder slice above. Handle geometry is persisted + // inside each node rather than recomputed on render, so the slice owns it on load: it + // needs the pin-spacing constants to re-flow a block's pins after healing a project + // saved with the old two-sided VAR_IN_OUT pin. The constants describe how components + // lay pins out, so they live with the components that draw them. + 'frontend/store/slices/fbd/slice.ts': ['components'], // Device CONNECT flow (D72) — resolves RTU params from the board debug spec // via the shared `resolveDebugConnection` resolver, same as the activity bar's // debugger/post-flash paths. diff --git a/src/frontend/components/_atoms/graphical-editor/__tests__/in-out-pin-rules.test.ts b/src/frontend/components/_atoms/graphical-editor/__tests__/in-out-pin-rules.test.ts new file mode 100644 index 000000000..1e5c01204 --- /dev/null +++ b/src/frontend/components/_atoms/graphical-editor/__tests__/in-out-pin-rules.test.ts @@ -0,0 +1,190 @@ +import { + blockInputVariables, + blockOutputVariables, + IN_OUT_MARKER_WIDTH, + findOccupiedInOutPin, + inOutVariableNames, + migrateInOutSourceEdges, + stripInOutOutputHandles, +} from '../in-out-pin-rules' + +/** Irrigation_Main_Controller: State is VAR_IN_OUT, Moisture and T_Max are inputs. */ +const variables = [ + { name: 'State', class: 'inOut', type: { definition: 'user-data-type', value: 'Irrigation_State' } }, + { name: 'Moisture', class: 'input', type: { definition: 'base-type', value: 'BOOL' } }, + { name: 'T_Max', class: 'input', type: { definition: 'base-type', value: 'TIME' } }, +] + +const handle = (id: string, type: 'source' | 'target', top: number) => ({ + id, + type, + glbPosition: { x: 0, y: top }, + relPosition: { x: 0, y: top }, + style: { top }, +}) + +const blockNode = (id = 'imc') => ({ + id, + type: 'block', + position: { x: 0, y: 0 }, + data: { + variant: { name: 'Irrigation_Main_Controller', variables }, + inputHandles: [handle('State', 'target', 48), handle('Moisture', 'target', 96), handle('T_Max', 'target', 144)], + // What a project saved before the change carries: an output pin for the in-out. + outputHandles: [handle('State', 'source', 48)], + handles: [ + handle('State', 'target', 48), + handle('Moisture', 'target', 96), + handle('T_Max', 'target', 144), + handle('State', 'source', 48), + ], + outputConnector: handle('State', 'source', 48), + }, +}) + +describe('VAR_IN_OUT is a single input-side pin', () => { + it('puts an in-out parameter on the input side only', () => { + expect(blockInputVariables(variables).map((v) => v.name)).toEqual(['State', 'Moisture', 'T_Max']) + expect(blockOutputVariables(variables).map((v) => v.name)).toEqual([]) + expect([...inOutVariableNames(variables)]).toEqual(['State']) + }) + + it('leaves plain inputs and outputs alone', () => { + const ton = [ + { name: 'IN', class: 'input', type: { definition: 'base-type', value: 'BOOL' } }, + { name: 'PT', class: 'input', type: { definition: 'base-type', value: 'TIME' } }, + { name: 'Q', class: 'output', type: { definition: 'base-type', value: 'BOOL' } }, + { name: 'ET', class: 'output', type: { definition: 'base-type', value: 'TIME' } }, + ] + expect(blockInputVariables(ton).map((v) => v.name)).toEqual(['IN', 'PT']) + expect(blockOutputVariables(ton).map((v) => v.name)).toEqual(['Q', 'ET']) + expect(inOutVariableNames(ton).size).toBe(0) + }) +}) + +describe('an in-out pin accepts exactly one variable', () => { + const graph = { + nodes: [blockNode()], + edges: [{ source: 'v1', sourceHandle: 'output-variable', target: 'imc', targetHandle: 'State' }], + } + + it('rejects a second connection to an in-out pin', () => { + expect(findOccupiedInOutPin({ target: 'imc', targetHandle: 'State' }, graph)).toBe('State') + }) + + it('allows the first connection to an in-out pin', () => { + expect(findOccupiedInOutPin({ target: 'imc', targetHandle: 'State' }, { ...graph, edges: [] })).toBeUndefined() + }) + + it('does not restrict ordinary input pins', () => { + const busy = { + ...graph, + edges: [ + ...graph.edges, + { source: 'v2', sourceHandle: 'output-variable', target: 'imc', targetHandle: 'Moisture' }, + ], + } + expect(findOccupiedInOutPin({ target: 'imc', targetHandle: 'Moisture' }, busy)).toBeUndefined() + }) +}) + +describe('migrating projects saved with a two-sided in-out pin', () => { + it('re-points a wire leaving the in-out pin at whatever feeds the pin', () => { + // The Irrigation Controller's main POU: variable `State` feeds the pin, and the pin is + // read into two other blocks. + const edges = [ + { source: 'stateVar', sourceHandle: 'output-variable', target: 'imc', targetHandle: 'State' }, + { source: 'imc', sourceHandle: 'State', target: 'manualOverride', targetHandle: 'State' }, + { source: 'imc', sourceHandle: 'State', target: 'stateToNum', targetHandle: 'State' }, + ] + const result = migrateInOutSourceEdges([blockNode()], edges) + + expect(result.rewired).toBe(2) + expect(result.dropped).toBe(0) + expect(result.edges).toEqual([ + edges[0], + { source: 'stateVar', sourceHandle: 'output-variable', target: 'manualOverride', targetHandle: 'State' }, + { source: 'stateVar', sourceHandle: 'output-variable', target: 'stateToNum', targetHandle: 'State' }, + ]) + }) + + it('drops a wire whose in-out pin has nothing feeding it', () => { + const result = migrateInOutSourceEdges( + [blockNode()], + [{ source: 'imc', sourceHandle: 'State', target: 'manualOverride', targetHandle: 'State' }], + ) + expect(result).toMatchObject({ edges: [], rewired: 0, dropped: 1 }) + }) + + it('leaves a diagram without in-out pins untouched', () => { + const edges = [{ source: 'a', sourceHandle: 'Q', target: 'b', targetHandle: 'IN' }] + const plain = { ...blockNode(), data: { ...blockNode().data, variant: { name: 'TON', variables: [] } } } + expect(migrateInOutSourceEdges([plain], edges)).toEqual({ edges, rewired: 0, dropped: 0 }) + }) + + it('removes the stale in-out output pin from the saved handles', () => { + const healed = stripInOutOutputHandles(blockNode(), { connectorY: 48, connectorOffsetY: 48 }) + + expect(healed.data.outputHandles).toEqual([]) + expect(healed.data.handles?.map((h) => `${h.id}:${h.type}`)).toEqual([ + 'State:target', + 'Moisture:target', + 'T_Max:target', + ]) + expect(healed.data.outputConnector).toBeUndefined() + }) + + it('re-flows the remaining output pins so labels and pins stay aligned', () => { + const node = blockNode() + node.data.variant.variables = [ + { name: 'Q', class: 'output', type: { definition: 'base-type', value: 'BOOL' } }, + ...variables, + ] + node.data.outputHandles = [handle('State', 'source', 48), handle('Q', 'source', 96)] + node.data.outputConnector = handle('State', 'source', 48) + + const healed = stripInOutOutputHandles(node, { connectorY: 48, connectorOffsetY: 48 }) + + // `Q` was second; with the in-out gone it moves up into the first slot. + expect(healed.data.outputHandles).toEqual([{ ...handle('Q', 'source', 48), glbPosition: { x: 0, y: 0 } }]) + expect(healed.data.outputConnector?.id).toBe('Q') + }) + + it('is a no-op for a block that never had a two-sided in-out pin', () => { + const node = blockNode() + node.data.outputHandles = [] + node.data.handles = node.data.inputHandles + const healed = stripInOutOutputHandles(node, { connectorY: 48, connectorOffsetY: 48 }) + expect(healed).toBe(node) + }) +}) + +describe('block width reserves room for the ⟷ marker', () => { + // The marker only moves the width when the in-out pin is the WIDEST label and the block is + // not already at the maximum width — otherwise the marker is free. + const wideInOut = [ + { name: 'StateRef', class: 'inOut', type: { definition: 'base-type', value: 'INT' } }, + { name: 'B', class: 'input', type: { definition: 'base-type', value: 'BOOL' } }, + ] + const asPlainInput = wideInOut.map((v) => (v.class === 'inOut' ? { ...v, class: 'input' } : v)) + const variant = (vars: typeof wideInOut) => + ({ name: 'FB', type: 'function-block', variables: vars, documentation: '', extensible: false }) as never + + it('adds the marker width only for in-out pins', async () => { + const { getBlockSize } = await import('../fbd/utils/utils') + const at = { x: 0, y: 0 } + expect(getBlockSize(variant(wideInOut), at).width).toBe( + getBlockSize(variant(asPlainInput), at).width + IN_OUT_MARKER_WIDTH, + ) + }) + + it('leaves a block whose widest pin is not the in-out unchanged', async () => { + const { getBlockSize } = await import('../fbd/utils/utils') + const at = { x: 0, y: 0 } + // `Moisture` is wider than `State ⟷`, so it still sets the width. + expect(getBlockSize(variant(variables as never), at).width).toBe( + getBlockSize(variant(variables.map((v) => (v.class === 'inOut' ? { ...v, class: 'input' } : v)) as never), at) + .width, + ) + }) +}) diff --git a/src/frontend/components/_atoms/graphical-editor/block-output-debug-badges.tsx b/src/frontend/components/_atoms/graphical-editor/block-output-debug-badges.tsx index 48edbfef8..466043614 100644 --- a/src/frontend/components/_atoms/graphical-editor/block-output-debug-badges.tsx +++ b/src/frontend/components/_atoms/graphical-editor/block-output-debug-badges.tsx @@ -2,6 +2,7 @@ import { useDebugCompositeKey } from '../../../hooks/use-debug-composite-key' import { useIsDebuggerVisible } from '../../../hooks/use-debug-value' import { useIsGraphicalEditorActive } from '../../_features/[workspace]/editor/graphical/active-context' import { DebugValueBadge } from './debug-value-badge' +import { blockOutputVariables } from './in-out-pin-rules' type BlockOutputDebugBadgesProps = { blockType: string @@ -43,7 +44,9 @@ const BlockOutputDebugBadges = ({ return null } - const outputs = outputVariables.filter((v) => v.class === 'output' || v.class === 'inOut') + // A VAR_IN_OUT has no output pin, so it gets no output badge — its value is shown by the + // variable connected to its input pin, which carries the written-back value. + const outputs = blockOutputVariables(outputVariables) return ( <> diff --git a/src/frontend/components/_atoms/graphical-editor/diff/fbd-nodes.tsx b/src/frontend/components/_atoms/graphical-editor/diff/fbd-nodes.tsx index 5fec8d463..9b003d5ef 100644 --- a/src/frontend/components/_atoms/graphical-editor/diff/fbd-nodes.tsx +++ b/src/frontend/components/_atoms/graphical-editor/diff/fbd-nodes.tsx @@ -5,6 +5,7 @@ import { BlockNodeVisual } from '../fbd/block-visual' import { CommentVisual } from '../fbd/comment-visual' import { ConnectionVisual } from '../fbd/connection-visual' import { VariableVisual } from '../fbd/variable-visual' +import { blockInputVariables, blockOutputVariables } from '../in-out-pin-rules' import { DiffWrapper, renderFBDHandles } from './diff-wrapper' export function ReadOnlyFBDBlock({ data, width, height }: NodeProps) { @@ -15,8 +16,8 @@ export function ReadOnlyFBDBlock({ data, width, height }: NodeProps) { const blockName = variant?.name ?? '???' const blockType = variant?.type ?? '' const blockVars = variant?.variables ?? [] - const inputs = blockVars.filter((v) => v.class === 'input' || v.class === 'inOut').map((v) => v.name) - const outputs = blockVars.filter((v) => v.class === 'output' || v.class === 'inOut').map((v) => v.name) + const inputs = blockInputVariables(blockVars).map((v) => v.name) + const outputs = blockOutputVariables(blockVars).map((v) => v.name) const varName = (data.variable as { name?: string })?.name ?? '' const showInstanceName = blockType !== 'function' && blockType !== 'generic' && varName const w = (width as number) ?? 216 diff --git a/src/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsx b/src/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsx index 5653a69c1..240b25135 100644 --- a/src/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsx +++ b/src/frontend/components/_atoms/graphical-editor/diff/ladder-nodes.tsx @@ -2,6 +2,7 @@ import type { NodeProps } from '@xyflow/react' import type { DiffStatus } from '../../../../../middleware/shared/ports/version-control-port' import { PlaceholderNodeFilled } from '../../../../assets/icons/flow/Placeholder' +import { blockInputVariables, blockOutputVariables } from '../in-out-pin-rules' import { BlockNodeVisual } from '../ladder/block-visual' import { CoilVisual } from '../ladder/coil-visual' import { ContactVisual } from '../ladder/contact-visual' @@ -60,8 +61,8 @@ export function ReadOnlyBlock({ data, width, height }: NodeProps) { const blockName = variant?.name ?? '???' const blockType = variant?.type ?? '' const blockVars = variant?.variables ?? [] - const inputs = blockVars.filter((v) => v.class === 'input' || v.class === 'inOut').map((v) => v.name) - const outputs = blockVars.filter((v) => v.class === 'output' || v.class === 'inOut').map((v) => v.name) + const inputs = blockInputVariables(blockVars).map((v) => v.name) + const outputs = blockOutputVariables(blockVars).map((v) => v.name) const varName = (data.variable as { name?: string })?.name ?? '' const showInstanceName = blockType !== 'function' && blockType !== 'generic' && varName const w = (width as number) ?? 216 diff --git a/src/frontend/components/_atoms/graphical-editor/fbd/block.tsx b/src/frontend/components/_atoms/graphical-editor/fbd/block.tsx index fc2fe4418..582d4a3a9 100644 --- a/src/frontend/components/_atoms/graphical-editor/fbd/block.tsx +++ b/src/frontend/components/_atoms/graphical-editor/fbd/block.tsx @@ -13,8 +13,15 @@ import { HighlightedTextArea } from '../../highlighted-textarea' import { InputWithRef } from '../../input' import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '../../tooltip' import { BlockOutputDebugBadges } from '../block-output-debug-badges' +import { InOutPinMarker } from '../in-out-pin-marker' import { BlockVariant } from '../types/block' -import { getBlockDocumentation, getVariableRestrictionType } from '../utils' +import { + blockInputVariables, + blockOutputVariables, + getBlockDocumentation, + getVariableRestrictionType, + inOutVariableNames, +} from '../utils' import { buildBlockNode } from './buildNodes' import { CustomHandle } from './handle' import { BasicNodeData, BlockNodeData, BlockProps } from './utils' @@ -61,12 +68,9 @@ export const BlockNodeElement = ({ type: blockType, } = (data.variant as BlockVariant) ?? DEFAULT_BLOCK_TYPE - const inputConnectors = blockVariables - .filter((variable) => variable.class === 'input' || variable.class === 'inOut') - .map((variable) => variable.name) - const outputConnectors = blockVariables - .filter((variable) => variable.class === 'output' || variable.class === 'inOut') - .map((variable) => variable.name) + const inputConnectors = blockInputVariables(blockVariables).map((variable) => variable.name) + const outputConnectors = blockOutputVariables(blockVariables).map((variable) => variable.name) + const inOutConnectors = inOutVariableNames(blockVariables) const [blockNameValue, setBlockNameValue] = useState(blockType === 'generic' ? '' : blockName) const [validBlockNameValue, setValidBlockNameValue] = useState(blockNameValue) @@ -313,6 +317,7 @@ export const BlockNodeElement = ({ style={{ top: DEFAULT_BLOCK_CONNECTOR_Y + index * DEFAULT_BLOCK_CONNECTOR_Y_OFFSET - 10, left: 6 }} > {connector} + {inOutConnectors.has(connector) && } ))} {outputConnectors.map((connector, index) => ( @@ -643,19 +648,11 @@ const Block = (block: BlockProps) => { const newNode = { ...updatedNewNode } - const originalNodeInputs = (node.data.variant as BlockVariant).variables.filter( - (variable) => variable.class === 'input' || variable.class === 'inOut', - ) - const originalNodeSources = (node.data.variant as BlockVariant).variables.filter( - (variable) => variable.class === 'output' || variable.class === 'inOut', - ) + const originalNodeInputs = blockInputVariables((node.data.variant as BlockVariant).variables) + const originalNodeSources = blockOutputVariables((node.data.variant as BlockVariant).variables) - const updatedInputVariables = newNode.data.variant.variables.filter( - (variable) => variable.class === 'input' || variable.class === 'inOut', - ) - const updatedOutputVariables = newNode.data.variant.variables.filter( - (variable) => variable.class === 'output' || variable.class === 'inOut', - ) + const updatedInputVariables = blockInputVariables(newNode.data.variant.variables) + const updatedOutputVariables = blockOutputVariables(newNode.data.variant.variables) let newNodes = [...rung.nodes] newNodes = newNodes.map((nodeItem) => (nodeItem.id === node.id ? newNode : nodeItem)) diff --git a/src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts b/src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts index 2ca409646..3b626f9b7 100644 --- a/src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts +++ b/src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts @@ -5,6 +5,7 @@ import type { PLCVariable } from '../../../../../../middleware/shared/ports/type import type { FBDFlowType } from '../../../../../store/slices/fbd' import type { LadderFlowType } from '../../../../../store/slices/ladder' import { resolveArrayVariableByName } from '../../../../../utils/PLC/array-variable-utils' +import { blockInputVariables, blockOutputVariables, IN_OUT_MARKER_WIDTH } from '../../in-out-pin-rules' import { BlockVariant } from '../../types/block' import { customNodeTypes } from '..' import { buildHandle } from '../handle' @@ -176,12 +177,9 @@ export const getBlockSize = ( y: number }, ) => { - const inputConnectors = variant.variables - .filter((variable) => variable.class === 'input' || variable.class === 'inOut') - .map((variable) => variable.name) - const outputConnectors = variant.variables - .filter((variable) => variable.class === 'output' || variable.class === 'inOut') - .map((variable) => variable.name) + const inputVariables = blockInputVariables(variant.variables) + const inputConnectors = inputVariables.map((variable) => variable.name) + const outputConnectors = blockOutputVariables(variant.variables).map((variable) => variable.name) const blockHeight = DEFAULT_BLOCK_CONNECTOR_Y + @@ -191,8 +189,10 @@ export const getBlockSize = ( let variableInputWidth = 0 let variableOutputWidth = 0 const blockNameWidth = variant.name.length * 12 - inputConnectors.forEach((input) => { - const inputWidth = input.length * 12 + inputVariables.forEach((input) => { + // An in-out pin also renders the ⟷ marker after its name; pay for it here so a long + // name plus the arrow cannot overflow the block. + const inputWidth = input.name.length * 12 + (input.class === 'inOut' ? IN_OUT_MARKER_WIDTH : 0) if (inputWidth > variableInputWidth) variableInputWidth = inputWidth }) outputConnectors.forEach((output) => { diff --git a/src/frontend/components/_atoms/graphical-editor/in-out-pin-marker.tsx b/src/frontend/components/_atoms/graphical-editor/in-out-pin-marker.tsx new file mode 100644 index 000000000..32eb614f8 --- /dev/null +++ b/src/frontend/components/_atoms/graphical-editor/in-out-pin-marker.tsx @@ -0,0 +1,33 @@ +/** + * The ⟷ badge that marks a `VAR_IN_OUT` pin, drawn after the pin name (`State ⟷`). + * + * An in-out parameter has one pin, on the input side, so without a marker it is + * indistinguishable from a plain input. CODESYS marks it the same way, with a left-right + * arrow, which keeps the two editors readable in the same way for anyone moving between + * them. + * + * It is drawn as an SVG rather than the `⟷` character: the glyph is missing from several of + * the fonts the editors fall back to, and where it exists it sits on the baseline instead of + * beside the pin name. The arrow is `w-3` (12px) and `ml-1` (4px) — together the + * `IN_OUT_MARKER_WIDTH` that block sizing reserves, so a long in-out name plus the arrow + * cannot overflow the block. + */ +const InOutPinMarker = () => ( + + + +) + +export { InOutPinMarker } diff --git a/src/frontend/components/_atoms/graphical-editor/in-out-pin-rules.ts b/src/frontend/components/_atoms/graphical-editor/in-out-pin-rules.ts new file mode 100644 index 000000000..3fdd5e9d3 --- /dev/null +++ b/src/frontend/components/_atoms/graphical-editor/in-out-pin-rules.ts @@ -0,0 +1,214 @@ +/** + * Rules that follow from a `VAR_IN_OUT` parameter being a single, by-reference pin. + * + * Two of them matter to a diagram: + * - an in-out pin accepts exactly ONE variable, because a second wire would alias the same + * parameter twice with no defined order (CODESYS: "The 'X' pin internally contains more + * than one associated connection. This is not allowed."); + * - nothing may read the pin back out. The block writes THROUGH the reference, so the + * caller's own variable already holds the result — which is why a wire that used to leave + * an in-out pin can be re-pointed at whatever feeds the pin without changing behaviour. + */ +import type { PLCVariable } from '../../../../middleware/shared/ports/types' + +/** + * Which side of a block a parameter appears on. + * + * A `VAR_IN_OUT` parameter is a SINGLE pin on the INPUT side, the way CODESYS draws it (with a + * ⟷ marker over the pin). It used to get a pin on both sides, which let a diagram read the + * value back out of the block — something CODESYS rejects outright ("No external access to + * VAR_IN_OUT parameter"), because an in-out is passed by reference and the block's copy IS the + * caller's variable. + * + * Nothing is lost by dropping the output pin: the compiler already emits the copy-out from the + * input-side connection alone, so a call still generates + * `FB.PARAM = VAR; FB(); VAR = FB.PARAM;` exactly as before. + * + * These helpers are the single source of truth for that rule — pin geometry, pin labels, the + * generated XML and the connection checks all derive from them, so the two sides cannot drift. + */ +type BlockParameter = { name: string; class: string } + +export const blockInputVariables = (variables: T[]): T[] => + variables.filter((variable) => variable.class === 'input' || variable.class === 'inOut') + +export const blockOutputVariables = (variables: T[]): T[] => + variables.filter((variable) => variable.class === 'output') + +/** + * Horizontal space the ⟷ marker adds to an in-out pin's label, in pixels. + * + * Block width is measured from the pin labels, so the marker has to be paid for there or a + * long in-out name plus the arrow overflows the block. Keep in step with InOutPinMarker. + */ +export const IN_OUT_MARKER_WIDTH = 16 + +/** Names of a block's `VAR_IN_OUT` parameters, for the pin marker and the connection checks. */ +export const inOutVariableNames = (variables: T[]): Set => + new Set(variables.filter((variable) => variable.class === 'inOut').map((variable) => variable.name)) + +/** A pin as the editors persist it inside a node's `data`. */ +interface HandleLike { + id: string + type: string + glbPosition?: { x: number; y: number } + relPosition?: { x: number; y: number } + style?: Record +} + +/** The minimum a node has to look like for these rules to apply. */ +interface BlockLikeNode { + id: string + type?: string + position?: { x: number; y: number } + data: { + variant?: { variables?: { name: string; class: string }[] } + handles?: HandleLike[] + inputHandles?: HandleLike[] + outputHandles?: HandleLike[] + outputConnector?: HandleLike + } +} + +/** Vertical placement of a block's pins, which differs between FBD and Ladder. */ +export interface PinGeometry { + connectorY: number + connectorOffsetY: number +} +interface EdgeLike { + source: string + sourceHandle?: string | null + target: string + targetHandle?: string | null +} +interface GraphLike { + nodes: BlockLikeNode[] + edges: EdgeLike[] +} + +const inOutPinsOf = (node: BlockLikeNode | undefined): Set => + new Set( + (node?.type === 'block' ? (node.data.variant?.variables ?? []) : []) + .filter((variable) => variable.class === 'inOut') + .map((variable) => variable.name), + ) + +/** + * The in-out pin this connection would overfill, or undefined when the connection is fine. + * Returns the pin NAME so the caller can name it in the message. + */ +export const findOccupiedInOutPin = ( + connection: { target?: string | null; targetHandle?: string | null }, + graph: GraphLike, +): string | undefined => { + const { target, targetHandle } = connection + if (!target || !targetHandle) return undefined + + const targetNode = graph.nodes.find((node) => node.id === target) + if (!inOutPinsOf(targetNode).has(targetHandle)) return undefined + + const alreadyWired = graph.edges.some((edge) => edge.target === target && edge.targetHandle === targetHandle) + return alreadyWired ? targetHandle : undefined +} + +/** + * Re-point every wire that LEAVES an in-out pin at whatever feeds that pin. + * + * Projects saved before the in-out pin became input-only can contain such wires — the + * Irrigation Controller's `main` reads `Irrigation_Main_Controller.State` into two other + * blocks. The read is equivalent to reading the variable connected to the pin (the block + * wrote through the reference), so re-pointing keeps the diagram and the generated code + * behaving exactly as before instead of silently dropping the wire. + * + * A wire whose in-out pin has nothing feeding it cannot be salvaged and is dropped; the + * count comes back so the caller can say so. + */ +export const migrateInOutSourceEdges = ( + nodes: BlockLikeNode[], + edges: E[], +): { edges: E[]; rewired: number; dropped: number } => { + const inOutByNode = new Map>() + for (const node of nodes) { + const pins = inOutPinsOf(node) + if (pins.size > 0) inOutByNode.set(node.id, pins) + } + if (inOutByNode.size === 0) return { edges, rewired: 0, dropped: 0 } + + const leavesInOutPin = (edge: EdgeLike): boolean => + !!edge.sourceHandle && !!inOutByNode.get(edge.source)?.has(edge.sourceHandle) + + if (!edges.some(leavesInOutPin)) return { edges, rewired: 0, dropped: 0 } + + // What feeds each in-out pin: node id -> pin name -> the wire's source. + const feed = new Map>() + for (const edge of edges) { + if (!edge.targetHandle || !inOutByNode.get(edge.target)?.has(edge.targetHandle)) continue + const pins = feed.get(edge.target) ?? new Map() + pins.set(edge.targetHandle, { source: edge.source, sourceHandle: edge.sourceHandle }) + feed.set(edge.target, pins) + } + + let rewired = 0 + let dropped = 0 + const out: E[] = [] + for (const edge of edges) { + if (!leavesInOutPin(edge)) { + out.push(edge) + continue + } + const source = feed.get(edge.source)?.get(edge.sourceHandle as string) + if (!source) { + dropped++ + continue + } + rewired++ + out.push({ ...edge, source: source.source, sourceHandle: source.sourceHandle }) + } + return { edges: out, rewired, dropped } +} + +/** True when `variable` is declared as VAR_IN_OUT on the POU that owns the diagram. */ +export const isInOutVariable = (variable: Pick | undefined): boolean => + variable?.class === 'inOut' + +/** + * Drop the output-side pin of every in-out parameter from a block node's persisted handles. + * + * Handle geometry is saved inside the node, not recomputed on load, so a project written + * before this change still carries the in-out's right-hand pin — it would keep rendering and + * keep accepting wires. Removing it also re-flows the remaining output pins, whose vertical + * position comes from their index, so labels and pins stay aligned. + */ +export const stripInOutOutputHandles = (node: N, geometry: PinGeometry): N => { + if (node.type !== 'block') return node + const inOutPins = inOutPinsOf(node) + if (inOutPins.size === 0) return node + + const outputHandles = (node.data.outputHandles ?? []).filter((handle) => !inOutPins.has(handle.id)) + if (outputHandles.length === (node.data.outputHandles ?? []).length) return node + + const top = (index: number): number => geometry.connectorY + index * geometry.connectorOffsetY + const reflowed = outputHandles.map((handle, index) => ({ + ...handle, + glbPosition: handle.glbPosition + ? { ...handle.glbPosition, y: (node.position?.y ?? 0) + index * geometry.connectorOffsetY } + : handle.glbPosition, + relPosition: handle.relPosition ? { ...handle.relPosition, y: top(index) } : handle.relPosition, + style: handle.style ? { ...handle.style, top: top(index) } : handle.style, + })) + const inputHandles = node.data.inputHandles ?? [] + + return { + ...node, + data: { + ...node.data, + handles: [...inputHandles, ...reflowed], + outputHandles: reflowed, + // `outputConnector` is the block's primary source pin; drop it if it was the in-out. + outputConnector: + node.data.outputConnector && inOutPins.has(node.data.outputConnector.id) + ? reflowed[0] + : node.data.outputConnector, + }, + } +} diff --git a/src/frontend/components/_atoms/graphical-editor/ladder/block.tsx b/src/frontend/components/_atoms/graphical-editor/ladder/block.tsx index 0e252b784..8692626d3 100644 --- a/src/frontend/components/_atoms/graphical-editor/ladder/block.tsx +++ b/src/frontend/components/_atoms/graphical-editor/ladder/block.tsx @@ -17,8 +17,15 @@ import { HighlightedTextArea } from '../../highlighted-textarea' import { InputWithRef } from '../../input' import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '../../tooltip' import { BlockOutputDebugBadges } from '../block-output-debug-badges' +import { InOutPinMarker } from '../in-out-pin-marker' import { BlockVariant as newBlockVariant } from '../types/block' -import { getBlockDocumentation, getVariableRestrictionType } from '../utils' +import { + blockInputVariables, + blockOutputVariables, + getBlockDocumentation, + getVariableRestrictionType, + inOutVariableNames, +} from '../utils' import { buildBlockNode } from './buildNodes' import { CustomHandle } from './handle' import { getLadderPouVariablesRungNodeAndEdges } from './utils' @@ -71,12 +78,9 @@ export const BlockNodeElement = ({ type: blockType, } = (data.variant as BlockVariant) ?? DEFAULT_BLOCK_TYPE - const inputConnectors = blockVariables - .filter((variable) => variable.class === 'input' || variable.class === 'inOut') - .map((variable) => variable.name) - const outputConnectors = blockVariables - .filter((variable) => variable.class === 'output' || variable.class === 'inOut') - .map((variable) => variable.name) + const inputConnectors = blockInputVariables(blockVariables).map((variable) => variable.name) + const outputConnectors = blockOutputVariables(blockVariables).map((variable) => variable.name) + const inOutConnectors = inOutVariableNames(blockVariables) const [blockNameValue, setBlockNameValue] = useState(blockType === 'generic' ? '' : blockName) const [validBlockNameValue, setValidBlockNameValue] = useState(blockNameValue) @@ -380,6 +384,7 @@ export const BlockNodeElement = ({ return (
{connector} + {inOutConnectors.has(connector) && }
) })} diff --git a/src/frontend/components/_atoms/graphical-editor/ladder/utils/utils.ts b/src/frontend/components/_atoms/graphical-editor/ladder/utils/utils.ts index d1980888e..eb1413504 100644 --- a/src/frontend/components/_atoms/graphical-editor/ladder/utils/utils.ts +++ b/src/frontend/components/_atoms/graphical-editor/ladder/utils/utils.ts @@ -8,6 +8,7 @@ import { getVariableRestrictionType as _getVariableRestrictionType, validateVariableType as _validateVariableType, } from '../../../../../utils/PLC/validate-variable-type' +import { blockInputVariables, blockOutputVariables, IN_OUT_MARKER_WIDTH } from '../../in-out-pin-rules' import { buildHandle } from '../handle' import { DEFAULT_BLOCK_CONNECTOR_Y, DEFAULT_BLOCK_CONNECTOR_Y_OFFSET, DEFAULT_BLOCK_WIDTH } from './constants' import type { BasicNodeData, BlockVariant } from './types' @@ -121,12 +122,9 @@ export const getBlockSize = ( y: number }, ) => { - const inputConnectors = variant.variables - .filter((variable) => variable.class === 'input' || variable.class === 'inOut') - .map((variable) => variable.name) - const outputConnectors = variant.variables - .filter((variable) => variable.class === 'output' || variable.class === 'inOut') - .map((variable) => variable.name) + const inputVariables = blockInputVariables(variant.variables) + const inputConnectors = inputVariables.map((variable) => variable.name) + const outputConnectors = blockOutputVariables(variant.variables).map((variable) => variable.name) const blockHeight = DEFAULT_BLOCK_CONNECTOR_Y + @@ -136,8 +134,10 @@ export const getBlockSize = ( let variableInputWidth = 0 let variableOutputWidth = 0 const blockNameWidth = variant.name.length * 12 - inputConnectors.forEach((input) => { - const inputWidth = input.length * 12 + inputVariables.forEach((input) => { + // An in-out pin also renders the ⟷ marker after its name; pay for it here so a long + // name plus the arrow cannot overflow the block. + const inputWidth = input.name.length * 12 + (input.class === 'inOut' ? IN_OUT_MARKER_WIDTH : 0) if (inputWidth > variableInputWidth) variableInputWidth = inputWidth }) outputConnectors.forEach((output) => { @@ -196,18 +196,14 @@ export const getBlockSize = ( } export const getBlockVariantAndExecutionControl = (variantLib: BlockVariant, executionControl: boolean) => { - const inputConnectors = variantLib.variables - .filter((variable) => variable.class === 'input' || variable.class === 'inOut') - .map((variable) => ({ - name: variable.name, - type: variable.type, - })) - const outputConnectors = variantLib.variables - .filter((variable) => variable.class === 'output' || variable.class === 'inOut') - .map((variable) => ({ - name: variable.name, - type: variable.type, - })) + const inputConnectors = blockInputVariables(variantLib.variables).map((variable) => ({ + name: variable.name, + type: variable.type, + })) + const outputConnectors = blockOutputVariables(variantLib.variables).map((variable) => ({ + name: variable.name, + type: variable.type, + })) const mustHaveExecutionControlEnabled = inputConnectors.length === 0 || diff --git a/src/frontend/components/_atoms/graphical-editor/utils/index.ts b/src/frontend/components/_atoms/graphical-editor/utils/index.ts index c852a54c0..c76eda7b4 100644 --- a/src/frontend/components/_atoms/graphical-editor/utils/index.ts +++ b/src/frontend/components/_atoms/graphical-editor/utils/index.ts @@ -4,6 +4,7 @@ import { getVariableRestrictionType, validateVariableType as _validateVariableType, } from '../../../../utils/PLC/validate-variable-type' +import { blockInputVariables, blockOutputVariables } from '../in-out-pin-rules' import { BlockVariant } from '../ladder/utils/types' import { BlockVariant as newBlockVariant } from '../types/block' @@ -15,13 +16,8 @@ export const getVariableByName = (variables: PLCVariable[], name: string): PLCVa } export const getBlockDocumentation = (blockVariant: newBlockVariant): string => { - const inputVariables = blockVariant.variables.filter( - (variable) => variable.class === 'input' || variable.class === 'inOut', - ) - - const outputVariables = blockVariant.variables.filter( - (variable) => variable.class === 'output' || variable.class === 'inOut', - ) + const inputVariables = blockInputVariables(blockVariant.variables) + const outputVariables = blockOutputVariables(blockVariant.variables) const documentationString = `${blockVariant.documentation ? `${blockVariant.documentation}\n\n` : ''}INPUT: ${inputVariables @@ -52,4 +48,5 @@ export const validateVariableType = ( return _validateVariableType(selectedType, expectedType.type.value) } +export { blockInputVariables, blockOutputVariables, inOutVariableNames } from '../in-out-pin-rules' export { getVariableRestrictionType } diff --git a/src/frontend/components/_molecules/graphical-editor/fbd/index.tsx b/src/frontend/components/_molecules/graphical-editor/fbd/index.tsx index 7d53d1b7f..3f46227f7 100644 --- a/src/frontend/components/_molecules/graphical-editor/fbd/index.tsx +++ b/src/frontend/components/_molecules/graphical-editor/fbd/index.tsx @@ -33,6 +33,7 @@ import { getFunctionBlockVariablesToCleanup } from '../../../../utils/graphical/ import { newGraphicalEditorNodeID } from '../../../../utils/new-graphical-editor-node-id' import { CustomFbdNodeTypes, customNodeTypes } from '../../../_atoms/graphical-editor/fbd' import { BlockNode } from '../../../_atoms/graphical-editor/fbd/utils/types' +import { findOccupiedInOutPin } from '../../../_atoms/graphical-editor/in-out-pin-rules' import { getVariableRestrictionType } from '../../../_atoms/graphical-editor/utils' import { ReactFlowPanel } from '../../../_atoms/react-flow' import { toast } from '../../../_features/[app]/toast/use-toast' @@ -555,6 +556,22 @@ export const FBDBody = ({ rung, nodeDivergences = [], isDebuggerActive = false } * It is used to update the local rung state */ const handleOnConnect = (connection: Connection) => { + // A VAR_IN_OUT pin takes exactly one variable. It is passed by reference, so a second + // wire would mean two variables aliasing the same parameter with no defined order — + // CODESYS refuses it too ("The 'X' pin internally contains more than one associated + // connection. This is not allowed."). + const occupiedInOutPin = findOccupiedInOutPin(connection, rungLocal) + if (occupiedInOutPin) { + toast({ + title: 'Can not connect', + description: + `The '${occupiedInOutPin}' pin already has a connection. An in-out pin takes exactly ` + + 'one variable — remove the existing connection first.', + variant: 'fail', + }) + return + } + captureAndPush(pouName) setRungLocal((rung) => ({ diff --git a/src/frontend/store/slices/fbd/slice.ts b/src/frontend/store/slices/fbd/slice.ts index 4119bd7bc..b734dab02 100644 --- a/src/frontend/store/slices/fbd/slice.ts +++ b/src/frontend/store/slices/fbd/slice.ts @@ -2,8 +2,21 @@ import { addEdge } from '@xyflow/react' import { produce } from 'immer' import { StateCreator } from 'zustand' +import { + DEFAULT_BLOCK_CONNECTOR_Y, + DEFAULT_BLOCK_CONNECTOR_Y_OFFSET, +} from '../../../components/_atoms/graphical-editor/fbd/utils/constants' +import { + migrateInOutSourceEdges, + stripInOutOutputHandles, +} from '../../../components/_atoms/graphical-editor/in-out-pin-rules' import { FBDFlowSlice, FBDFlowState } from './types' +const FBD_PIN_GEOMETRY = { + connectorY: DEFAULT_BLOCK_CONNECTOR_Y, + connectorOffsetY: DEFAULT_BLOCK_CONNECTOR_Y_OFFSET, +} + export const createFBDFlowSlice: StateCreator = (setState) => ({ fbdFlows: [], @@ -15,8 +28,14 @@ export const createFBDFlowSlice: StateCreator { const flowIndex = fbdFlows.findIndex((f) => f.name === flow.name) + // A VAR_IN_OUT pin no longer has an output side. Projects saved before that carry + // both the stale right-hand pin (handle geometry lives in the node, it is not + // recomputed on load) and any wires leaving it, so heal both here. + const migrated = migrateInOutSourceEdges(flow.rung.nodes, flow.rung.edges) const rung = { ...flow.rung, + nodes: flow.rung.nodes.map((node) => stripInOutOutputHandles(node, FBD_PIN_GEOMETRY)), + edges: migrated.edges, selectedNodes: [], } // Reset updated to false on load — the flow is being loaded from a saved project. diff --git a/src/frontend/store/slices/ladder/slice.ts b/src/frontend/store/slices/ladder/slice.ts index c58259f5d..6f3465051 100644 --- a/src/frontend/store/slices/ladder/slice.ts +++ b/src/frontend/store/slices/ladder/slice.ts @@ -3,16 +3,29 @@ import { produce } from 'immer' import { StateCreator } from 'zustand' import type { PLCVariable } from '../../../../middleware/shared/ports/types' +import { + migrateInOutSourceEdges, + stripInOutOutputHandles, +} from '../../../components/_atoms/graphical-editor/in-out-pin-rules' import { defaultCustomNodesStyles, nodesBuilder, } from '../../../components/_atoms/graphical-editor/ladder/node-builders' +import { + DEFAULT_BLOCK_CONNECTOR_Y, + DEFAULT_BLOCK_CONNECTOR_Y_OFFSET, +} from '../../../components/_atoms/graphical-editor/ladder/utils/constants' import type { LadderBlockConnectedVariables } from '../../../components/_atoms/graphical-editor/ladder/utils/types' import { removeElements } from '../../../components/_molecules/graphical-editor/ladder/rung/ladder-utils/elements' import { deriveHandleBranches } from '../../../components/_molecules/graphical-editor/ladder/rung/ladder-utils/elements/handle-branch' import { LadderFlowSlice, LadderFlowState } from './types' import { duplicateLadderRung } from './utils' +const LADDER_PIN_GEOMETRY = { + connectorY: DEFAULT_BLOCK_CONNECTOR_Y, + connectorOffsetY: DEFAULT_BLOCK_CONNECTOR_Y_OFFSET, +} + export const createLadderFlowSlice: StateCreator = (setState) => ({ ladderFlows: [], @@ -25,6 +38,21 @@ export const createLadderFlowSlice: StateCreator { const flowIndex = ladderFlows.findIndex((f) => f.name === flow.name) + // A VAR_IN_OUT pin no longer has an output side. Heal projects saved before that: + // drop the stale right-hand pin (handle geometry is persisted, not recomputed) and + // re-point any wire that left it at whatever feeds the pin. + flow = { + ...flow, + rungs: flow.rungs.map((rung) => { + const migrated = migrateInOutSourceEdges(rung.nodes, rung.edges) + return { + ...rung, + nodes: rung.nodes.map((node) => stripInOutOutputHandles(node, LADDER_PIN_GEOMETRY)), + edges: migrated.edges, + } + }), + } + // Check if any block node has legacy connectedVariables (object instead of array). // Only scan + migrate if legacy data is detected — modern projects skip this entirely. const needsMigration = flow.rungs.some((rung) =>