From 457e4e30ad37a69408497192d2e7c87ce0c7788a Mon Sep 17 00:00:00 2001 From: Gavin Colonese Date: Mon, 18 May 2026 23:00:22 -0400 Subject: [PATCH] fix(ipc): rotate_component uses absolute angle (matches schema) (#159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: IPC rotate_component now uses absolute angle as documented The IPC rotate handler was adding the angle to the current rotation (relative), but the schema documents it as absolute. This caused unexpected behavior where setting angle=0 had no effect on a component already at 180°. Now correctly sets the rotation to the exact angle specified, matching the SWIG backend behavior. Co-Authored-By: Claude Opus 4.6 (1M context) * docs(changelog): add unreleased entry for rotate_component absolute-angle fix --------- Co-authored-by: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 7 +++ python/kicad_interface.py | 5 +- tests/test_ipc_rotate_absolute.py | 76 +++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 tests/test_ipc_rotate_absolute.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 691ca96..886a521 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ All notable changes to the KiCAD MCP Server project are documented here. ### Bug Fixes +- **`rotate_component` now treats `angle` as an absolute target rotation**, + matching its schema description. Previously the IPC backend added the + supplied angle to the current rotation, so two consecutive + `rotate_component(angle=90)` calls would rotate the part to 180° instead + of leaving it at 90°. Workflows that relied on the additive behavior will + need to be updated. + - **Project-scope `sym-lib-table` is now visible to symbol-discovery tools**: `search_symbols`, `list_symbol_libraries`, `list_library_symbols`, and `get_symbol_info` previously only consulted the global `sym-lib-table`. A diff --git a/python/kicad_interface.py b/python/kicad_interface.py index aa26339..d0940eb 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -5538,9 +5538,8 @@ print("ok") if not target: return {"success": False, "message": f"Component {reference} not found"} - # Calculate new rotation - current_rotation = target.get("rotation", 0) - new_rotation = (current_rotation + angle) % 360 + # Use angle as absolute rotation (matches schema description) + new_rotation = angle % 360 # Use move_component with new rotation (position stays the same) success = self.ipc_board_api.move_component( diff --git a/tests/test_ipc_rotate_absolute.py b/tests/test_ipc_rotate_absolute.py new file mode 100644 index 0000000..7a5c9e4 --- /dev/null +++ b/tests/test_ipc_rotate_absolute.py @@ -0,0 +1,76 @@ +"""Regression test for IPC rotate_component absolute-angle behavior. + +Bug: ``_ipc_rotate_component`` accumulated the supplied angle on top of the +current rotation, but the tool schema documents ``rotation`` as the absolute +target angle. Two consecutive ``rotate_component(angle=90)`` calls would land +the part at 180° instead of leaving it at 90°. +""" + +import sys +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + + +def _make_iface(current_rotation: float) -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", True): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + + iface.use_ipc = True + iface.board = None + iface.ipc_board_api = MagicMock() + iface.ipc_board_api.list_components.return_value = [ + { + "reference": "R1", + "position": {"x": 10.0, "y": 20.0, "unit": "mm"}, + "rotation": current_rotation, + } + ] + iface.ipc_board_api.move_component.return_value = True + return iface + + +def test_rotation_is_absolute_not_additive(): + """angle=90 sets rotation to 90° regardless of current rotation.""" + iface = _make_iface(current_rotation=270) + result = iface._ipc_rotate_component({"reference": "R1", "angle": 90}) + + assert result["success"] is True + assert result["newRotation"] == 90 + _, kwargs = iface.ipc_board_api.move_component.call_args + assert kwargs["rotation"] == 90 + + +def test_rotation_is_absolute_when_current_is_zero(): + iface = _make_iface(current_rotation=0) + result = iface._ipc_rotate_component({"reference": "R1", "angle": 180}) + assert result["newRotation"] == 180 + + +def test_rotation_normalized_to_modulo_360(): + """Values >= 360 are normalized.""" + iface = _make_iface(current_rotation=0) + result = iface._ipc_rotate_component({"reference": "R1", "angle": 450}) + assert result["newRotation"] == 90 + + +def test_position_preserved_during_rotate(): + """Rotating must not move the part.""" + iface = _make_iface(current_rotation=0) + iface._ipc_rotate_component({"reference": "R1", "angle": 90}) + _, kwargs = iface.ipc_board_api.move_component.call_args + assert kwargs["x"] == 10.0 + assert kwargs["y"] == 20.0 + + +def test_unknown_reference_returns_failure(): + iface = _make_iface(current_rotation=0) + result = iface._ipc_rotate_component({"reference": "DOES_NOT_EXIST", "angle": 90}) + assert result["success"] is False + iface.ipc_board_api.move_component.assert_not_called()