fix: correct BGA pin renumbering with missing pins and non-default origins by claw-explorer · Pull Request #564 · tscircuit/footprinter

claw-explorer · 2026-03-28T19:38:08Z

Summary

Fixes incorrect BGA pin numbering when using missing pins together with non-default origin parameters (blorigin, brorigin, trorigin).

Bug

When combining missing("center") with e.g. blorigin(true) on a 3×3 BGA, the generated pads had:

Pin 0 appearing (invalid — pins should be 1-indexed)
Incorrect renumbering (e.g., pins 0, 1, 2, 4, 5, 7, 8, 9 instead of 1, 2, 3, 4, 5, 6, 7, 8)

Before (blorigin + missing center):

Pin 7: (-1, -1) [C1]
Pin 8: (0, -1)  [C2]
Pin 9: (1, -1)  [C3]
Pin 4: (-1, 0)  [B1]
Pin 5: (1, 0)   [B3]
Pin 0: (-1, 1)  [A1]  ← invalid pin 0!
Pin 1: (0, 1)   [A2]
Pin 2: (1, 1)   [A3]

After (correct):

Pin 6: (-1, -1) [C1]
Pin 7: (0, -1)  [C2]
Pin 8: (1, -1)  [C3]
Pin 4: (-1, 0)  [B1]
Pin 5: (1, 0)   [B3]
Pin 1: (-1, 1)  [A1]  ← correct pin 1
Pin 2: (0, 1)   [A2]
Pin 3: (1, 1)   [A3]

Root Cause

The missing_pins_passed running counter assumed physical grid iteration order (top-to-bottom, left-to-right) matched pin numbering order. This is only true for the default tl origin. With bl/br/tr origins, pin numbering follows a different order than iteration, so the counter tracked the wrong count.

Fix

Replace the running counter with a per-pin calculation that counts how many missing pin numbers are strictly less than the current pin_num. This produces the correct renumbered index regardless of iteration order.

Tests

Added 5 new tests covering:

blorigin + missing center
brorigin + missing center
trorigin + missing center
String parsing with blorigin + missing specific pin
All origins produce same physical pad positions with missing pins

All 400 tests pass (395 existing + 5 new).

The string parser only recognized 'res' and 'cap' as passive footprints that use imperial/metric sizing (e.g., res0402, cap0603). The 'led' and 'diode' footprints also use the passive() function but were incorrectly parsed as having num_pins instead of an imperial size code. For example, 'led0402' was parsed as { fn: 'led', num_pins: 402 } instead of { fn: 'led', imperial: '0402' }, causing a runtime error: 'Could not determine required pad dimensions (p, pw, ph)'. Fix: extend the passive footprint check in the string parser proxy to include 'led' and 'diode' alongside 'res' and 'cap'. Tests: added 11 new tests covering led and diode string parsing with imperial sizes, metric sizes, THT variant, and builder API parity.

…igins When using missing pins (e.g., missing center) together with a non-default origin (blorigin, brorigin, trorigin), pin numbers were incorrectly calculated. The running counter `missing_pins_passed` only worked with the default top-left origin because it assumed physical iteration order matched pin numbering order. With non-default origins, the iteration visits grid cells in a fixed top-to-bottom, left-to-right order, but pin numbers follow a different order (e.g., bottom-to-top for blorigin). This mismatch caused: - Pin 0 appearing (invalid) - Incorrect pin renumbering (gaps, wrong assignments) Fix: Replace the running counter with a per-pin calculation that counts how many missing pin numbers are lower than the current pin number. This produces the correct renumbered index regardless of iteration order. Added tests covering all non-default origins with missing pins.

graphite-app · 2026-03-28T19:39:59Z

tests/bga-missing-with-origin.test.ts

