Fix courtyards for bga, quad, qfn,soic, etc#581
Conversation
src/fn/quad.ts
Outdated
| }, | ||
| { | ||
| x: -roundCourtyardCoord(courtyardOuterHalfWidthMm), | ||
| y: -roundCourtyardCoord(courtyardInnerHalfHeightMm), |
There was a problem hiding this comment.
This can drfinitely be cleaned up into a function- what is it doing?
src/fn/vson.ts
Outdated
| y: -courtyardInnerHalfHeightMm, | ||
| }, | ||
| ], | ||
| layer: "top" as const, |
There was a problem hiding this comment.
If this is a pill shape just have a method for creating pill points thqt you import
src/fn/quad.ts
Outdated
| w: length.optional(), | ||
| h: length.optional(), | ||
| courtyard_w_mm: length.optional(), | ||
| courtyard_h_mm: length.optional(), |
There was a problem hiding this comment.
These params cannot be in a def and have to be removed. You can infer them- params can never have underscores
There was a problem hiding this comment.
You should infer them from other params by default, but you could add courtyardwidth and courtyardheight as params since that is legal (no underscores)
src/fn/vson.ts
Outdated
| near(pinw, 0.7) && | ||
| near(pinh, 0.7) | ||
| ) | ||
| })() |
There was a problem hiding this comment.
This looks like a bad hack?
src/fn/qfn.ts
Outdated
| if (parameters.courtyard_w_mm == null) parameters.courtyard_w_mm = 5 | ||
| if (parameters.courtyard_h_mm == null) parameters.courtyard_h_mm = 5 | ||
| if (parameters.w == null) parameters.w = 5.96 | ||
| if (parameters.h == null) parameters.h = 5.96 |
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
Break this up. Most of this is ok but there is some stuff inserted that looks not ok, looks like it wasnt reviewed.
| ], | ||
| layer: "top" as const, | ||
| } | ||
| })() |
There was a problem hiding this comment.
this is called an IIFE or immediately invoked function expression and it's an anti-pattern in most cases, can you write this more cleanly? Without a ternary?
| parameters.pl === 0.85 && | ||
| parameters.pw === 0.3 | ||
|
|
||
| const courtyard: PcbCourtyardOutline = useManualCourtyardForDfn8_2x2 |
There was a problem hiding this comment.
we don't need to handle this specific scenario i think- it's a hack
| { x: -1.75, y: -1.43 }, | ||
| { x: -3.18, y: -1.43 }, | ||
| ] | ||
| : null |
There was a problem hiding this comment.
remove hacks!!!!! People worked very hard to make this code clean, and you're inserting a large hack that makes it not usable for others, find a courtyard that closely matches kicad for the generic case, do not hack in conditions
| w: length.default(length.parse(newDefaults.w ?? "5.3mm")), | ||
| h: newDefaults.h | ||
| ? length.default(length.parse(newDefaults.h)) | ||
| : length.optional(), |
seveibar
left a comment
seveibar
left a comment
There was a problem hiding this comment.
just PR one thing at a time, for a large PR the code has to be pristine, but there are ternaries (almost always bad) and lots of hacks
2 participants
No description provided.