feat: implement TO-220 horizontal variant (to220h)#535
Closed
victorjzq wants to merge 1 commit intotscircuit:mainfrom
Closed
feat: implement TO-220 horizontal variant (to220h)#535victorjzq wants to merge 1 commit intotscircuit:mainfrom
victorjzq wants to merge 1 commit intotscircuit:mainfrom
Conversation
Closes tscircuit#185 Adds to220h footprint for the TO-220 horizontal (lying-down) package: - Signal pins in a horizontal row at 2.54mm pitch - Mounting tab as a plated hole with silkscreen outline - tabup parameter: tab extends toward +Y (default: tabdown, toward -Y) - String aliases: to220h_3, to220h_3_tabup, TO-220-3_Horizontal, TO-220-3_Horizontal_TabUp - 5 tests with SVG snapshot coverage
Contributor
Author
|
Closing duplicate - #533 covers the same implementation. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+1
to
+58
| import { test, expect } from "bun:test" | ||
| import { convertCircuitJsonToPcbSvg } from "circuit-to-svg" | ||
| import { fp } from "src/footprinter" | ||
|
|
||
| test("to220h_3 (3 pins, tab down, default)", () => { | ||
| const circuitJson = fp.string("to220h_3").circuitJson() | ||
| const svgContent = convertCircuitJsonToPcbSvg(circuitJson) | ||
|
|
||
| expect(circuitJson).toBeDefined() | ||
| expect(circuitJson.length).toBeGreaterThan(0) | ||
|
|
||
| // Should have 3 signal plated holes + 1 tab hole | ||
| const holes = circuitJson.filter((e: any) => e.type === "pcb_plated_hole") | ||
| expect(holes).toHaveLength(4) | ||
|
|
||
| expect(svgContent).toMatchSvgSnapshot(import.meta.path, "to220h_3") | ||
| }) | ||
|
|
||
| test("to220h_3_tabup (3 pins, tab up)", () => { | ||
| const circuitJson = fp.string("to220h_3_tabup").circuitJson() | ||
| const svgContent = convertCircuitJsonToPcbSvg(circuitJson) | ||
|
|
||
| expect(circuitJson).toBeDefined() | ||
| expect(circuitJson.length).toBeGreaterThan(0) | ||
|
|
||
| expect(svgContent).toMatchSvgSnapshot(import.meta.path, "to220h_3_tabup") | ||
| }) | ||
|
|
||
| test("to220h_2 (2 pins, tab down)", () => { | ||
| const circuitJson = fp.string("to220h_2").circuitJson() | ||
| const svgContent = convertCircuitJsonToPcbSvg(circuitJson) | ||
|
|
||
| expect(circuitJson).toBeDefined() | ||
| expect(circuitJson.length).toBeGreaterThan(0) | ||
|
|
||
| expect(svgContent).toMatchSvgSnapshot(import.meta.path, "to220h_2") | ||
| }) | ||
|
|
||
| test("TO-220-3_Horizontal (KiCad alias)", () => { | ||
| const circuitJson = fp.string("TO-220-3_Horizontal").circuitJson() | ||
|
|
||
| expect(circuitJson).toBeDefined() | ||
| expect(circuitJson.length).toBeGreaterThan(0) | ||
|
|
||
| // Must match the canonical to220h_3 output | ||
| const canonical = fp.string("to220h_3").circuitJson() | ||
| expect(JSON.stringify(circuitJson)).toEqual(JSON.stringify(canonical)) | ||
| }) | ||
|
|
||
| test("TO-220-3_Horizontal_TabUp (KiCad alias)", () => { | ||
| const circuitJson = fp.string("TO-220-3_Horizontal_TabUp").circuitJson() | ||
|
|
||
| expect(circuitJson).toBeDefined() | ||
| expect(circuitJson.length).toBeGreaterThan(0) | ||
|
|
||
| const canonical = fp.string("to220h_3_tabup").circuitJson() | ||
| expect(JSON.stringify(circuitJson)).toEqual(JSON.stringify(canonical)) | ||
| }) |
Contributor
There was a problem hiding this comment.
This test file contains 5 test() calls (lines 5, 19, 29, 39, and 50), which violates the rule that a *.test.ts file may have AT MOST one test(...). After the first test, the user should split into multiple, numbered files. This file should be split into separate files like to220h1.test.ts, to220h2.test.ts, to220h3.test.ts, to220h4.test.ts, and to220h5.test.ts, with each file containing only one test() call.
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
Comment on lines
+93
to
+109
| const bodyBottom = tabSign > 0 ? pinsBottom : tabCenterY - tabh / 2 - 0.3 | ||
| const bodyTop = tabSign > 0 ? tabCenterY + tabh / 2 + 0.3 : pinsTop | ||
|
|
||
| const silkBody: PcbSilkscreenPath = { | ||
| type: "pcb_silkscreen_path", | ||
| layer: "top", | ||
| pcb_component_id: "", | ||
| pcb_silkscreen_path_id: "", | ||
| route: [ | ||
| { x: -halfW, y: pinsTop }, | ||
| { x: halfW, y: pinsTop }, | ||
| { x: halfW, y: pinsBottom }, | ||
| { x: -halfW, y: pinsBottom }, | ||
| { x: -halfW, y: pinsTop }, | ||
| ], | ||
| stroke_width: 0.1, | ||
| } |
Contributor
There was a problem hiding this comment.
The variables bodyBottom and bodyTop are calculated but never used. The silkBody silkscreen path uses pinsTop and pinsBottom instead, causing the body outline to always draw around just the pins regardless of tab direction. This contradicts the comment stating it should draw "around the body region (between pins and tab)".
Fix:
const silkBody: PcbSilkscreenPath = {
type: "pcb_silkscreen_path",
layer: "top",
pcb_component_id: "",
pcb_silkscreen_path_id: "",
route: [
{ x: -halfW, y: bodyTop },
{ x: halfW, y: bodyTop },
{ x: halfW, y: bodyBottom },
{ x: -halfW, y: bodyBottom },
{ x: -halfW, y: bodyTop },
],
stroke_width: 0.1,
}
Suggested change
| const bodyBottom = tabSign > 0 ? pinsBottom : tabCenterY - tabh / 2 - 0.3 | |
| const bodyTop = tabSign > 0 ? tabCenterY + tabh / 2 + 0.3 : pinsTop | |
| const silkBody: PcbSilkscreenPath = { | |
| type: "pcb_silkscreen_path", | |
| layer: "top", | |
| pcb_component_id: "", | |
| pcb_silkscreen_path_id: "", | |
| route: [ | |
| { x: -halfW, y: pinsTop }, | |
| { x: halfW, y: pinsTop }, | |
| { x: halfW, y: pinsBottom }, | |
| { x: -halfW, y: pinsBottom }, | |
| { x: -halfW, y: pinsTop }, | |
| ], | |
| stroke_width: 0.1, | |
| } | |
| const bodyBottom = tabSign > 0 ? pinsBottom : tabCenterY - tabh / 2 - 0.3 | |
| const bodyTop = tabSign > 0 ? tabCenterY + tabh / 2 + 0.3 : pinsTop | |
| const silkBody: PcbSilkscreenPath = { | |
| type: "pcb_silkscreen_path", | |
| layer: "top", | |
| pcb_component_id: "", | |
| pcb_silkscreen_path_id: "", | |
| route: [ | |
| { x: -halfW, y: bodyTop }, | |
| { x: halfW, y: bodyTop }, | |
| { x: halfW, y: bodyBottom }, | |
| { x: -halfW, y: bodyBottom }, | |
| { x: -halfW, y: bodyTop }, | |
| ], | |
| stroke_width: 0.1, | |
| } |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #185
Summary
to220hfootprint for the TO-220 Horizontal (lying-down) package variant, matching KiCad'sTO-220-3_Horizontal_TabDownandTO-220-3_Horizontal_TabUpfootprintstabupparameter controls tab direction:false(default) = tab toward −Y (TabDown),true= tab toward +Y (TabUp)TO-220-3_Horizontal→to220h_3,TO-220-3_Horizontal_TabUp→to220h_3_tabupto220h_NnamingSupported strings
to220h_3to220h_3_tabupTO-220-3_Horizontalto220h_3TO-220-3_Horizontal_TabUpto220h_3_tabupTest plan
to220h_3– 3 signal holes + 1 tab hole, SVG snapshotto220h_3_tabup– tab in opposite direction, SVG snapshotto220h_2– 2-pin variant, SVG snapshotTO-220-3_Horizontalalias round-trips to canonical outputTO-220-3_Horizontal_TabUpalias round-trips to canonical outputto220andto220ftests still passfootprint-completenesstest passes