diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 3c853fc..7462008 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -2123,6 +2123,87 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + @staticmethod + def _apply_mirror_to_symbol_sexp( + schematic_path: str, + reference: str, + mirror: Optional[str], + ) -> bool: + """ + Apply or remove (mirror x/y) on a placed symbol instance by direct + S-expression manipulation of the .kicad_sch file. + + KiCad stores mirroring as a sibling token to (at ...): + (symbol ... (at x y rot) (mirror x) ...) + kicad-skip has no API for this, so we patch the raw text. + + Args: + schematic_path: Absolute path to .kicad_sch file + reference: Reference designator, e.g. "Q1" + mirror: "x", "y", or None (None removes any existing mirror) + + Returns: + True on success, False if symbol not found. + """ + import re + + with open(schematic_path, "r", encoding="utf-8") as f: + content = f.read() + + def find_symbol_block(text: str, ref: str): + """Return (start, end) indices of the symbol block for ref.""" + i = 0 + n = len(text) + while i < n: + idx = text.find("(symbol ", i) + if idx == -1: + break + depth = 0 + j = idx + while j < n: + if text[j] == "(": + depth += 1 + elif text[j] == ")": + depth -= 1 + if depth == 0: + block = text[idx : j + 1] + ref_pattern = ( + r'\(property\s+"Reference"\s+"' + re.escape(ref) + r'"' + ) + if re.search(ref_pattern, block): + return idx, j + 1 + break + j += 1 + i = idx + 1 + return None, None + + start, end = find_symbol_block(content, reference) + if start is None: + logger.warning(f"Symbol '{reference}' not found in {schematic_path}") + return False + + block = content[start:end] + + # Remove any existing (mirror ...) from block + block = re.sub(r"\s*\(mirror\s+[xy]\)", "", block) + + # Insert new mirror token right after the (at ...) token + if mirror in ("x", "y"): + block = re.sub( + r"(\(at\s+[\d\.\-]+\s+[\d\.\-]+\s+[\d\.\-]+\))", + r"\1" + f" (mirror {mirror})", + block, + count=1, + ) + + new_content = content[:start] + block + content[end:] + + with open(schematic_path, "w", encoding="utf-8", newline="\n") as f: + f.write(new_content) + + logger.info(f"Applied mirror={mirror!r} to symbol '{reference}' in {schematic_path}") + return True + def _handle_rotate_schematic_component(self, params: Dict[str, Any]) -> Dict[str, Any]: """Rotate a schematic component""" logger.info("Rotating schematic component") @@ -2130,7 +2211,7 @@ class KiCADInterface: schematic_path = params.get("schematicPath") reference = params.get("reference") angle = params.get("angle", 0) - mirror = params.get("mirror") + mirror = params.get("mirror") # "x", "y", or None if not schematic_path or not reference: return { @@ -2152,17 +2233,26 @@ class KiCADInterface: pos[2] = angle symbol.at.value = pos - if mirror: - if hasattr(symbol, "mirror"): - symbol.mirror.value = mirror - else: - logger.warning( - f"Mirror '{mirror}' requested for {reference}, " - f"but symbol has no mirror attribute; skipped" - ) - SchematicManager.save_schematic(schematic, schematic_path) - return {"success": True, "reference": reference, "angle": angle} + + # Apply mirror via direct S-expression manipulation + # (kicad-skip has no API for setting mirror on a placed symbol) + if mirror is not None: + success = KiCADInterface._apply_mirror_to_symbol_sexp( + schematic_path, reference, mirror + ) + if not success: + return { + "success": False, + "message": f"Rotation applied but mirror failed for '{reference}'", + } + + return { + "success": True, + "reference": reference, + "angle": angle, + "mirror": mirror, + } return {"success": False, "message": f"Component {reference} not found"} diff --git a/tests/test_rotate_schematic_mirror.py b/tests/test_rotate_schematic_mirror.py new file mode 100644 index 0000000..78a8034 --- /dev/null +++ b/tests/test_rotate_schematic_mirror.py @@ -0,0 +1,171 @@ +""" +Unit tests for rotate_schematic_component mirror fix. + +Verifies that _apply_mirror_to_symbol_sexp correctly adds, removes +and toggles (mirror x/y) in .kicad_sch S-expression files. +""" + +import os +import sys +import importlib.util +import tempfile +import textwrap +from unittest.mock import patch, MagicMock + +# Stub KiCad-only and server-only modules so kicad_interface can be +# imported in a plain Python environment. +_pcbnew_mock = MagicMock() +_pcbnew_mock.__file__ = "/fake/pcbnew.so" # prevents AttributeError at module level +_pcbnew_mock.GetBuildVersion.return_value = "9.0.0" +sys.modules.setdefault("pcbnew", _pcbnew_mock) + +for _modname in ("skip", "resources", "schemas", + "resources.resource_definitions", "schemas.tool_schemas"): + sys.modules.setdefault(_modname, MagicMock()) + +sys.modules["resources.resource_definitions"].RESOURCE_DEFINITIONS = {} +sys.modules["resources.resource_definitions"].handle_resource_read = MagicMock() +sys.modules["schemas.tool_schemas"].TOOL_SCHEMAS = [] + +# Import KiCADInterface directly, bypassing package __init__ chains +_spec = importlib.util.spec_from_file_location( + "kicad_interface", + os.path.join(os.path.dirname(__file__), "..", "python", "kicad_interface.py"), +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) +KiCADInterface = _mod.KiCADInterface +SchematicManager = _mod.SchematicManager + +# --------------------------------------------------------------------------- +# Minimal .kicad_sch snippets +# --------------------------------------------------------------------------- + +SCHEMATIC_TEMPLATE = textwrap.dedent("""\ + (kicad_sch (version 20250114) (generator "test") + (symbol (lib_id "Transistor_BJT:MMBT3904") + (at 75 105 0) + (unit 1) + (property "Reference" "Q1" (at 0 0 0)) + (property "Value" "MMBT3904" (at 0 0 0)) + ) + ) +""") + +SCHEMATIC_TEMPLATE_MIRRORED_X = textwrap.dedent("""\ + (kicad_sch (version 20250114) (generator "test") + (symbol (lib_id "Transistor_BJT:MMBT3904") + (at 75 105 0) (mirror x) + (unit 1) + (property "Reference" "Q1" (at 0 0 0)) + (property "Value" "MMBT3904" (at 0 0 0)) + ) + ) +""") + + +def _write_temp_sch(content: str) -> str: + fd, path = tempfile.mkstemp(suffix=".kicad_sch") + os.close(fd) + with open(path, "w", encoding="utf-8") as f: + f.write(content) + return path + + +def _read(path: str) -> str: + with open(path, encoding="utf-8") as f: + return f.read() + + +# --------------------------------------------------------------------------- +# Tests for _apply_mirror_to_symbol_sexp +# --------------------------------------------------------------------------- + +def test_apply_mirror_x_adds_token(): + """mirror='x' should insert (mirror x) after the (at ...) token.""" + path = _write_temp_sch(SCHEMATIC_TEMPLATE) + try: + result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "x") + assert result is True + content = _read(path) + assert "(mirror x)" in content + assert "(mirror y)" not in content + finally: + os.unlink(path) + + +def test_apply_mirror_y_adds_token(): + """mirror='y' should insert (mirror y) after the (at ...) token.""" + path = _write_temp_sch(SCHEMATIC_TEMPLATE) + try: + result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "y") + assert result is True + content = _read(path) + assert "(mirror y)" in content + assert "(mirror x)" not in content + finally: + os.unlink(path) + + +def test_apply_mirror_none_removes_existing(): + """mirror=None should remove an existing (mirror x) token.""" + path = _write_temp_sch(SCHEMATIC_TEMPLATE_MIRRORED_X) + try: + result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", None) + assert result is True + content = _read(path) + assert "(mirror x)" not in content + assert "(mirror y)" not in content + finally: + os.unlink(path) + + +def test_apply_mirror_x_replaces_existing_y(): + """mirror='x' should replace an existing (mirror y) without duplication.""" + path = _write_temp_sch(SCHEMATIC_TEMPLATE_MIRRORED_X.replace("mirror x", "mirror y")) + try: + result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "Q1", "x") + assert result is True + content = _read(path) + assert content.count("(mirror x)") == 1 + assert "(mirror y)" not in content + finally: + os.unlink(path) + + +def test_apply_mirror_returns_false_for_unknown_reference(): + """Should return False gracefully when reference not found.""" + path = _write_temp_sch(SCHEMATIC_TEMPLATE) + try: + result = KiCADInterface._apply_mirror_to_symbol_sexp(path, "U99", "x") + assert result is False + assert "(mirror x)" not in _read(path) + finally: + os.unlink(path) + + +def test_rotate_handler_no_mirror_still_works(tmp_path): + """rotate_schematic_component without mirror param should not crash.""" + sch_path = str(tmp_path / "test.kicad_sch") + with open(sch_path, "w") as f: + f.write(SCHEMATIC_TEMPLATE) + + iface = KiCADInterface.__new__(KiCADInterface) + + with patch.object(_mod.SchematicManager, "load_schematic") as mock_load, \ + patch.object(_mod.SchematicManager, "save_schematic"): + mock_sym = MagicMock() + mock_sym.property.Reference.value = "Q1" + mock_sym.at.value = [75, 105, 0] + mock_sch = MagicMock() + mock_sch.symbol = [mock_sym] + mock_load.return_value = mock_sch + + result = iface._handle_rotate_schematic_component({ + "schematicPath": sch_path, + "reference": "Q1", + "angle": 90, + }) + + assert result["success"] is True + assert result["angle"] == 90