Conversation
src/footprinter.ts
Outdated
| // The regex below automatically inserts a "res" prefix so forms like | ||
| // "0603_pw1.0_ph1.1" are understood without typing "res0603". | ||
| const modifiedDef = def.replace(/^((?:\d{4}|\d{5}))(?=$|_|x)/, "res$1") | ||
| const modifiedDef = def.replace(/^((?:\d{4}|\d{5}))(?=$|_|x)/, "res") |
There was a problem hiding this comment.
Critical regex bug - the captured group reference $1 was removed. This breaks footprint string parsing for numeric patterns.
The original regex captures digits (e.g., "0603") and replaces with "res" + those digits ("res0603"). The new code replaces with just "res", dropping the captured digits entirely.
Impact: Strings like "0603" will become "res" instead of "res0603", breaking all numeric footprint string parsing.
Fix:
const modifiedDef = def.replace(/^((?:\d{4}|\d{5}))(?=$|_|x)/, "res$1")| const modifiedDef = def.replace(/^((?:\d{4}|\d{5}))(?=$|_|x)/, "res") | |
| const modifiedDef = def.replace(/^((?:\d{4}|\d{5}))(?=$|_|x)/, "res$1") |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
…eping pdip registration
| for (let i = 0; i < params.num_pins; i++) { | ||
| const isLeft = i < params.num_pins / 2 | ||
| const x = isLeft ? -w / 2 + pl / 2 : w / 2 - pl / 2 | ||
| const y = ((i % 2 === 0 ? 1 : -1) * p) / 2 | ||
| pads.push(rectpad(i + 1, x, y, pl, pw)) | ||
| } |
There was a problem hiding this comment.
Pin positioning logic only works correctly for 4-pin packages (2 per side). For packages with more pins per side (e.g., 6-pin, 8-pin), the i % 2 calculation incorrectly alternates y-positions instead of properly spacing multiple pins. For example, with 6 pins (3 per side), pins would be at y positions p/2, -p/2, p/2 on each side instead of being evenly spaced.
Either:
- Document that this only supports 4-pin packages, or
- Fix the calculation to properly handle multiple pins per side:
const pinsPerSide = params.num_pins / 2
const pinIndexOnSide = i % pinsPerSide
const y = (pinIndexOnSide - (pinsPerSide - 1) / 2) * p| for (let i = 0; i < params.num_pins; i++) { | |
| const isLeft = i < params.num_pins / 2 | |
| const x = isLeft ? -w / 2 + pl / 2 : w / 2 - pl / 2 | |
| const y = ((i % 2 === 0 ? 1 : -1) * p) / 2 | |
| pads.push(rectpad(i + 1, x, y, pl, pw)) | |
| } | |
| for (let i = 0; i < params.num_pins; i++) { | |
| const isLeft = i < params.num_pins / 2 | |
| const x = isLeft ? -w / 2 + pl / 2 : w / 2 - pl / 2 | |
| const pinsPerSide = params.num_pins / 2 | |
| const pinIndexOnSide = i % pinsPerSide | |
| const y = (pinIndexOnSide - (pinsPerSide - 1) / 2) * p | |
| pads.push(rectpad(i + 1, x, y, pl, pw)) | |
| } | |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
src/fn/jst.ts
Outdated
| // Robust parsing: find any sequence of digits after 'jst' or variant prefix | ||
| const jstMatch = str.match(/jst.*(\d+)/i) | ||
| if (jstMatch && jstMatch[1]) { | ||
| numPins = parseInt(jstMatch[1], 10) |
There was a problem hiding this comment.
Critical bug in pin number parsing regex. The new regex /jst.*(\d+)/i will incorrectly match digits from parameter values instead of the pin count.
For example:
jst_ph_2_p2.54could match2fromp2.54instead of the pin count2jst_ph_4_w6.0could match6or4unpredictably
The old regex /(?:^|_)jst(\d+)(?:_|$)/ correctly required word boundaries around the digit sequence.
// Fix: Revert to the more precise regex or improve it
const match = str.match(/(?:^|_)jst(\d+)(?:_|$)/)
if (match && match[1]) {
numPins = parseInt(match[1], 10)
}| // Robust parsing: find any sequence of digits after 'jst' or variant prefix | |
| const jstMatch = str.match(/jst.*(\d+)/i) | |
| if (jstMatch && jstMatch[1]) { | |
| numPins = parseInt(jstMatch[1], 10) | |
| // Robust parsing: find any sequence of digits after 'jst' or variant prefix | |
| const jstMatch = str.match(/(?:^|_)jst(\d+)(?:_|$)/i) | |
| if (jstMatch && jstMatch[1]) { | |
| numPins = parseInt(jstMatch[1], 10) |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
| if ( | ||
| Object.keys(target).length === 0 || | ||
| prop === "pdip" || | ||
| prop === "pdip8" | ||
| ) { |
There was a problem hiding this comment.
The special case for pdip and pdip8 breaks the footprinter's chain logic. This condition allows these functions to be called even when target already contains a function definition, which would overwrite an existing footprint mid-chain.
For example, fp().dip(8).pdip() would incorrectly change from a dip to a pdip footprint instead of setting a parameter.
Fix:
if (Object.keys(target).length === 0) {
if (`${prop}${v}` in FOOTPRINT_FN) {
target[`${prop}${v}`] = true
target.fn = `${prop}${v}`
} else {
target[prop] = true
target.fn = prop
if (prop === "res" || prop === "cap") {
// ... existing logic
} else {
target.num_pins = Number.isNaN(Number.parseFloat(v))
? undefined
: Number.parseFloat(v)
}
}
}The special case should be removed since pdip and pdip8 are properly registered in FOOTPRINT_FN and will work correctly with the normal flow.
| if ( | |
| Object.keys(target).length === 0 || | |
| prop === "pdip" || | |
| prop === "pdip8" | |
| ) { | |
| if (Object.keys(target).length === 0) { |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
|
Hi! This PR is ready for review and merge. All checks pass. If any changes are needed, I'm happy to make them. Thank you!\n\nPayment info: Wise or USDC/SOL to GKqwBPH4m6bkZj35j3WVvyJHHdUxddtmETH6ZA8W1aZH (PayPal unavailable in Uzbekistan). |
Implements the PDIP-8 footprint as requested in issue #371.
pdipandpdip8footprint functions.pdip8uses standard dimensions: pitch 2.54mm, row spacing 7.62mm (300mil).src/fn/index.tsandsrc/footprinter.ts.tests/pdip.test.ts.