Skip to content
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
1 change: 1 addition & 0 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export { checkSourceTracesHavePcbTraces } from "./lib/check-source-traces-have-p
export { checkTracesAreContiguous } from "./lib/check-traces-are-contiguous/check-traces-are-contiguous"
export { checkPcbTracesOutOfBoard } from "./lib/check-trace-out-of-board/checkTraceOutOfBoard"
export { checkPcbComponentOverlap } from "./lib/check-pcb-components-overlap/checkPcbComponentOverlap"
export { checkPcbTraceLengths } from "./lib/check-pcb-trace-lengths"
export { checkPadPadClearance } from "./lib/check-pad-pad-clearance"
export { checkPadTraceClearance } from "./lib/check-pad-trace-clearance"
export { checkViaTraceClearance } from "./lib/check-via-trace-clearance"
Expand Down
93 changes: 93 additions & 0 deletions lib/check-pcb-trace-lengths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import type {
AnyCircuitElement,
PcbTrace,
PcbTraceRoutePoint,
PcbTraceTooLongWarning,
SourceTrace,
} from "circuit-json"

const DEFAULT_VIA_LENGTH_MM = 1.6

const getRoutePointPosition = (routePoint: PcbTraceRoutePoint) =>
routePoint.route_type === "through_pad"
? routePoint.start
: { x: routePoint.x, y: routePoint.y }

const getPcbTraceLength = (pcbTrace: PcbTrace): number => {
if (pcbTrace.trace_length !== undefined) return pcbTrace.trace_length

let traceLength = 0

for (
let routePointIndex = 0;
routePointIndex < pcbTrace.route.length;
routePointIndex++
) {
const routePoint = pcbTrace.route[routePointIndex]
if (!routePoint) continue

if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
continue
}
Comment on lines +29 to +32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Critical bug: The continue statement skips calculating the distance from the via to the next route point. This causes traces with vias to report shorter lengths than actual, potentially missing length violations.

Impact: Traces containing vias will have their lengths underreported by the distance between the via and the next route point, causing the check to miss traces that actually exceed their maximum length.

Fix: Remove the continue statement and let the code fall through to calculate the distance to the next point:

if (routePoint.route_type === "via") {
  traceLength += DEFAULT_VIA_LENGTH_MM
  // Don't continue - still need to calculate distance to next point
}
Suggested change
if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
continue
}
if (routePoint.route_type === "via") {
traceLength += DEFAULT_VIA_LENGTH_MM
// Don't continue - still need to calculate distance to next point
}

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.


const nextRoutePoint = pcbTrace.route[routePointIndex + 1]
if (!nextRoutePoint) continue

const routePointPosition = getRoutePointPosition(routePoint)
const nextRoutePointPosition = getRoutePointPosition(nextRoutePoint)
traceLength += Math.hypot(
nextRoutePointPosition.x - routePointPosition.x,
nextRoutePointPosition.y - routePointPosition.y,
)
}

return traceLength
}