+import { expect, test } from "bun:test"
+import type { PcbSmtPad } from "circuit-json"
+import { convertCircuitJsonToPcbSvg } from "circuit-to-svg"
+import { fp } from "../src/footprinter"
+
+const getPads = (soup: any[]) =>
+  soup.filter((el): el is PcbSmtPad => el.type === "pcb_smtpad")
+
+const getPinNums = (pads: PcbSmtPad[]) =>
+  pads.map((p) => Number(p.port_hints?.[0])).sort((a, b) => a - b)
+
+test("bga missing center with blorigin renumbers pins correctly", () => {
+  const soup = fp()
+    .bga(8)
+    .grid("3x3")
+    .p(1)
+    .missing("center")
+    .blorigin(true)
+    .soup()
+
+  const pads = getPads(soup)
+  expect(pads).toHaveLength(8)
+
+  const pinNums = getPinNums(pads)
+  // Should be consecutively renumbered 1-8 with no gaps or zeroes
+  expect(pinNums).toEqual([1, 2, 3, 4, 5, 6, 7, 8])
+
+  // Pin 1 should be at bottom-left (A1)
+  const pin1 = pads.find((p) => p.port_hints?.[0] === "1")
+  expect(pin1?.x).toBe(-1)
+  expect(pin1?.y).toBe(1)
+  expect(pin1?.port_hints?.[1]).toBe("A1")
+
+  const svgContent = convertCircuitJsonToPcbSvg(soup)
+  expect(svgContent).toMatchSvgSnapshot(
+    import.meta.path,
+    "bga_3x3_blorigin_missing_center",
+  )
+})
+
+test("bga missing center with brorigin renumbers pins correctly", () => {
+  const soup = fp()
+    .bga(8)
+    .grid("3x3")
+    .p(1)
+    .missing("center")
+    .brorigin(true)
+    .soup()
+
+  const pads = getPads(soup)
+  expect(pads).toHaveLength(8)
+
+  const pinNums = getPinNums(pads)
+  expect(pinNums).toEqual([1, 2, 3, 4, 5, 6, 7, 8])
+
+  // Pin 1 should be at bottom-right (A1)
+  const pin1 = pads.find((p) => p.port_hints?.[0] === "1")
+  expect(pin1?.x).toBe(1)
+  expect(pin1?.y).toBe(1)
+  expect(pin1?.port_hints?.[1]).toBe("A1")
+
+  const svgContent = convertCircuitJsonToPcbSvg(soup)
+  expect(svgContent).toMatchSvgSnapshot(
+    import.meta.path,
+    "bga_3x3_brorigin_missing_center",
+  )
+})
+
+test("bga missing center with trorigin renumbers pins correctly", () => {
+  const soup = fp()
+    .bga(8)
+    .grid("3x3")
+    .p(1)
+    .missing("center")
+    .trorigin(true)
+    .soup()
+
+  const pads = getPads(soup)
+  expect(pads).toHaveLength(8)
+
+  const pinNums = getPinNums(pads)
+  expect(pinNums).toEqual([1, 2, 3, 4, 5, 6, 7, 8])
+
+  // Pin 1 should be at top-right (A1)
+  const pin1 = pads.find((p) => p.port_hints?.[0] === "1")
+  expect(pin1?.x).toBe(1)
+  expect(pin1?.y).toBe(-1)
+  expect(pin1?.port_hints?.[1]).toBe("A1")
+
+  const svgContent = convertCircuitJsonToPcbSvg(soup)
+  expect(svgContent).toMatchSvgSnapshot(
+    import.meta.path,
+    "bga_3x3_trorigin_missing_center",
+  )
+})
+
+test("bga missing specific pin with blorigin via string", () => {
+  // 3x3 grid, blorigin, missing B2 (center)
+  // With blorigin: B2 is grid position (1,1) → pin_y=1, pin_x=1 → pin 5
+  const soup = fp.string("bga8_grid3x3_p1_missing(B2)_blorigin").circuitJson()
+
+  const pads = getPads(soup)
+  expect(pads).toHaveLength(8)
+
+  const pinNums = getPinNums(pads)
+  expect(pinNums).toEqual([1, 2, 3, 4, 5, 6, 7, 8])
+
+  const svgContent = convertCircuitJsonToPcbSvg(soup)
+  expect(svgContent).toMatchSvgSnapshot(
+    import.meta.path,
+    "bga_3x3_blorigin_missing_B2",
+  )
+})
+
+test("all origins produce same physical pad positions with missing center", () => {
+  const origins = [
+    { name: "tl", opts: {} },
+    { name: "bl", opts: { blorigin: true } },
+    { name: "br", opts: { brorigin: true } },
+    { name: "tr", opts: { trorigin: true } },
+  ] as const
+
+  const allPadPositions: Array<Array<{ x: number; y: number }>> = []
+
+  for (const origin of origins) {
+    let builder = fp().bga(8).grid("3x3").p(1).missing("center")
+    if ("blorigin" in origin.opts) builder = builder.blorigin(true)
+    if ("brorigin" in origin.opts) builder = builder.brorigin(true)
+    if ("trorigin" in origin.opts) builder = builder.trorigin(true)
+
+    const soup = builder.soup()
+    const pads = getPads(soup)
+
+    // All should have exactly 8 pads
+    expect(pads).toHaveLength(8)
+
+    // All should have pins 1-8
+    const pinNums = getPinNums(pads)
+    expect(pinNums).toEqual([1, 2, 3, 4, 5, 6, 7, 8])
+
+    // Collect sorted physical positions
+    const positions = pads
+      .map((p) => ({ x: p.x, y: p.y }))
+      .sort((a, b) => (a.x === b.x ? a.y - b.y : a.x - b.x))
+    allPadPositions.push(positions)
+  }
+
+  // All origins should produce the same set of physical positions
+  for (let i = 1; i < allPadPositions.length; i++) {
+    for (let j = 0; j < allPadPositions[0]!.length; j++) {
+      expect(allPadPositions[i]![j]!.x).toBe(allPadPositions[0]![j]!.x)
+      expect(allPadPositions[i]![j]!.y).toBe(allPadPositions[0]![j]!.y)
+    }
+  }
+})


