The eeschema-ground-truth fixture (_build_mirror_case) passed
'mirror': 'x' to ComponentManager.add_component, which silently drops
the kwarg — so the resulting .kicad_sch had no (mirror x|y) token,
eeschema rendered an unmirrored symbol, and our pin coords (also
unmirrored) tautologically matched. The mirror tests were GREEN both
before and after the rotation/mirror fix in 7e67cb9, providing zero
regression coverage for the mirror semantics.
Fix:
- _build_mirror_case now applies the mirror via the same low-level
helper (WireDragger.update_symbol_rotation_mirror) that
rotate_schematic_component uses, with a guard assertion that the
written file actually contains (mirror x|y).
- Two new pin-down unit tests in test_add_schematic_component.py
document and lock down ComponentManager.add_component's silent-drop
behavior for mirror, so the next person to touch that path knows to
update the eeschema-truth fixture if they grow real mirror support.
Verified: with the production fix at 7e67cb9 reverted, the kicad-cli
mirror tests now go RED (previously they stayed GREEN regardless).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two bugs in WireDragger.pin_world_xy (and corresponding bugs in
PinLocator.get_pin_angle) caused pin coordinates and angles to land on
the wrong pin in 4 of 8 polarized cases (rot=90, rot=270, mirror x on a
vertical part, mirror y on a vertical part). Verified end-to-end against
`kicad-cli sch export netlist`.
(1) Rotation direction. After PR #145's `-ly` Y-flip, calling the
standard math (Y-up CCW) `_rotate` is effectively CW in screen Y-down.
eeschema's TRANSFORM(0,1,-1,0) for rot=90 is screen-CCW. They agreed at
0° and 180° (where the rotation matrices coincide) but disagreed at 90°
and 270°.
(2) Mirror axis semantics swapped. Per eeschema symbol.h:43-44,
SYM_MIRROR_X = TRANSFORM(1,0,0,-1) negates Y, and SYM_MIRROR_Y =
TRANSFORM(-1,0,0,1) negates X. Our code did the inverse: `mirror_x`
negated the X component and `mirror_y` negated the Y component.
Fix shape for `_rotate`: chose option (b) — leave `_rotate` as standard
math and negate the angle at the call site (`_rotate(lx, ly, -rotation)`).
This converts math-CCW to screen-CCW without disturbing
`TestRotatePoint`'s direct expectations of `_rotate`.
Final composition order in `pin_world_xy` matches eeschema's parser
(rotation set first into m_transform, then mirror composed via
`new = old * temp` so the mirror is applied first to the coordinate):
1. Y-flip: ly = -ly (lib Y-up → screen Y-down)
2. Mirror: if mirror_x: ly = -ly (negate screen-Y)
if mirror_y: lx = -lx (negate screen-X)
3. Rotate: _rotate(lx, ly, -rotation) (screen-CCW)
4. Translate: add (sym_x, sym_y)
Verified by hand for {rot=90, rot=270} × {none, mirror_x, mirror_y}
against the TRANSFORM matrices in transform.cpp:44 and symbol.h:43-44.
`PinLocator.get_pin_angle` mirrors the same composition in angle space.
For an angle, Y-flip and mirror_x both negate the angle; mirror_y maps
to (180 - angle). The screen-CCW rotation in `pin_world_xy` corresponds
to subtracting (not adding) the symbol rotation in standard atan2
convention — fixed accordingly. Geometry test
(`test_get_pin_angle.py::test_get_pin_angle_matches_geometric_expectation`)
derives expected angles from `pin_world_xy` itself, so it pins the two
together.
`tests/test_rotate_schematic_mirror.py::test_pin_positions_mirror_x_flips_x`
encoded the OLD inverted semantics and is updated/renamed to
`test_pin_positions_mirror_x_flips_y` with a pin that has non-zero Y so
the assertion is meaningful under the corrected semantics.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`test_rotate_handler_no_crash` permanently replaced
`sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS` with `[]`, leaking into
later tests. When test_wire_connectivity (or any test) ran after this one
and did `from schemas.tool_schemas import TOOL_SCHEMAS`, it got the empty
list and `TOOL_SCHEMAS["get_wire_connections"]` raised `TypeError: list
indices must be integers or slices, not str`.
Save the original sys.modules entries and restore them in a `finally`
block so the stubs are scoped to the test body. Whole suite now passes
(678 tests, previously 4 failed in TestSchema when run in suite order).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Use None default + 'instances or []' to keep mutable-default lint happy
and satisfy the Iterable annotation.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
All 10 failing tests in test_hierarchical_pad_net_map.py share the same root cause:
broken test fixture, not a production bug. PR #88 (commit 7cafbda, "fix:
get_schematic_pin_locations now accounts for mirror flags") introduced
PinLocator._get_symbol_transform, which reads symbol position/rotation/lib_id
directly from the .kicad_sch file via sexpdata + WireDragger.find_symbol — it
deliberately bypasses the kicad-skip cache so mirror/rotation mutations are
authoritative. The hierarchical-pad-net-map tests, however, only mocked
skip.Schematic and wrote a lib_symbols-only stub to disk with no (symbol ...)
instance, so _get_symbol_transform returned None and every pin lookup failed
with "Could not read transform for R1/R2". The tests last passed at 8a42812
(introduction) and broke at 7cafbda; PR #145 did not touch this code path.
Per-test classification (all 10): broken fixture.
- TestLabelAtPin::test_global_label_pin1 — fixture
- TestLabelAtPin::test_global_label_pin2 — fixture
- TestLabelAtPin::test_both_pins_mapped — fixture
- TestLabelAtPin::test_local_label_also_works — fixture
- TestLabelAtPin::test_hierarchical_label_also_works — fixture
- TestLabelViaWire::test_label_one_hop_away — fixture
- TestLabelViaWire::test_label_two_hops_away — fixture
- TestMultipleSubsheets::test_components_in_subsheet_collected — fixture
- TestMultipleSubsheets::test_top_and_sub_components_merged — fixture
- TestRotatedSymbol::test_90_degree_rotation — fixture (rotation expected
values already match the post-PR-145 convention; only fixture needed)
Fix: add _build_sch_with_instances() helper that emits a real (symbol ...)
block alongside the existing TestLib:R lib_symbols, so sexpdata can resolve
the transform. The skip.Schematic mock is still used for labels and wires.
All 15 tests in this file now pass; the broader related set
(test_pin_locator_y_flip, test_get_pin_angle, test_move_with_wire_preservation,
test_pin_locator_and_component) also pass — 84 total.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Resolves the four failing tests in tests/test_pin_locator_and_component.py
left behind by the PR #145 / commit 3c22580 Y-flip work.
Per-test rationale:
- TestPinLocatorYAxisNegation::{test_pin1_y_above_center_for_rotation_0,
test_pin2_y_below_center_for_rotation_0, test_pin1_rotated_90}: stale.
Their assertions encoded the *correct* post-PR-145 convention (96.19,
103.81, etc.), but their setup MagicMock'd self._schematic_cache while
bypassing _get_symbol_transform, which reads the .kicad_sch file
directly via sexpdata. The end-to-end Y-flip behaviour is already
covered against eeschema in tests/test_pin_locator_y_flip.py — keeping
three mock-based duplicates added no value, so they were removed.
- TestPinLocatorReferenceRstrip::test_get_pin_location_finds_symbol_with_trailing_underscore:
revealed a real production bug. PinLocator.get_pin_location strips a
trailing "_" on the kicad-skip lookup path, but the sexpdata-based
_get_symbol_transform delegates to WireDragger.find_symbol which used an
exact-equality comparison. With kicad-skip's "R1_" artifact the function
returned None, so the whole pin-location call failed even when the symbol
was clearly present. Fixed find_symbol to apply the same rstrip("_") on
the stored reference before comparing, mirroring the existing behaviour
in PinLocator. The test was also rewritten to use a real temp .kicad_sch
(with the on-disk reference mangled to "R1_") so it actually exercises
both lookup paths instead of bypassing one with mocks.
Files changed:
- python/commands/wire_dragger.py:78-89 — rstrip("_") on the reference
read out of the symbol property before comparing to the caller-supplied
reference.
- tests/test_pin_locator_and_component.py — removed three stale mock-based
Y-axis tests (covered by tests/test_pin_locator_y_flip.py end-to-end);
rewrote rstrip tests to use a real schematic file so _get_symbol_transform
is actually exercised.
Verified: tests/test_pin_locator_and_component.py + test_pin_locator_y_flip.py
+ test_get_pin_angle.py + test_move_with_wire_preservation.py — 69 passed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR #145 restored the Y-axis flip in WireDragger.pin_world_xy so pin
coordinates now match the schematic (Y-down) frame instead of the
library (Y-up) frame. PinLocator.get_pin_angle was the companion to
that transform but never received the matching fix: it was returning
the library-frame angle (with mirror handling but no Y-flip), so
angles came out 180° off along the Y axis.
This was masked before PR #145 because pin_world_xy was wrong in the
same direction — both functions skipped the Y-flip, so callers that
compared pin endpoints to angles saw a self-consistent picture. Once
pin_world_xy was corrected the inconsistency surfaced.
Apply the same lib→screen Y-flip (negate angle) after the mirror
handling and before the symbol-rotation add, matching pin_world_xy's
order: mirror in lib space → Y-flip → rotate → translate (no
translate for angles since angles are translation-invariant).
Fixes the 24 parametrized cases in
tests/test_get_pin_angle.py::test_get_pin_angle_matches_geometric_expectation
(pin × mirror × rotation matrix). The test derives its expected value
from pin_world_xy itself, making it the canonical geometric oracle.
test_pin_locator_y_flip and test_move_with_wire_preservation continue
to pass.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
search_symbols, list_symbol_libraries, list_library_symbols, and
get_symbol_info previously only consulted the global sym-lib-table. A
library registered with project scope (an entry in
<project>/sym-lib-table) was therefore invisible — even right after
open_project succeeded — making add_schematic_component the only tool
that could see it.
Fix has two parts:
1. Wrap project_commands.open_project and project_commands.create_project
in handlers that rebuild SymbolLibraryCommands.library_manager against
the project directory. After open_project, project-scope libraries are
automatically visible to subsequent search/list/info calls.
2. Add an optional projectPath parameter to the four discovery tools
(accepts a project directory, .kicad_pro, .kicad_pcb, or .kicad_sch
path). Stateless callers can resolve project libraries without first
calling open_project. SymbolLibraryCommands._derive_project_path also
walks up from schematicPath/boardPath to find the directory that owns
the project, mirroring the logic in _handle_add_schematic_component.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The KiCad s-expression parser rejects raw newline and carriage-return
characters inside quoted string literals — a multi-line text annotation
written through `add_text` produced a `.kicad_sch` file that eeschema
silently tolerated but `kicad-cli sch ...` refused with "Failed to load
schematic." The escape pass only handled backslashes and double quotes.
Add `\\n` → `\\\\n` and `\\r` → `\\\\r` to the same escape chain. Order
matters: backslashes are escaped first so we don't double-escape our
own escapes.
A new regression test (`test_escapes_newlines_in_multiline_text`)
checks both that the resulting quoted string literal contains no raw
newline characters and that the file round-trips cleanly through the
sexpdata parser.
End-to-end smoke: a 4-line annotation written through the patched
add_text now passes `kicad-cli sch erc` (exit 0) where the previous
behaviour failed parse.
Note: the same escape gap exists in `_make_hierarchical_label_text`
and `_make_sheet_pin_text` for unescaped quotes/newlines in the user-
supplied text. Not fixed here to keep this PR scoped to the documented
add_text bug; happy to fold it in if a reviewer prefers.
Library symbol pins are stored Y-up (positive Y is upward in the symbol
editor's coordinate system) but `.kicad_sch` is Y-down (positive Y is
downward in the schematic). `pin_world_xy` was returning `sym_y + ry`
without negating the rotated lib Y, so for any non-symmetric symbol
pin 1 and pin 2 ended up at swapped world positions.
For symmetric two-pin passives (R, C non-polarized) this was invisible
because pin 1 and pin 2 are electrically equivalent. For polarized
parts — electrolytic and polymer caps, diodes, MOSFETs, BJTs — it
silently swapped polarity. A label snapped to a polarized cap's pin 1
ended up on pin 2, which is catastrophic at first power-up.
The order matches eeschema's actual transformation:
mirror in lib space → Y-flip to screen → rotate → translate.
The existing regression test in test_pin_locator_y_flip.py was already
written with the correct expected coordinates but the matching code fix
was never landed; that test now passes.
Three tests in test_move_with_wire_preservation.py had baked the buggy
expected coordinates into their assertions; updated those to the correct
y-flipped values. The touching-pin fixture had to flip R2's Y from
-7.62 to +7.62 so the two pins still meet under the corrected formula.
Verified end-to-end on a 46-component aerospace PDB schematic: all
8 polarized-part pins (4 polymer caps + 4 TVS diodes) now produce
world coordinates that match the labels actually placed in the file.
1. add_schematic_net_label failed on schematics with no existing labels.
The clone-based path required a pre-existing label to copy from;
the documented "fallback to sexpdata" was a misleading log line —
the RuntimeError was caught and the call silently returned False.
Restore hand-built sexpdata construction (without the buggy
fields_autoplaced token, with orientation-aware justify).
2. get_pin_angle returned the wrong angle for every mirrored symbol
(off by exactly 180°, all rotations, both mirror axes). The
mirror_x and mirror_y formulas were swapped relative to the
pin_world_xy convention — pin_world_xy mirrors a position by
flipping its local axis component, so the matching angle
transform is (180 - θ) for mirror_x and -θ for mirror_y.
Add regression tests:
- test_add_label_empty_schematic.py — first label on empty schematic,
orientation-aware justify.
- test_get_pin_angle.py — full 24-case matrix
(4 rotations × 3 mirror states × 2 pins).
Upstream added `from annotations import AnnotationLoader` and moved
`from commands.wire_manager import WireManager` to module-level in
kicad_interface.py. The smoke test now stubs annotations and ensures
python/ is on sys.path so commands.* imports resolve without installing.
Previously get_pin_location and get_pin_angle read symbol state from a
kicad-skip cache that does not reflect (mirror x/y) tokens written by
rotate_schematic_component. Pin coordinates were always computed as if
the symbol was unmirrored.
Fix:
- Added _get_symbol_transform() which reads position, rotation, mirror_x,
mirror_y, and lib_id directly from the .kicad_sch file via sexpdata +
WireDragger.find_symbol (the authoritative source after a rotate/mirror)
- get_pin_location now delegates the full transform (mirror → rotate →
translate) to WireDragger.pin_world_xy, matching the logic used by
move_schematic_component and rotate_schematic_component
- get_pin_angle now applies mirror-induced angle reflection before adding
symbol rotation: mirror_x negates the angle, mirror_y reflects across 180°
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two bugs fixed:
1. fields_autoplaced yes was always injected — caused incorrect visual
rendering of label text in KiCAD. Removed by using clone() which
copies an existing label without that field.
2. (justify left bottom) was hardcoded regardless of orientation.
For orientation 180/270 KiCAD requires (justify right bottom).
Now set correctly via new_label.effects.justify._tree[1].
Implementation switches from manual sexpdata list construction to
kicad-skip Schematic.label[0].clone(), which produces a structurally
correct label that KiCAD can round-trip without modification.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the handler used kicad-skip to apply rotation and mirror.
kicad-skip has no API for (mirror x/y) on placed symbols, causing:
'NoneType' object has no attribute 'value'
Fix:
- Rewrote _handle_rotate_schematic_component to use sexpdata (same
approach as move_schematic_component) for both rotation and mirror
- Added WireDragger.compute_pin_positions_for_rotation: computes old
and new pin world positions when rotation/mirror changes at fixed (x,y)
- Added WireDragger.update_symbol_rotation_mirror: updates (at) rotation
and adds/removes/replaces the (mirror x/y) sexpdata token cleanly
- Connected wires now follow pin positions after rotate/mirror via the
existing WireDragger.drag_wires infrastructure
Tests: 10 unit tests in tests/test_rotate_schematic_mirror.py covering
update_symbol_rotation_mirror, compute_pin_positions_for_rotation, and
a handler smoke test.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PWR_FLAG instances use a #FLG reference prefix, not #PWR, so their
pin positions were never registered as virtual connection anchors in
_parse_virtual_connections. As a result, find_orphaned_wires reported
wire ends terminating on a PWR_FLAG as dangling. Other call sites
(schematic_analysis.py:127, kicad_interface.py:3814) already recognize
#FLG as a power symbol; align this site with them.
Also coerce a previously-validated Optional[int] to int in board/layers.py
so the file passes mypy (required by the pre-commit hook); behavior is
unchanged because the value is already None-checked above.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The disclaimer is project-stance commentary, not a changelog entry; moved
to local notes outside the repo.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Refactor _handle_rotate_schematic_component to use raw sexp throughout
and write the schematic once instead of three times
- Hoist sexpdata and WireManager imports to module scope; drop the
unnecessary underscore aliases in move/rotate handlers
- Move WireManager._SUB_UNIT_RE to the top of the class body
- Promote the per-call Symbol("symbol")/Symbol("unit") allocations in
_parse_lib_pins / _collect_pin_positions to module-level _SYM_*
constants
- Document the assumption behind _SUB_UNIT_RE's greedy match
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replaces the manual add_schematic_junction tool with automatic junction
management. WireManager.sync_junctions inserts/removes junction dots
based on wire endpoints plus component pin positions and is invoked
after add_wire, add_polyline_wire, delete_wire, move, and rotate.
- Pin-aware: parses lib_symbols and applies KiCad's mirror/rotate/
translate transform to compute world pin coordinates
- Multi-unit safe: filters lib_symbols sub-units by the placed
symbol's (unit N) field plus the unit-0 common body
- Removes the now-unused WireManager.add_junction static method
- Updates CHANGELOG [Unreleased] with the tool removal notice
- Adds .mcp.json to .gitignore (machine-local paths)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Configuring logging at module import time hard-failed in environments
without write access to the user's home directory (sandboxed pytest
runners, restricted CI containers, read-only filesystems). The
unhandled OSError/PermissionError aborted module import and cascaded
into ~100 test failures during collection.
Wrap the FileHandler setup in try/except and fall back to console-only
logging when the log directory cannot be created. Production behavior
is unchanged - file logging in ~/.kicad-mcp/logs is still used
whenever the directory is writable.
Also picks up an isort-driven import reorder applied by the project's
pre-commit hook.
The symbol-to-schematic y-flip was applied twice in sequence (two identical
negation blocks with matching comments), cancelling out and leaving pin
Y-coordinates mirrored about the placement Y. For symmetric passives the
bug is invisible (pin 1 and pin 2 are electrically interchangeable); for
ICs with non-equivalent pins (power pins, opamp inputs, etc.) this causes
tools that go through PinLocator — connect_to_net, add_schematic_connection,
add_schematic_net_label with componentRef+pinNumber — to place connections
at the mirror-flipped pin. Verified against kicad-cli generate_netlist
ground truth: on a Device:R placed at (111.76, 83.82), pin 1 resolves to
y=80.01 (actual) vs y=87.63 (pre-fix).
This is a regression of PR #103, which originally fixed the y-negation;
the redundant second block was added subsequently.
Includes a regression test with both a straight Device:R and a rotated
Device:C to exercise the y-flip + rotation pipeline.
connect_to_net and connect_passthrough previously only touched the
schematic: a wire stub plus a net label were added via
ConnectionManager, but the corresponding pad on the .kicad_pcb was
never updated.
pcbnew.SaveBoard() silently drops every net that is not referenced by
at least one board element (pad/track/via/zone). A net that exists
only in the schematic therefore disappears on save, and in KiCad the
board's ratsnest never shows anything.
Fix:
- new _assign_net_to_pad(component_ref, pin_name, net_name):
ensures the net exists on the board (creates NETINFO_ITEM if not)
and sets it on the matching pad of the matching footprint.
- _handle_connect_to_net calls it after the schematic op when
self.board is loaded.
- _handle_connect_passthrough parses ConnectionManager's 'connected'
entries ('J1/1 <-> J2/1 [PIN_1]') and assigns nets to both pads.
- register 'connect_to_net' in _BOARD_MUTATING_COMMANDS so the
existing auto-save path persists the pad assignment to disk.
Passing net_name='*' now deletes all tracks on the board (respecting
the include_vias flag). Useful for wiping a test layout before
re-routing without having to iterate every net by name.
board.GetLayerStack() was removed in KiCad 9. Call SetLayerName and
SetLayerType directly on the board instead, and grow the copper layer
count via SetCopperLayerCount when adding inner layers. Without this,
add_layer raises AttributeError on any KiCad 9 installation.