export const checkPcbTraceLengths = (
circuitJson: AnyCircuitElement[],
): PcbTraceTooLongWarning[] => {
const sourceTraces = circuitJson.filter(
(element): element is SourceTrace => element.type === "source_trace",
)
const pcbTraces = circuitJson.filter(
(element): element is PcbTrace => element.type === "pcb_trace",
)

const sourceTracesById = new Map(
sourceTraces.map((sourceTrace) => [
sourceTrace.source_trace_id,
sourceTrace,
]),
)
const warnings: PcbTraceTooLongWarning[] = []

for (const pcbTrace of pcbTraces) {
if (!pcbTrace.source_trace_id) continue

const sourceTrace = sourceTracesById.get(pcbTrace.source_trace_id)
if (!sourceTrace) continue

const maximumTraceLength = sourceTrace.max_length
if (typeof maximumTraceLength !== "number") continue

const actualTraceLength = getPcbTraceLength(pcbTrace)
if (actualTraceLength <= maximumTraceLength) continue

warnings.push({
type: "pcb_trace_too_long_warning",
pcb_trace_too_long_warning_id: `pcb_trace_too_long_warning_${pcbTrace.pcb_trace_id}`,
warning_type: "pcb_trace_too_long_warning",
message: `PCB trace is ${actualTraceLength.toFixed(2)}mm long, exceeding the ${maximumTraceLength}mm maximum`,
pcb_trace_id: pcbTrace.pcb_trace_id,
source_trace_id: sourceTrace.source_trace_id,
source_net_id: sourceTrace.connected_source_net_ids[0],
actual_trace_length: actualTraceLength,
maximum_trace_length: maximumTraceLength,
subcircuit_id: pcbTrace.subcircuit_id ?? sourceTrace.subcircuit_id,
})
}

return warnings
}
2 changes: 2 additions & 0 deletions lib/run-all-checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { checkPadTraceClearance } from "./check-pad-trace-clearance"
import { checkPcbComponentsOutOfBoard } from "./check-pcb-components-out-of-board/checkPcbComponentsOutOfBoard"
import { checkViasOffBoard } from "./check-pcb-components-out-of-board/checkViasOffBoard"
import { checkPcbComponentOverlap } from "./check-pcb-components-overlap/checkPcbComponentOverlap"
import { checkPcbTraceLengths } from "./check-pcb-trace-lengths"
import { checkPinMustBeConnected } from "./check-pin-must-be-connected"
import { checkSameNetViaSpacing } from "./check-same-net-via-spacing"
import { checkSourceTracesHavePcbTraces } from "./check-source-traces-have-pcb-traces"
Expand Down Expand Up @@ -48,6 +49,7 @@ export async function runAllRoutingChecks(circuitJson: AnyCircuitElement[]) {
return [
...checkEachPcbPortConnectedToPcbTraces(circuitJson),
...checkSourceTracesHavePcbTraces(circuitJson),
...checkPcbTraceLengths(circuitJson),
...checkEachPcbTraceNonOverlapping(circuitJson),
...checkPadTraceClearance(circuitJson),
...checkViaTraceClearance(circuitJson),
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
"@types/bun": "^1.2.8",
"@types/debug": "^4.1.12",
"bun-match-svg": "^0.0.11",
"circuit-json": "^0.0.428",
"circuit-json": "^0.0.453",
"circuit-to-svg": "^0.0.388",
"debug": "^4.3.5",
"format-si-unit": "^0.0.7",
"tscircuit": "^0.0.1786",
"tsup": "^8.2.3",
"zod": "^3.23.8"
Expand Down
78 changes: 78 additions & 0 deletions tests/lib/check-pcb-trace-lengths.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { expect, test } from "bun:test"
import type { AnyCircuitElement } from "circuit-json"
import { checkPcbTraceLengths, runAllRoutingChecks } from "../.."

const circuitJson = [
{
type: "source_trace",
source_trace_id: "overlength_source_trace",
connected_source_port_ids: [],
connected_source_net_ids: ["signal_net"],
max_length: 10,
subcircuit_id: "board",
},
{
type: "source_trace",
source_trace_id: "short_source_trace",
connected_source_port_ids: [],
connected_source_net_ids: [],
max_length: 10,
},
{
type: "source_trace",
source_trace_id: "unconstrained_source_trace",
connected_source_port_ids: [],
connected_source_net_ids: [],
max_length: null,
},
{
type: "pcb_trace",
pcb_trace_id: "overlength_pcb_trace",
source_trace_id: "overlength_source_trace",
subcircuit_id: "board",
route: [
{ route_type: "wire", x: 0, y: 0, width: 0.15, layer: "top" },
{ route_type: "wire", x: 12, y: 0, width: 0.15, layer: "top" },
],
},
{
type: "pcb_trace",
pcb_trace_id: "short_pcb_trace",
source_trace_id: "short_source_trace",
trace_length: 8,
route: [],
},
{
type: "pcb_trace",
pcb_trace_id: "unconstrained_pcb_trace",
source_trace_id: "unconstrained_source_trace",
trace_length: 20,
route: [],
},
] as AnyCircuitElement[]

test("warns when a PCB trace exceeds its source trace maximum length", () => {
expect(checkPcbTraceLengths(circuitJson)).toEqual([
{
type: "pcb_trace_too_long_warning",
pcb_trace_too_long_warning_id:
"pcb_trace_too_long_warning_overlength_pcb_trace",
warning_type: "pcb_trace_too_long_warning",
message: "PCB trace is 12.00mm long, exceeding the 10mm maximum",
pcb_trace_id: "overlength_pcb_trace",
source_trace_id: "overlength_source_trace",
source_net_id: "signal_net",
actual_trace_length: 12,
maximum_trace_length: 10,
subcircuit_id: "board",
},
])
})

test("is included in the routing check pipeline", async () => {
const results = await runAllRoutingChecks(circuitJson)

expect(
results.filter((result) => result.type === "pcb_trace_too_long_warning"),
).toEqual(checkPcbTraceLengths(circuitJson))
})
Comment on lines +54 to +78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file contains two test(...) calls (lines 54 and 72), which violates the rule that a *.test.ts file may have AT MOST one test(...). Please split this into two separate numbered files, e.g. check-pcb-trace-lengths1.test.ts (for the 'warns when a PCB trace exceeds its source trace maximum length' test) and check-pcb-trace-lengths2.test.ts (for the 'is included in the routing check pipeline' test).

Spotted by Graphite (based on custom rule: Custom rule)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Loading