This test file contains multiple test() functions (at least 6 different tests), which violates the rule that a *.test.ts file may have AT MOST one test(...). After that, the user should split into multiple, numbered files. This file should be split into separate numbered files like bga-missing-with-origin1.test.ts, bga-missing-with-origin2.test.ts, etc., with each file containing only one test() function.

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

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

graphite-app · 2026-04-01T19:18:23Z

tests/diode-string-parsing3.test.ts

+import { expect, test } from "bun:test"
+import { convertCircuitJsonToPcbSvg } from "circuit-to-svg"
+import { fp } from "../src/footprinter"
+
+test("diode1206 string produces 2 SMT pads", () => {
+  const soup = fp.string("diode1206").circuitJson()
+  const pads = soup.filter((e: any) => e.type === "pcb_smtpad")
+  expect(pads).toHaveLength(2)
+
+  const svgContent = convertCircuitJsonToPcbSvg(soup)
+  expect(svgContent).toMatchSvgSnapshot(import.meta.path, "diode1206_string")
+})


This test file violates the rule that a *.test.ts file may have AT MOST one test(...). While this file only contains one test, there are multiple diode-string-parsing test files, indicating these should be numbered consistently. The file should be renamed following the numbered pattern or the tests should be consolidated into fewer files.

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

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

claw-explorer added 2 commits March 28, 2026 15:07

claw-explorer requested review from seveibar and techmannih as code owners March 28, 2026 19:38

graphite-app bot reviewed Mar 28, 2026

View reviewed changes

refactor: split tests to one test per file (per review feedback)

b3dd8ed

graphite-app bot reviewed Apr 1, 2026

View reviewed changes

style: format test files

be4dc6d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: correct BGA pin renumbering with missing pins and non-default origins#564

fix: correct BGA pin renumbering with missing pins and non-default origins#564
claw-explorer wants to merge 4 commits intotscircuit:mainfrom
claw-explorer:fix/bga-missing-pins-with-nondefault-origin

claw-explorer commented Mar 28, 2026

Uh oh!

graphite-app bot Mar 28, 2026

Uh oh!

graphite-app bot Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

claw-explorer commented Mar 28, 2026

Summary

Bug

Before (blorigin + missing center):

After (correct):

Root Cause

Fix

Tests

Uh oh!

graphite-app bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant