From 614a8458f0acb2db14bba063e16af6e75f6225d1 Mon Sep 17 00:00:00 2001 From: Karstein Phobic Nyvold Kvistad Date: Wed, 29 Apr 2026 13:52:35 +0200 Subject: [PATCH] feat(live-values): depth-1 sub-property descent Extends the pump beyond top-level vars: when a var's declared type resolves to another mirror .st file, we descend one level and read each of that type's vars as .. Parser: parseVarDecls(text) now returns name + declared type per decl. ARRAY/POINTER/REFERENCE wrappers stripped to the inner type. parseVarNames is kept as a thin wrapper for the existing callers. Pump: new resolveTypeMirror(typeName, deviceRoot) dep. Pump itself doesn't know how to find a type's source -- the caller plugs in a strategy. server.ts wires a recursive walk under the device root looking for '.st'; the mirror layout guarantees stable filenames for POU/FB/DUT (every code-bearing object). Pump's deviceRootFor(absPath) parses the abs path back to the device root by locating the '/mcp-mirror//' segment. If that fails we fall back to the file's parent dir (still works for same-folder type lookups, just won't find types in sibling folders). Constructor now accepts partial deps and fills in safe defaults (no-op resolveTypeMirror -> never descend, matching v0.3 top-level-only behaviour). Existing tests don't need to change. 407/407 tests pass; new coverage: - parseVarDecls: name+type extraction, ARRAY/POINTER/REFERENCE stripping, AT %loc prefix, missing-type pathological case - LiveValuesPump.tick: descends when resolver returns content; doesn't descend when resolver returns null (primitives) --- src/live-values-pump.ts | 136 +++++++++++++++++++++++----- src/server.ts | 33 +++++++ tests/unit/live-values-pump.test.ts | 71 +++++++++++++++ tests/unit/var-block-parse.test.ts | 49 +++++++++- 4 files changed, 263 insertions(+), 26 deletions(-) diff --git a/src/live-values-pump.ts b/src/live-values-pump.ts index 437eb23..e114c82 100644 --- a/src/live-values-pump.ts +++ b/src/live-values-pump.ts @@ -5,26 +5,37 @@ import { readSelection } from './state-read'; const VAR_OPEN_RE = /^\s*(?:VAR(?:_INPUT|_OUTPUT|_IN_OUT|_GLOBAL|_TEMP|_CONFIG|_EXTERNAL|_STAT)?)\b/i; const VAR_CLOSE_RE = /^\s*END_VAR\b/i; const VAR_DECL_RE = /^\s*([A-Za-z_]\w*)\b/; +// `name [AT %loc] : ` -- captures the type name (first identifier after ':'). +// Type may be a primitive (INT/BOOL), an ARRAY, a POINTER, a REFERENCE, or a +// user-defined identifier. We only return the leaf identifier; ARRAY / POINTER +// / REFERENCE-OF wrappers don't contribute a sub-property scan. +const VAR_TYPE_RE = /:\s*(?:ARRAY\s*\[[^\]]+\]\s*OF\s+|POINTER\s+TO\s+|REFERENCE\s+TO\s+)?([A-Za-z_]\w*)/i; + +export interface VarDecl { + name: string; + /** Declared type as written. null if we couldn't extract a type identifier. */ + type: string | null; +} /** - * Parse all variable names declared in any VAR / VAR_INPUT / VAR_OUTPUT / - * VAR_GLOBAL / etc. block. One name per declaration line; skips lines that - * are entirely inside (* ... *) blocks (joined across lines) or are - * `// ...` comments. + * Parse all variable declarations in any VAR / VAR_INPUT / VAR_OUTPUT / + * VAR_GLOBAL / etc. block. Returns name + declared type per decl. Skips + * (* ... *) comments (joined across lines) and `// ...` line comments. * - * Recognises the IEC declaration grammar enough for typical PLC code: - * `name [AT %loc] : type [:= init];`. Doesn't try to parse multi-name - * shorthand (`a, b : INT;`) — that's vanishingly rare in practice and - * the cost of getting it wrong is just a missing overlay. + * Recognises `name [AT %loc] : [ARRAY [...] OF | POINTER TO | REFERENCE TO] + * [:= init];`. Wrapper modifiers (ARRAY/POINTER/REFERENCE) are + * stripped — the inner type is what we'd descend into for sub-property + * paths, and a containing ARRAY of struct doesn't get pretty-printed in + * v0.3+ anyway. Multi-name shorthand 'a, b : INT;' yields only the first + * name (vanishingly rare in practice). */ -export function parseVarNames(text: string): string[] { - const names: string[] = []; +export function parseVarDecls(text: string): VarDecl[] { + const decls: VarDecl[] = []; const lines = text.split(/\r?\n/); let inVarBlock = false; let inMultilineComment = false; - for (let raw of lines) { - // Strip block comments first, threading the open-state across lines. + for (const raw of lines) { let scrubbed = ''; let i = 0; while (i < raw.length) { @@ -51,7 +62,6 @@ export function parseVarNames(text: string): string[] { scrubbed += raw[i]; i++; } - // Strip line comments. const slash = scrubbed.indexOf('//'); if (slash >= 0) scrubbed = scrubbed.slice(0, slash); @@ -63,10 +73,17 @@ export function parseVarNames(text: string): string[] { inVarBlock = false; continue; } - const m = VAR_DECL_RE.exec(scrubbed); - if (m) names.push(m[1]); + const nameM = VAR_DECL_RE.exec(scrubbed); + if (!nameM) continue; + const typeM = VAR_TYPE_RE.exec(scrubbed); + decls.push({ name: nameM[1], type: typeM ? typeM[1] : null }); } - return names; + return decls; +} + +/** Backward-compat: name-only view used by callers that don't need types yet. */ +export function parseVarNames(text: string): string[] { + return parseVarDecls(text).map((d) => d.name); } // ─── Pump ──────────────────────────────────────────────────────────────── @@ -81,6 +98,13 @@ export interface PumpDeps { readSelection?: typeof readSelection; /** Read the POU's mirror file from disk. */ readPouFile?: (absPath: string) => Promise; + /** + * Resolve a user-defined type name to its mirror .st file content, + * or return null if no such mirror file exists (built-in types, + * library types we can't introspect, etc.). Used for sub-property + * descent. + */ + resolveTypeMirror?: (typeName: string, deviceRoot: string) => Promise; /** Read a single PLC variable. Returns its current value as a string. */ readVariable?: (projectFilePath: string, variablePath: string) => Promise; /** Write the live-values snapshot. Defaults to writeLiveValues. */ @@ -100,7 +124,19 @@ export class LiveValuesPump { private timer: ReturnType | null = null; private busy = false; - constructor(private cfg: PumpConfig, private deps: Required) {} + private deps: Required; + + constructor(private cfg: PumpConfig, deps: PumpDeps) { + this.deps = { + readSelection: deps.readSelection ?? readSelection, + readPouFile: deps.readPouFile ?? (() => Promise.reject(new Error('readPouFile not provided'))), + // No resolver provided -> never descend (no sub-property paths). Pump + // still works for top-level vars only, matching v0.3 behaviour. + resolveTypeMirror: deps.resolveTypeMirror ?? (async () => null), + readVariable: deps.readVariable ?? (() => Promise.reject(new Error('readVariable not provided'))), + writeLiveValues: deps.writeLiveValues ?? writeLiveValues, + }; + } start(): void { if (this.timer) return; @@ -124,17 +160,40 @@ export class LiveValuesPump { if (sel.status !== 'ok') return; const pou = sel.payload.selection; const text = await this.deps.readPouFile(pou.abs_path); - const names = parseVarNames(text); - if (names.length === 0) return; + const decls = parseVarDecls(text); + if (decls.length === 0) return; + + // Build the read-list: top-level vars + depth-1 sub-properties when + // the type resolves to another mirror .st file. + // Each entry maps the dotted PLC path (PLC_PRG.fb.x) to the snapshot + // key the TUI overlay matches against (the leaf name -- TUI matches + // identifiers in the line text, so `x` is what the line says). + const reads: Array<{ pouPath: string; key: string }> = []; + const deviceRoot = this.deviceRootFor(pou.abs_path); + for (const d of decls) { + reads.push({ pouPath: `${pou.name}.${d.name}`, key: d.name }); + if (!d.type) continue; + const childText = await this.deps.resolveTypeMirror(d.type, deviceRoot); + if (!childText) continue; + for (const cd of parseVarDecls(childText)) { + reads.push({ + pouPath: `${pou.name}.${d.name}.${cd.name}`, + // Overlay key is the LEAF name -- the TUI scans line text for + // identifiers and matches against this map. Sub-property names + // win over top-level when they collide; that's harmless because + // a struct's member would only appear inside the struct's own + // declaration, not the parent POU's source. + key: cd.name, + }); + } + } + const values: Record = {}; const ts = Date.now(); - for (const name of names) { + for (const r of reads) { try { - const v = await this.deps.readVariable( - sel.payload.project_dir, - `${pou.name}.${name}` - ); - values[name] = { value: v, ts }; + const v = await this.deps.readVariable(sel.payload.project_dir, r.pouPath); + values[r.key] = { value: v, ts }; } catch { // single-var failure: skip silently, others may succeed } @@ -150,4 +209,31 @@ export class LiveValuesPump { this.busy = false; } } + + /** + * Given a POU's abs path under a mirror tree, return the device root + * (the directory where its sibling .st files live). Used by + * resolveTypeMirror to look up `.st` next to the current POU. + * + * Example: + * /abs/proj/mcp-mirror/CodesysRpi/Plc Logic/Application/PLC_PRG.st + * -> /abs/proj/mcp-mirror/CodesysRpi + * + * The mirror layout always has a `mcp-mirror//...` shape, so + * we walk up until we find that segment. If we can't, return the parent + * dir as a best-effort (resolveTypeMirror still works for same-folder + * lookups, just won't find types declared in sibling folders). + */ + private deviceRootFor(absPath: string): string { + const norm = absPath.replace(/\\/g, '/'); + const idx = norm.lastIndexOf('/mcp-mirror/'); + if (idx < 0) { + const p = norm.split('/'); + p.pop(); + return p.join('/'); + } + const after = norm.slice(idx + '/mcp-mirror/'.length); + const deviceName = after.split('/')[0]; + return `${norm.slice(0, idx)}/mcp-mirror/${deviceName}`; + } } diff --git a/src/server.ts b/src/server.ts index bda70fc..220b96a 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3205,6 +3205,39 @@ export async function startMcpServer(config: ServerConfig): Promise { { readSelection, readPouFile: (absPath) => fs.promises.readFile(absPath, 'utf8'), + // Best-effort recursive walk of the device root looking for + // .st. The mirror layout puts every code-bearing object + // (POU, FB, DUT) at a stable filename matching its declared name, + // so this is a clean lookup even though the directory tree is + // arbitrary nesting under "Plc Logic/Application/...". + resolveTypeMirror: async (typeName, deviceRoot) => { + const target = `${typeName}.st`; + const walk = async (dir: string): Promise => { + let entries: import('fs').Dirent[]; + try { + entries = await fs.promises.readdir(dir, { withFileTypes: true }); + } catch { + return null; + } + for (const e of entries) { + const full = path.join(dir, e.name); + if (e.isFile() && e.name === target) { + try { + return await fs.promises.readFile(full, 'utf8'); + } catch { + return null; + } + } + } + for (const e of entries) { + if (!e.isDirectory()) continue; + const found = await walk(path.join(dir, e.name)); + if (found !== null) return found; + } + return null; + }; + return walk(deviceRoot); + }, readVariable: async (projectFilePath, variablePath) => { // Reuse the existing read_variable script. Returns the value as // a string captured from script stdout. Errors throw. diff --git a/tests/unit/live-values-pump.test.ts b/tests/unit/live-values-pump.test.ts index 5d9b319..82fac65 100644 --- a/tests/unit/live-values-pump.test.ts +++ b/tests/unit/live-values-pump.test.ts @@ -103,4 +103,75 @@ describe('LiveValuesPump.tick', () => { await expect(pump.tick()).resolves.toBeUndefined(); expect(writeLiveValues).not.toHaveBeenCalled(); }); + + it('descends one level for vars whose type resolves to a mirror file', async () => { + const parent = [ + 'PROGRAM PLC_PRG', + 'VAR', + ' fb : FB_Test;', + 'END_VAR', + ].join('\n'); + const child = [ + 'FUNCTION_BLOCK FB_Test', + 'VAR', + ' inner : INT;', + ' bAlive : BOOL;', + 'END_VAR', + ].join('\n'); + const writeLiveValues = vi.fn(async () => {}); + const reads: string[] = []; + const pump = new LiveValuesPump( + { stateFilePath: '/state.json', liveValuesFilePath: '/lv.json', intervalMs: 500 }, + { + readSelection: vi.fn(async () => okSelection()), + readPouFile: vi.fn(async () => parent), + resolveTypeMirror: vi.fn(async (typeName: string) => + typeName === 'FB_Test' ? child : null + ), + readVariable: vi.fn(async (_proj: string, varPath: string) => { + reads.push(varPath); + if (varPath === 'PLC_PRG.fb') return ''; + if (varPath === 'PLC_PRG.fb.inner') return '7'; + if (varPath === 'PLC_PRG.fb.bAlive') return 'TRUE'; + throw new Error('unknown'); + }), + writeLiveValues, + } + ); + await pump.tick(); + expect(reads).toContain('PLC_PRG.fb.inner'); + expect(reads).toContain('PLC_PRG.fb.bAlive'); + const payload = writeLiveValues.mock.calls[0][2]; + expect(payload.values.inner.value).toBe('7'); + expect(payload.values.bAlive.value).toBe('TRUE'); + }); + + it('does not descend into vars with primitive types (no mirror file)', async () => { + const parent = [ + 'PROGRAM PLC_PRG', + 'VAR', + ' counter : INT;', + ' bFlag : BOOL;', + 'END_VAR', + ].join('\n'); + const writeLiveValues = vi.fn(async () => {}); + const resolveTypeMirror = vi.fn(async () => null); + const pump = new LiveValuesPump( + { stateFilePath: '/state.json', liveValuesFilePath: '/lv.json', intervalMs: 500 }, + { + readSelection: vi.fn(async () => okSelection()), + readPouFile: vi.fn(async () => parent), + resolveTypeMirror, + readVariable: vi.fn(async () => '1'), + writeLiveValues, + } + ); + await pump.tick(); + // resolveTypeMirror still gets ASKED for primitives; the resolver returns + // null for each, so no descent. That's fine — caller decides what's + // resolvable. + expect(resolveTypeMirror).toHaveBeenCalled(); + const payload = writeLiveValues.mock.calls[0][2]; + expect(Object.keys(payload.values).sort()).toEqual(['bFlag', 'counter']); + }); }); diff --git a/tests/unit/var-block-parse.test.ts b/tests/unit/var-block-parse.test.ts index 2f59819..ea24861 100644 --- a/tests/unit/var-block-parse.test.ts +++ b/tests/unit/var-block-parse.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { parseVarNames } from '../../src/live-values-pump'; +import { parseVarNames, parseVarDecls } from '../../src/live-values-pump'; describe('parseVarNames', () => { it('extracts vars from a single VAR block', () => { @@ -55,3 +55,50 @@ describe('parseVarNames', () => { expect(parseVarNames(text)).toEqual(['bRelay', 'iCount']); }); }); + +describe('parseVarDecls', () => { + it('returns name + declared type per decl', () => { + const text = [ + 'PROGRAM PLC_PRG', + 'VAR', + ' counter : INT := 0;', + ' bRunning : BOOL;', + ' fb : FB_Test;', + 'END_VAR', + ].join('\n'); + expect(parseVarDecls(text)).toEqual([ + { name: 'counter', type: 'INT' }, + { name: 'bRunning', type: 'BOOL' }, + { name: 'fb', type: 'FB_Test' }, + ]); + }); + + it('strips ARRAY [...] OF wrapper', () => { + const text = ['VAR', ' buf : ARRAY [0..9] OF INT;', 'END_VAR'].join('\n'); + expect(parseVarDecls(text)).toEqual([{ name: 'buf', type: 'INT' }]); + }); + + it('strips POINTER TO and REFERENCE TO', () => { + const text = [ + 'VAR', + ' p : POINTER TO BOOL;', + ' r : REFERENCE TO MyStruct;', + 'END_VAR', + ].join('\n'); + expect(parseVarDecls(text)).toEqual([ + { name: 'p', type: 'BOOL' }, + { name: 'r', type: 'MyStruct' }, + ]); + }); + + it('handles AT %loc prefix', () => { + const text = ['VAR', ' bRelay AT %QX0.1 : BOOL := FALSE;', 'END_VAR'].join('\n'); + expect(parseVarDecls(text)).toEqual([{ name: 'bRelay', type: 'BOOL' }]); + }); + + it('returns type=null when there is no `: ` clause on the line', () => { + // Pathological: missing type. Don't crash; just emit name with null type. + const text = ['VAR', ' weird;', 'END_VAR'].join('\n'); + expect(parseVarDecls(text)).toEqual([{ name: 'weird', type: null }]); + }); +});