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>
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>
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.
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>
Mechanical application of the `.gitattributes` rules from the prior commit.
All 50 files differ only in line endings — verified by
`git diff --cached --ignore-all-space` being empty.
Before: main had 42 CRLF + 27 LF Python files plus mixed-ending in YAML,
templates, and shell scripts. After: every text file is LF (except the
Windows-native *.ps1, *.bat scripts which remain CRLF per gitattributes).
This eliminates the noisy-diff failure mode seen in PR #102, where a
small logic change produced a 918-line diff due to whole-file CRLF→LF
conversion.
Add type annotations to all previously untyped functions and remove 9
suppressed error codes (call-arg, assignment, return-value, operator,
has-type, dict-item, misc, list-item, annotation-unchecked) by fixing
the underlying type issues.
Add [[tool.mypy.overrides]] with ignore_missing_imports for KiCAD-specific
modules (pcbnew, sexpdata, skip, cairosvg, kipy, PIL) so the pre-commit
mypy hook passes in its isolated venv. Add types-requests and pytest to
additional_dependencies in .pre-commit-config.yaml.
Also fixes several real bugs uncovered by stricter checks: incorrect static
calls to instance methods in swig_backend, wrong return type on get_size,
missing value param in BoardAPI.place_component, variable shadowing in
kicad_process.py, unqualified LibraryManager reference in kicad_interface,
and missing top-level Path import.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When moving a schematic component whose pins directly touch pins of
stationary components (no wire segment, just pin-to-pin contact),
synthesize bridge wires to preserve the electrical connection after
the move. Also fixes duplicate-pin-position collision in old_to_new
map and updates symbol reference assignment to use setAllReferences.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When moving a schematic component, connected wires are stretched/shifted
to follow the component (like KiCAD's drag behaviour), preserving
connectivity instead of leaving dangling wire stubs.
Also fixes property labels (value, reference, etc.) so they shift with
the symbol rather than staying at their original positions.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
