From 819cdeeef5df79182f8609d2df43f1505abe49ad Mon Sep 17 00:00:00 2001 From: ShiboSoftwareDev Date: Thu, 12 Mar 2026 22:36:10 +0200 Subject: [PATCH] Improve PCB calc coordinate error handling and edge-prop diagnostics --- .../NormalComponent/NormalComponent.ts | 24 ++++-- .../PrimitiveComponent/PrimitiveComponent.ts | 74 +++++++++++------ ...oup_doInitialPcbCalcPlacementResolution.ts | 68 ++++++++-------- .../primitive-components/SilkscreenText.ts | 16 ++-- .../components/pcb/calc-pcb-position.test.tsx | 79 +++++++++++++++++-- .../silkscreen-text.test.tsx | 28 +++++++ 6 files changed, 206 insertions(+), 83 deletions(-) diff --git a/lib/components/base-components/NormalComponent/NormalComponent.ts b/lib/components/base-components/NormalComponent/NormalComponent.ts index 471a5f40b..58bbb94b9 100644 --- a/lib/components/base-components/NormalComponent/NormalComponent.ts +++ b/lib/components/base-components/NormalComponent/NormalComponent.ts @@ -1750,28 +1750,28 @@ export class NormalComponent< this._validatePcbCoordinateReferences({ rawValue: pcbLeftEdgeX, axis: "pcbX", - propertyNameForError: "pcbLeftEdgeX", + propertyName: "pcbLeftEdgeX", }) } if (pcbRightEdgeX !== undefined) { this._validatePcbCoordinateReferences({ rawValue: pcbRightEdgeX, axis: "pcbX", - propertyNameForError: "pcbRightEdgeX", + propertyName: "pcbRightEdgeX", }) } if (pcbTopEdgeY !== undefined) { this._validatePcbCoordinateReferences({ rawValue: pcbTopEdgeY, axis: "pcbY", - propertyNameForError: "pcbTopEdgeY", + propertyName: "pcbTopEdgeY", }) } if (pcbBottomEdgeY !== undefined) { this._validatePcbCoordinateReferences({ rawValue: pcbBottomEdgeY, axis: "pcbY", - propertyNameForError: "pcbBottomEdgeY", + propertyName: "pcbBottomEdgeY", }) } } @@ -1826,10 +1826,14 @@ export class NormalComponent< (props.pcbX !== undefined ? this._resolvePcbCoordinate(props.pcbX, "pcbX") : pcbLeftEdgeX !== undefined - ? this._resolvePcbCoordinate(pcbLeftEdgeX, "pcbX") + + ? this._resolvePcbCoordinate(pcbLeftEdgeX, "pcbX", { + propertyName: "pcbLeftEdgeX", + }) + componentWidth / 2 : pcbRightEdgeX !== undefined - ? this._resolvePcbCoordinate(pcbRightEdgeX, "pcbX") - + ? this._resolvePcbCoordinate(pcbRightEdgeX, "pcbX", { + propertyName: "pcbRightEdgeX", + }) - componentWidth / 2 : undefined) @@ -1838,10 +1842,14 @@ export class NormalComponent< (props.pcbY !== undefined ? this._resolvePcbCoordinate(props.pcbY, "pcbY") : pcbTopEdgeY !== undefined - ? this._resolvePcbCoordinate(pcbTopEdgeY, "pcbY") - + ? this._resolvePcbCoordinate(pcbTopEdgeY, "pcbY", { + propertyName: "pcbTopEdgeY", + }) - componentHeight / 2 : pcbBottomEdgeY !== undefined - ? this._resolvePcbCoordinate(pcbBottomEdgeY, "pcbY") + + ? this._resolvePcbCoordinate(pcbBottomEdgeY, "pcbY", { + propertyName: "pcbBottomEdgeY", + }) + componentHeight / 2 : undefined) diff --git a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts index 5c18b37c0..aeb8f96a4 100644 --- a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts +++ b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts @@ -244,21 +244,21 @@ export abstract class PrimitiveComponent< this._validatePcbCoordinateReferences({ rawValue: rawPcbX, axis: "pcbX", - propertyNameForError: "pcbX", + propertyName: "pcbX", }) this._validatePcbCoordinateReferences({ rawValue: rawPcbY, axis: "pcbY", - propertyNameForError: "pcbY", + propertyName: "pcbY", }) } protected _validatePcbCoordinateReferences(params: { rawValue: unknown axis: "pcbX" | "pcbY" - propertyNameForError?: string + propertyName?: string }): void { - const { rawValue, axis, propertyNameForError = axis } = params + const { rawValue, axis, propertyName = axis } = params if (typeof rawValue !== "string") return const isNormalComponent = (this as any)._isNormalComponent === true @@ -270,8 +270,8 @@ export abstract class PrimitiveComponent< calcIdentifiers = extractCalcIdentifiers(rawValue) } catch { this._reportInvalidComponentPropertyError( - propertyNameForError, - `Invalid ${propertyNameForError} value for ${this.componentName}: Invalid calc() expression. expression="${rawValue}"`, + propertyName, + `Invalid ${propertyName} value for ${this.componentName}: Invalid calc() expression. expression="${rawValue}"`, ) return } @@ -282,8 +282,8 @@ export abstract class PrimitiveComponent< if (includesComponentVariable && !allowComponentVariables) { this._reportInvalidComponentPropertyError( - propertyNameForError, - `Invalid ${propertyNameForError} value for ${this.componentName}: component-relative calc references are not supported for footprint elements (${this.componentName}); ${propertyNameForError} will be ignored. expression="${rawValue}"`, + propertyName, + `Invalid ${propertyName} value for ${this.componentName}: component-relative calc references are not supported for footprint elements (${this.componentName}); ${propertyName} will be ignored. expression="${rawValue}"`, ) } } @@ -295,10 +295,16 @@ export abstract class PrimitiveComponent< allowBoardVariables?: boolean allowComponentVariables?: boolean componentVariables?: Record + propertyName?: string } = {}, ): number { if (rawValue == null) return 0 - if (typeof rawValue === "number") return rawValue + const propertyName = options.propertyName ?? axis + + if (typeof rawValue === "number") { + if (Number.isFinite(rawValue)) return rawValue + return 0 + } if (typeof rawValue !== "string") { throw new Error( `Invalid ${axis} value for ${this.componentName}: ${String(rawValue)}`, @@ -320,9 +326,11 @@ export abstract class PrimitiveComponent< const boardVariables = board?._getBoardCalcVariables() ?? {} if (includesBoardVariable && !board) { - throw new Error( - `Cannot resolve ${axis} for ${this.componentName}: no board found for board.* variables`, + this._reportInvalidComponentPropertyError( + propertyName, + `Invalid ${propertyName} value for ${this.componentName}: no board found for board.* variables. expression="${rawValue}"`, ) + return 0 } if ( @@ -330,9 +338,11 @@ export abstract class PrimitiveComponent< board && Object.keys(boardVariables).length === 0 ) { - throw new Error( - "Cannot do calculations based on board size when the board is auto-sized", + this._reportInvalidComponentPropertyError( + propertyName, + `Invalid ${propertyName} value for ${this.componentName}: Cannot do calculations based on board size when the board is auto-sized. expression="${rawValue}"`, ) + return 0 } Object.assign(knownVariables, boardVariables) @@ -374,9 +384,11 @@ export abstract class PrimitiveComponent< return evaluateCalcString(rawValue, { knownVariables }) } catch (error) { const message = error instanceof Error ? error.message : String(error) - throw new Error( - `Invalid ${axis} value for ${this.componentName}: ${message}`, + this._reportInvalidComponentPropertyError( + propertyName, + `Invalid ${propertyName} value for ${this.componentName}: ${message}. expression="${rawValue}"`, ) + return 0 } } @@ -429,16 +441,28 @@ export abstract class PrimitiveComponent< ) } - resolvePcbCoordinate( - rawValue: unknown, - axis: "pcbX" | "pcbY", - options: { - allowBoardVariables?: boolean - allowComponentVariables?: boolean - componentVariables?: Record - } = {}, - ): number { - return this._resolvePcbCoordinate(rawValue, axis, options) + resolvePcbCoordinate(params: { + rawValue: unknown + axis: "pcbX" | "pcbY" + allowBoardVariables?: boolean + allowComponentVariables?: boolean + componentVariables?: Record + propertyName?: string + }): number { + const { + rawValue, + axis, + allowBoardVariables, + allowComponentVariables, + componentVariables, + propertyName, + } = params + return this._resolvePcbCoordinate(rawValue, axis, { + allowBoardVariables, + allowComponentVariables, + componentVariables, + propertyName, + }) } /** diff --git a/lib/components/primitive-components/Group/Group_doInitialPcbCalcPlacementResolution.ts b/lib/components/primitive-components/Group/Group_doInitialPcbCalcPlacementResolution.ts index 64183509c..5cf013713 100644 --- a/lib/components/primitive-components/Group/Group_doInitialPcbCalcPlacementResolution.ts +++ b/lib/components/primitive-components/Group/Group_doInitialPcbCalcPlacementResolution.ts @@ -166,66 +166,66 @@ export function Group_doInitialPcbCalcPlacementResolution( } if (rawPcbX !== undefined) { - const resolvedPcbX = component.resolvePcbCoordinate(rawPcbX, "pcbX", { + const resolvedPcbX = component.resolvePcbCoordinate({ + rawValue: rawPcbX, + axis: "pcbX", allowComponentVariables: true, componentVariables: componentVars, }) component._resolvedPcbCalcOffsetX = resolvedPcbX nextCenter.x = resolvedPcbX } else if (rawPcbLeftEdgeX !== undefined) { - const resolvedPcbLeftEdgeX = component.resolvePcbCoordinate( - rawPcbLeftEdgeX, - "pcbX", - { - allowComponentVariables: true, - componentVariables: componentVars, - }, - ) + const resolvedPcbLeftEdgeX = component.resolvePcbCoordinate({ + rawValue: rawPcbLeftEdgeX, + axis: "pcbX", + allowComponentVariables: true, + componentVariables: componentVars, + propertyName: "pcbLeftEdgeX", + }) const resolvedPcbX = resolvedPcbLeftEdgeX + componentWidth / 2 component._resolvedPcbCalcOffsetX = resolvedPcbX nextCenter.x = resolvedPcbX } else if (rawPcbRightEdgeX !== undefined) { - const resolvedPcbRightEdgeX = component.resolvePcbCoordinate( - rawPcbRightEdgeX, - "pcbX", - { - allowComponentVariables: true, - componentVariables: componentVars, - }, - ) + const resolvedPcbRightEdgeX = component.resolvePcbCoordinate({ + rawValue: rawPcbRightEdgeX, + axis: "pcbX", + allowComponentVariables: true, + componentVariables: componentVars, + propertyName: "pcbRightEdgeX", + }) const resolvedPcbX = resolvedPcbRightEdgeX - componentWidth / 2 component._resolvedPcbCalcOffsetX = resolvedPcbX nextCenter.x = resolvedPcbX } if (rawPcbY !== undefined) { - const resolvedPcbY = component.resolvePcbCoordinate(rawPcbY, "pcbY", { + const resolvedPcbY = component.resolvePcbCoordinate({ + rawValue: rawPcbY, + axis: "pcbY", allowComponentVariables: true, componentVariables: componentVars, }) component._resolvedPcbCalcOffsetY = resolvedPcbY nextCenter.y = resolvedPcbY } else if (rawPcbTopEdgeY !== undefined) { - const resolvedPcbTopEdgeY = component.resolvePcbCoordinate( - rawPcbTopEdgeY, - "pcbY", - { - allowComponentVariables: true, - componentVariables: componentVars, - }, - ) + const resolvedPcbTopEdgeY = component.resolvePcbCoordinate({ + rawValue: rawPcbTopEdgeY, + axis: "pcbY", + allowComponentVariables: true, + componentVariables: componentVars, + propertyName: "pcbTopEdgeY", + }) const resolvedPcbY = resolvedPcbTopEdgeY - componentHeight / 2 component._resolvedPcbCalcOffsetY = resolvedPcbY nextCenter.y = resolvedPcbY } else if (rawPcbBottomEdgeY !== undefined) { - const resolvedPcbBottomEdgeY = component.resolvePcbCoordinate( - rawPcbBottomEdgeY, - "pcbY", - { - allowComponentVariables: true, - componentVariables: componentVars, - }, - ) + const resolvedPcbBottomEdgeY = component.resolvePcbCoordinate({ + rawValue: rawPcbBottomEdgeY, + axis: "pcbY", + allowComponentVariables: true, + componentVariables: componentVars, + propertyName: "pcbBottomEdgeY", + }) const resolvedPcbY = resolvedPcbBottomEdgeY + componentHeight / 2 component._resolvedPcbCalcOffsetY = resolvedPcbY nextCenter.y = resolvedPcbY diff --git a/lib/components/primitive-components/SilkscreenText.ts b/lib/components/primitive-components/SilkscreenText.ts index fb4e0dee6..5f9be3af2 100644 --- a/lib/components/primitive-components/SilkscreenText.ts +++ b/lib/components/primitive-components/SilkscreenText.ts @@ -97,14 +97,14 @@ export class SilkscreenText extends PrimitiveComponent< isFlipped ? flipY() : identity(), ), { - x: this.resolvePcbCoordinate( - resolvedPcbSxPcbX ?? props.pcbX ?? 0, - "pcbX", - ), - y: this.resolvePcbCoordinate( - resolvedPcbSxPcbY ?? props.pcbY ?? 0, - "pcbY", - ), + x: this.resolvePcbCoordinate({ + rawValue: resolvedPcbSxPcbX ?? props.pcbX ?? 0, + axis: "pcbX", + }), + y: this.resolvePcbCoordinate({ + rawValue: resolvedPcbSxPcbY ?? props.pcbY ?? 0, + axis: "pcbY", + }), }, ) : this._getGlobalPcbPositionBeforeLayout() diff --git a/tests/components/pcb/calc-pcb-position.test.tsx b/tests/components/pcb/calc-pcb-position.test.tsx index e6f066554..4eb118370 100644 --- a/tests/components/pcb/calc-pcb-position.test.tsx +++ b/tests/components/pcb/calc-pcb-position.test.tsx @@ -68,12 +68,75 @@ test("calc expressions using board bounds fail for auto-sized boards", () => { , ) - try { - circuit.render() - throw new Error("render should have thrown") - } catch (error) { - expect((error as Error).message).toContain( - "Cannot do calculations based on board size when the board is auto-sized", - ) - } + circuit.render() + + const invalidPropertyErrors = + circuit.db.source_invalid_component_property_error.list() + expect(invalidPropertyErrors.length).toBeGreaterThan(0) + + const message = invalidPropertyErrors + .filter((element) => "message" in element) + .map((element) => element.message) + .join("\n") + expect(message).toMatchInlineSnapshot( + `"Invalid pcbX value for Resistor: Cannot do calculations based on board size when the board is auto-sized. expression="calc(board.minX + 1mm)""`, + ) +}) + +test("calc expressions on edge props fail for auto-sized boards", () => { + const { circuit } = getTestFixture() + + circuit.add( + + + , + ) + + circuit.render() + + const invalidPropertyErrors = + circuit.db.source_invalid_component_property_error.list() + expect(invalidPropertyErrors.length).toBeGreaterThan(0) + + const message = invalidPropertyErrors + .filter((element) => "message" in element) + .map((element) => element.message) + .join("\n") + expect(message).toMatchInlineSnapshot(` + "Invalid pcbLeftEdgeX value for Resistor: Cannot do calculations based on board size when the board is auto-sized. expression="calc(board.minX + 1mm)" + Invalid pcbTopEdgeY value for Resistor: Cannot do calculations based on board size when the board is auto-sized. expression="calc(board.maxY - 1mm)"" + `) +}) + +test("calc expressions using board bounds without a board report no-board error", () => { + const { circuit } = getTestFixture() + + circuit.add( + , + ) + + circuit.render() + + const invalidPropertyErrors = + circuit.db.source_invalid_component_property_error.list() + expect(invalidPropertyErrors.length).toBeGreaterThan(0) + + const message = invalidPropertyErrors + .filter((element) => "message" in element) + .map((element) => element.message) + .join("\n") + expect(message).toMatchInlineSnapshot( + `"Invalid pcbX value for Resistor: no board found for board.* variables. expression="calc(board.minX + 1mm)""`, + ) }) diff --git a/tests/components/primitive-components/silkscreen-text.test.tsx b/tests/components/primitive-components/silkscreen-text.test.tsx index 4d77fca91..1ac96b1a8 100644 --- a/tests/components/primitive-components/silkscreen-text.test.tsx +++ b/tests/components/primitive-components/silkscreen-text.test.tsx @@ -29,3 +29,31 @@ test("SilkscreenText rendering", () => { expect(project).toMatchPcbSnapshot(import.meta.path) }) + +test("SilkscreenText malformed calc does not throw and reports validation error", () => { + const { project } = getTestFixture() + + project.add( + + + , + ) + + project.render() + + const invalidPropertyErrors = + project.db.source_invalid_component_property_error.list() + expect(invalidPropertyErrors.length).toBeGreaterThan(0) + + const message = invalidPropertyErrors + .filter((element) => "message" in element) + .map((element) => element.message) + .join("\n") + expect(message).toMatchInlineSnapshot( + `"Invalid pcbX value for SilkscreenText: Invalid calc() expression. expression="calc board.minX + 1mm)""`, + ) + + const silkscreenTexts = project.db.pcb_silkscreen_text.list() + expect(silkscreenTexts.length).toBe(1) + expect(silkscreenTexts[0].anchor_position.x).toBe(0) +})