From faf3ce23ed85ea8b95c80757cffbd794c425025a Mon Sep 17 00:00:00 2001 From: Rishabh Gupta Date: Tue, 8 Apr 2025 01:39:41 +0530 Subject: [PATCH 1/2] fix: same name of the components should add an error to the circuit json --- lib/components/normal-components/Board.ts | 10 -- .../primitive-components/Group/Group.ts | 57 ++++++++++- .../repro12-same-name-of-components.test.tsx | 96 +++++++++++++++++++ 3 files changed, 148 insertions(+), 15 deletions(-) create mode 100644 tests/repros/repro12-same-name-of-components.test.tsx diff --git a/lib/components/normal-components/Board.ts b/lib/components/normal-components/Board.ts index f2aea3b6..79314935 100644 --- a/lib/components/normal-components/Board.ts +++ b/lib/components/normal-components/Board.ts @@ -144,14 +144,4 @@ export class Board extends Group { _computePcbGlobalTransformBeforeLayout(): Matrix { return identity() } - - doInitialPcbDesignRuleChecks() { - if (this.root?.pcbDisabled) return - if (this.getInheritedProperty("routingDisabled")) return - const { db } = this.root! - const errors = checkEachPcbTraceNonOverlapping(db.toArray()) - for (const error of errors) { - db.pcb_trace_error.insert(error) - } - } } diff --git a/lib/components/primitive-components/Group/Group.ts b/lib/components/primitive-components/Group/Group.ts index 769bd401..e566aaa2 100644 --- a/lib/components/primitive-components/Group/Group.ts +++ b/lib/components/primitive-components/Group/Group.ts @@ -4,8 +4,6 @@ import { groupProps, } from "@tscircuit/props" import * as SAL from "@tscircuit/schematic-autolayout" -import { CapacityMeshAutorouter } from "lib/utils/autorouting/CapacityMeshAutorouter" -import type { SimplifiedPcbTrace } from "lib/utils/autorouting/SimpleRouteJson" import { type LayerRef, type PcbTrace, @@ -16,15 +14,21 @@ import { } from "circuit-json" import { ConnectivityMap } from "circuit-json-to-connectivity-map" import Debug from "debug" -import type { SimpleRouteJson } from "lib/utils/autorouting/SimpleRouteJson" +import { CapacityMeshAutorouter } from "lib/utils/autorouting/CapacityMeshAutorouter" +import type { GenericLocalAutorouter } from "lib/utils/autorouting/GenericLocalAutorouter" +import type { + SimpleRouteJson, + SimplifiedPcbTrace, +} from "lib/utils/autorouting/SimpleRouteJson" +import { getSimpleRouteJsonFromCircuitJson } from "lib/utils/public-exports" import { z } from "zod" import { NormalComponent } from "../../base-components/NormalComponent/NormalComponent" import type { Trace } from "../Trace/Trace" import type { TraceI } from "../Trace/TraceI" import { TraceHint } from "../TraceHint" import type { ISubcircuit } from "./ISubcircuit" -import { getSimpleRouteJsonFromCircuitJson } from "lib/utils/public-exports" -import type { GenericLocalAutorouter } from "lib/utils/autorouting/GenericLocalAutorouter" +import type { PrimitiveComponent } from "lib/components/base-components/PrimitiveComponent" +import { checkEachPcbTraceNonOverlapping } from "@tscircuit/checks" export class Group = typeof groupProps> extends NormalComponent @@ -714,4 +718,47 @@ export class Group = typeof groupProps> const autorouter = this._getAutorouterConfig() return autorouter.groupMode === "sequential-trace" } + + doInitialPcbDesignRuleChecks() { + if (this.root?.pcbDisabled) return + if (this.getInheritedProperty("routingDisabled")) return + const { db } = this.root! + + if (this.isSubcircuit) { + const subcircuitComponentsByName = new Map() + + for (const child of this.children) { + // Skip if child is itself a subcircuit + if ((child as any).isSubcircuit) continue + + if (child._parsedProps.name) { + const components = + subcircuitComponentsByName.get(child._parsedProps.name) || [] + components.push(child) + subcircuitComponentsByName.set(child._parsedProps.name, components) + } + } + + for (const [name, components] of subcircuitComponentsByName.entries()) { + if (components.length > 1) { + db.pcb_trace_error.insert({ + error_type: "pcb_trace_error", + message: `Multiple components found with name "${name}" in subcircuit "${this._parsedProps.name || "unnamed"}". Component names must be unique within a subcircuit.`, + source_trace_id: "", + pcb_trace_id: "", + pcb_component_ids: components + .map((c) => c.pcb_component_id!) + .filter(Boolean), + pcb_port_ids: [], + }) + } + } + } + + // Run other DRC checks + const errors = checkEachPcbTraceNonOverlapping(db.toArray()) + for (const error of errors) { + db.pcb_trace_error.insert(error) + } + } } diff --git a/tests/repros/repro12-same-name-of-components.test.tsx b/tests/repros/repro12-same-name-of-components.test.tsx new file mode 100644 index 00000000..5cf108a1 --- /dev/null +++ b/tests/repros/repro12-same-name-of-components.test.tsx @@ -0,0 +1,96 @@ +import { getTestFixture } from "tests/fixtures/get-test-fixture" +import { test } from "bun:test" +import { expect } from "bun:test" +test("repro-12: same name of components in subcircuit", async () => { + const { circuit } = getTestFixture() + + circuit.add( + + + + + , + ) + + await circuit.renderUntilSettled() + + const pcb_trace_errors = circuit.db.pcb_trace_error.list() + expect(pcb_trace_errors).toHaveLength(1) +}) + +test("repro-12: same name of components in board", async () => { + const { circuit } = getTestFixture() + + circuit.add( + + + + + , + ) + + await circuit.renderUntilSettled() + + const pcb_trace_errors = circuit.db.pcb_trace_error.list() + expect(pcb_trace_errors).toHaveLength(1) +}) + +test("repro-12: same name of components in different subcircuits", async () => { + const { circuit } = getTestFixture() + + circuit.add( + + + + + + + + + + + , + ) + + await circuit.renderUntilSettled() + + // No errors because the components are in different subcircuits + const pcb_trace_errors = circuit.db.pcb_trace_error.list() + expect(pcb_trace_errors).toHaveLength(0) +}) From ecf086e0e1cddc01dd0d11e839409a064244a83a Mon Sep 17 00:00:00 2001 From: Rishabh Gupta Date: Tue, 8 Apr 2025 02:00:01 +0530 Subject: [PATCH 2/2] add the drc check back to board --- lib/components/normal-components/Board.ts | 13 ++++++++++++ .../primitive-components/Group/Group.ts | 20 ++++++------------- .../repro12-same-name-of-components.test.tsx | 2 +- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/components/normal-components/Board.ts b/lib/components/normal-components/Board.ts index 79314935..5c899459 100644 --- a/lib/components/normal-components/Board.ts +++ b/lib/components/normal-components/Board.ts @@ -144,4 +144,17 @@ export class Board extends Group { _computePcbGlobalTransformBeforeLayout(): Matrix { return identity() } + + doInitialPcbDesignRuleChecks() { + if (this.root?.pcbDisabled) return + if (this.getInheritedProperty("routingDisabled")) return + const { db } = this.root! + + super.doInitialPcbDesignRuleChecks() + + const errors = checkEachPcbTraceNonOverlapping(db.toArray()) + for (const error of errors) { + db.pcb_trace_error.insert(error) + } + } } diff --git a/lib/components/primitive-components/Group/Group.ts b/lib/components/primitive-components/Group/Group.ts index e566aaa2..ff7400ca 100644 --- a/lib/components/primitive-components/Group/Group.ts +++ b/lib/components/primitive-components/Group/Group.ts @@ -4,6 +4,8 @@ import { groupProps, } from "@tscircuit/props" import * as SAL from "@tscircuit/schematic-autolayout" +import { CapacityMeshAutorouter } from "lib/utils/autorouting/CapacityMeshAutorouter" +import type { SimplifiedPcbTrace } from "lib/utils/autorouting/SimpleRouteJson" import { type LayerRef, type PcbTrace, @@ -14,21 +16,17 @@ import { } from "circuit-json" import { ConnectivityMap } from "circuit-json-to-connectivity-map" import Debug from "debug" -import { CapacityMeshAutorouter } from "lib/utils/autorouting/CapacityMeshAutorouter" -import type { GenericLocalAutorouter } from "lib/utils/autorouting/GenericLocalAutorouter" -import type { - SimpleRouteJson, - SimplifiedPcbTrace, -} from "lib/utils/autorouting/SimpleRouteJson" -import { getSimpleRouteJsonFromCircuitJson } from "lib/utils/public-exports" +import type { SimpleRouteJson } from "lib/utils/autorouting/SimpleRouteJson" import { z } from "zod" import { NormalComponent } from "../../base-components/NormalComponent/NormalComponent" import type { Trace } from "../Trace/Trace" import type { TraceI } from "../Trace/TraceI" import { TraceHint } from "../TraceHint" import type { ISubcircuit } from "./ISubcircuit" -import type { PrimitiveComponent } from "lib/components/base-components/PrimitiveComponent" +import { getSimpleRouteJsonFromCircuitJson } from "lib/utils/public-exports" +import type { GenericLocalAutorouter } from "lib/utils/autorouting/GenericLocalAutorouter" import { checkEachPcbTraceNonOverlapping } from "@tscircuit/checks" +import type { PrimitiveComponent } from "lib/components/base-components/PrimitiveComponent" export class Group = typeof groupProps> extends NormalComponent @@ -754,11 +752,5 @@ export class Group = typeof groupProps> } } } - - // Run other DRC checks - const errors = checkEachPcbTraceNonOverlapping(db.toArray()) - for (const error of errors) { - db.pcb_trace_error.insert(error) - } } } diff --git a/tests/repros/repro12-same-name-of-components.test.tsx b/tests/repros/repro12-same-name-of-components.test.tsx index 5cf108a1..04ac31ae 100644 --- a/tests/repros/repro12-same-name-of-components.test.tsx +++ b/tests/repros/repro12-same-name-of-components.test.tsx @@ -47,7 +47,7 @@ test("repro-12: same name of components in board", async () => { expect(pcb_trace_errors).toHaveLength(1) }) -test("repro-12: same name of components in different subcircuits", async () => { +test("repro-12: same name of components in different subcircuits should not be an error", async () => { const { circuit } = getTestFixture() circuit.add(