Detect copper placed inside PCB keepouts - #238
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| test("reports non-excluded component copper inside a keepout once", async () => { | ||
| const errors = checkPcbCopperOverKeepout(circuitJson) | ||
|
|
||
| expect(errors).toHaveLength(1) | ||
| expect(errors[0]?.pcb_placement_error_id).toBe( | ||
| "copper_over_keepout_pcb_component_c14_pcb_keepout_antenna", | ||
| ) | ||
| expect(errors[0]?.message).toContain("C14") | ||
| expect(errors[0]?.message).toContain("PCB keepout") | ||
| expect(errors[0]?.message).not.toContain("pcb_keepout_antenna") | ||
|
|
||
| const placementErrors = await runAllPlacementChecks(circuitJson) | ||
| expect( | ||
| placementErrors.some( | ||
| (error) => | ||
| "pcb_placement_error_id" in error && | ||
| error.pcb_placement_error_id === errors[0]?.pcb_placement_error_id, | ||
| ), | ||
| ).toBe(true) | ||
| }) | ||
|
|
||
| test("reports a via inside a circular keepout on a shared layer", () => { | ||
| const errors = checkPcbCopperOverKeepout([ | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "pcb_via_1", | ||
| x: 1, | ||
| y: 1, | ||
| hole_diameter: 0.2, | ||
| outer_diameter: 0.5, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_keepout", | ||
| pcb_keepout_id: "pcb_keepout_circle", | ||
| shape: "circle", | ||
| center: { x: 1, y: 1 }, | ||
| radius: 1, | ||
| layers: ["bottom"], | ||
| }, | ||
| ] as AnyCircuitElement[]) | ||
|
|
||
| expect(errors).toHaveLength(1) | ||
| expect(errors[0]?.pcb_placement_error_id).toBe( | ||
| "copper_over_keepout_pcb_via_1_pcb_keepout_circle", | ||
| ) | ||
| }) |
There was a problem hiding this comment.
This file contains two test(...) calls (one at line 98 and another at line 119), which violates the rule that a *.test.ts file may have AT MOST one test(...). The file should be split into two numbered files, e.g. check-pcb-copper-over-keepout1.test.ts and check-pcb-copper-over-keepout2.test.ts, each containing exactly one test(...) call.
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
| import { expect, test } from "bun:test" | ||
| import type { AnyCircuitElement } from "circuit-json" | ||
| import { convertCircuitJsonToPcbSvg } from "circuit-to-svg" | ||
| import { checkPcbCopperOverKeepout } from "lib/check-pcb-copper-over-keepout" | ||
|
|
||
| test("reports a via inside a circular keepout on a shared layer", () => { | ||
| const circleKeepoutCircuitJson = [ | ||
| { | ||
| type: "pcb_board", | ||
| pcb_board_id: "pcb_board_1", | ||
| center: { x: 1, y: 1 }, | ||
| width: 5, | ||
| height: 5, | ||
| thickness: 1.6, | ||
| num_layers: 2, | ||
| material: "fr4", | ||
| }, | ||
| { | ||
| type: "pcb_via", | ||
| pcb_via_id: "pcb_via_1", | ||
| x: 1, | ||
| y: 1, | ||
| hole_diameter: 0.2, | ||
| outer_diameter: 0.5, | ||
| layers: ["top", "bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_keepout", | ||
| pcb_keepout_id: "pcb_keepout_circle", | ||
| shape: "circle", | ||
| center: { x: 1, y: 1 }, | ||
| radius: 1, | ||
| layers: ["bottom"], | ||
| }, | ||
| { | ||
| type: "pcb_silkscreen_text", | ||
| pcb_silkscreen_text_id: "pcb_silkscreen_text_via", | ||
| pcb_component_id: "", | ||
| anchor_position: { x: 1, y: -0.4 }, | ||
| anchor_alignment: "center", | ||
| font: "tscircuit2024", | ||
| font_size: 0.3, | ||
| layer: "top", | ||
| text: "VIA ERROR", | ||
| }, | ||
| ] as AnyCircuitElement[] | ||
| const errors = checkPcbCopperOverKeepout(circleKeepoutCircuitJson) | ||
|
|
||
| expect(errors).toHaveLength(1) | ||
| expect(errors[0]?.pcb_placement_error_id).toBe( | ||
| "copper_over_keepout_pcb_via_1_pcb_keepout_circle", | ||
| ) | ||
| expect( | ||
| convertCircuitJsonToPcbSvg([...circleKeepoutCircuitJson, ...errors], { | ||
| shouldDrawErrors: true, | ||
| }), | ||
| ).toMatchSvgSnapshot(import.meta.path, "via-over-circle-keepout") | ||
| }) |
There was a problem hiding this comment.
The file name check-pcb-via-over-keepout.test.ts does not match any export name in the project. The function being tested is checkPcbCopperOverKeepout (exported from lib/check-pcb-copper-over-keepout.ts). Per the naming rule, file names should be consistent with the project and should generally match at least one export name. Since the companion test file is already named check-pcb-copper-over-keepout.test.ts, this second test file should be named to reflect the same export — for example check-pcb-copper-over-keepout2.test.ts — rather than introducing a new, inconsistent name that implies a different (non-existent) function.
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
1 participant
Summary
excluded_pcb_component_idsand deduplicate multiple overlapping pads into one error per component/keepout pairrunAllPlacementChecksOwnership and routing behavior
This detects a genuine pre-route placement violation: copper that already exists in a footprint (or an explicitly placed via) overlaps a PCB keepout before the autorouter runs. The router cannot legally connect a terminal whose pad is already inside forbidden copper space.
Core already consumes this category through its existing placement gate:
Board_doInitialPcbPlacementDesignRuleChecksrunsrunAllPlacementChecks, records thepcb_placement_error, andshouldSkipAutoroutingBecauseOfPlacementErrorsprevents routing until the placement is corrected (or placement checks are explicitly disabled).This is therefore different from a post-hoc validator that rejects otherwise successful autorouter output. The violation exists in the input placement, and the established pre-route gate handles it before any route is produced.
The smart-watch antenna network reproduces the issue: L2/C14 footprint copper enters a keepout that excludes only ANT1.
Visual regression coverage
C14: ERRORwhileANT1: EXCLUDEDis acceptedValidation
bun test tests/lib/check-pcb-copper-over-keepout.test.ts tests/lib/check-pcb-via-over-keepout.test.tsbun test— 186 passedbunx tsc --noEmitbun run buildbun run format:check— exits successfully with the repository's existing oversized-fixture notices