Fix JST PH: support jst_ph_N format and dynamic silkscreen#560
Open
mendarb wants to merge 2 commits intotscircuit:mainfrom
Open
Fix JST PH: support jst_ph_N format and dynamic silkscreen#560mendarb wants to merge 2 commits intotscircuit:mainfrom
mendarb wants to merge 2 commits intotscircuit:mainfrom
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Comment on lines
+34
to
59
| test("jst_ph_4 (trailing pin count format)", () => { | ||
| const circuitJson = fp.string("jst_ph_4").circuitJson() | ||
| const holes = circuitJson.filter( | ||
| (e: any) => e.type === "pcb_plated_hole", | ||
| ) | ||
| expect(holes.length).toBe(4) | ||
| const svgContent = convertCircuitJsonToPcbSvg(circuitJson) | ||
| expect(svgContent).toMatchSvgSnapshot(import.meta.path + "jst_ph_4") | ||
| }) | ||
|
|
||
| test("jst4_ph generates 4 pads", () => { | ||
| const circuitJson = fp.string("jst4_ph").circuitJson() | ||
| const holes = circuitJson.filter( | ||
| (e: any) => e.type === "pcb_plated_hole", | ||
| ) | ||
| expect(holes.length).toBe(4) | ||
| const svgContent = convertCircuitJsonToPcbSvg(circuitJson) | ||
| expect(svgContent).toMatchSvgSnapshot(import.meta.path + "jst4_ph") | ||
| }) | ||
|
|
||
| test("jst_sh_6 (trailing pin count format)", () => { | ||
| const circuitJson = fp.string("jst_sh_6").circuitJson() | ||
| const pads = circuitJson.filter((e: any) => e.type === "pcb_smtpad") | ||
| // 6 signal pads + 2 mounting pads = 8 | ||
| expect(pads.length).toBe(8) | ||
| }) |
Contributor
There was a problem hiding this comment.
The test file contains multiple test() functions, which violates the rule that a *.test.ts file may have AT MOST one test(...). The file has at least 4 test() functions: 'jst_ph_4 (trailing pin count format)', 'jst4_ph generates 4 pads', 'jst_sh_6 (trailing pin count format)', and others. To fix this, split the tests into multiple numbered files such as jst1.test.ts, jst2.test.ts, jst3.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.
- Add regex to parse pin count from trailing variant format (jst_ph_4, jst_sh_6) - Make PH silkscreen scale dynamically with pin count instead of hardcoded 6x5mm - Add tests for jst_ph_4 (4 plated holes), jst4_ph (4 plated holes), jst_sh_6 - Update jst2_ph snapshot for new dynamic silkscreen dimensions - All 383 tests pass
mendarb
force-pushed
the
fix-jst-ph-pin-count
branch
from
f33ea9d to
f51adc4
Compare
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
1 participant
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.
/claim #495
Summary
Fixes two issues with JST PH connectors:
Pin count parsing: Added support for
jst_ph_4format (variant_pincount with underscore separator). Previously onlyjst4_phworked. Now both formats are supported for all JST variants (PH, SH, ZH).Dynamic silkscreen sizing: The PH silkscreen body was hardcoded to a fixed 6x5mm box regardless of pin count. Now it scales dynamically based on the pin span, matching how the ZH variant already worked.
Changes
src/fn/jst.ts: Added regex pattern for trailing pin count format (jst_ph_4,jst_sh_6), updated PH silkscreen to use dynamic sizing based on pin spantests/jst.test.ts: Added tests forjst_ph_4(verifies 4 plated holes),jst4_ph(verifies 4 plated holes), andjst_sh_6formatsjst2_phsnapshot for new dynamic silkscreen dimensionsTest plan
jst_ph_4generates 4 plated holes with correctly sized silkscreenjst4_phstill works (backward compatible)jst_sh_6works via trailing formatjst_phandjst_shwithout pin count still throw errors