Skip to content

fix: same name of the components should add an error to the circuit json #724

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

Merged
merged 2 commits into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/components/normal-components/Board.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ export class Board extends Group<typeof boardProps> {
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)
Expand Down
39 changes: 39 additions & 0 deletions lib/components/primitive-components/Group/Group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ 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 { checkEachPcbTraceNonOverlapping } from "@tscircuit/checks"
import type { PrimitiveComponent } from "lib/components/base-components/PrimitiveComponent"

export class Group<Props extends z.ZodType<any, any, any> = typeof groupProps>
extends NormalComponent<Props>
Expand Down Expand Up @@ -714,4 +716,41 @@ export class Group<Props extends z.ZodType<any, any, any> = 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<string, PrimitiveComponent[]>()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subcircuit component name checking is good 👍 this one doesnt need to go on the board

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, we don't need to check it here in Board. But we will have to import from the Group class for this behaviour to be accepted, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can call the super method inside the board so that it runs the DRC checks that run for any subcircuit. You can alternatively create an internal underscore function that does what the subcircuit does. For example _checkForConflictingNames()


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: [],
})
}
}
}
}
}
96 changes: 96 additions & 0 deletions tests/repros/repro12-same-name-of-components.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<group subcircuit>
<resistor resistance="1k" footprint="0402" name="R1" schX={3} pcbX={3} />
<resistor
resistance="1k"
footprint="0402"
name="R1"
schX={-3}
pcbX={-3}
/>
<trace from=".R1 > .pin1" to=".R1 > .pin1" />
</group>,
)

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(
<board width="10mm" height="10mm">
<resistor resistance="1k" footprint="0402" name="R1" schX={3} pcbX={3} />
<resistor
resistance="1k"
footprint="0402"
name="R1"
schX={-3}
pcbX={-3}
/>
<trace from=".R1 > .pin1" to=".R1 > .pin1" />
</board>,
)

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 should not be an error", async () => {
const { circuit } = getTestFixture()

circuit.add(
<group>
<group subcircuit>
<resistor
resistance="1k"
footprint="0402"
name="R1"
schX={3}
pcbX={3}
/>
<resistor
resistance="1k"
footprint="0402"
name="R2"
schX={-3}
pcbX={-3}
/>
<trace from=".R1 > .pin1" to=".R1 > .pin1" />
</group>
<subcircuit>
<resistor
resistance="1k"
footprint="0402"
name="R1"
schX={3}
pcbX={3}
/>
<resistor
resistance="1k"
footprint="0402"
name="R2"
schX={-3}
pcbX={-3}
/>
</subcircuit>
</group>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid circuit that shouldnt error. Names must be unique within a subcircuit

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't I am checking for the pcb_trace_error to be 0, will update the test name to be clear.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Woops sorry ty!!

)

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)
})