Skip to content

fix: bind converted KiCad reference text to the instance name - #4710

Open
billythompsons wants to merge 3 commits into
tscircuit:mainfrom
billythompsons:fix-4708-convert-ref-placeholder
Open

billythompsons wants to merge 3 commits into
tscircuit:mainfrom
billythompsons:fix-4708-convert-ref-placeholder

Conversation

@billythompsons

Copy link
Copy Markdown

Fixes #4708.

Problem

tsci convert copies the KiCad fp_text reference placeholder into a literal text="REF**", so every placed instance of the converted footprint prints "REF**" instead of its reference designator.

Fix

After convertCircuitJsonToTscircuit runs, the KiCad reference placeholder (text="REF**" on a silkscreentext) is rebound to the component instance: text={props.name ?? "REF**"}. Ordinary user text stays literal, and the rewrite only applies when the generated component has props in scope.

Kept this in the CLI's convert path since the placeholder is a KiCad artifact known at conversion time; happy to move it into circuit-json-to-tscircuit if you'd rather handle it there.

Tests

  • convert-kicad-mod-reference-text.test.ts: converts the issue's two-pad .kicad_mod plus an fp_text user control - asserts the reference placeholder is rebound and user text stays literal.
  • convert-kicad-mod-reference-e2e.test.ts: builds a board with two instances (U1, U2) and asserts the emitted pcb_silkscreen_text values are exactly ["U1", "U2"] (the issue's failing scenario).

Full tests/cli/convert suite passes.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@billythompsons

Copy link
Copy Markdown
Author

Still active and ready for review. All six checks are green; the e2e test builds two instances of the converted footprint and verifies the silkscreen refs resolve to U1/U2 instead of the REF** placeholder.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@billythompsons

Copy link
Copy Markdown
Author

Status update: the focused regression fix for the requested CLI behavior remains green in CI and ready for review. I can rebase or make any requested changes promptly.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@billythompsons

Copy link
Copy Markdown
Author

Status update: the focused CLI regression fix remains green in CI and ready for review. I can rebase or make any requested changes promptly.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

@billythompsons

Copy link
Copy Markdown
Author

Rechecked against current main (fcbf759): the branch still merges cleanly and #4708 still reproduces on main - tsci convert on a KiCad footprint emits the literal REF** silkscreen text, so every instance renders the placeholder instead of U1/U2. This PR binds the converted reference text to the instance name, and the e2e test covers the two-instance case. Glad to rebase or adjust if you'd prefer a different approach.

@github-actions

Copy link
Copy Markdown
Contributor

This PR has been automatically marked as stale because it has had no recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tsci convert hardcodes KiCad reference placeholder REF** in reusable TSX footprints

1 participant