From b3ca93a98db3d994ec37abb5812b0c6cdc3b1152 Mon Sep 17 00:00:00 2001 From: Michael Parment Date: Wed, 8 Apr 2026 10:27:53 +0200 Subject: [PATCH 1/5] fix: create_schematic now respects the path parameter Previously the path argument to create_schematic() was accepted by the tool schema but silently ignored in SchematicManager.create_schematic() (python/commands/schematic.py). The schematic file was always written using only the bare filename, resolving to the MCP server's working directory. On systems where that directory is not writable (e.g. Program Files, OneDrive-synced folders) this caused: [Errno 13] Permission denied: 'myproject.kicad_sch' Fix: - Add path: Optional[str] = None parameter to SchematicManager.create_schematic() - Compute output_path as os.path.join(path, base_name) when path is given - Forward the resolved path from _handle_create_schematic() into create_schematic() Tests: added tests/test_create_schematic_path.py with three unit tests covering: path respected, no-path fallback, and no double-suffix on .kicad_sch names. Co-Authored-By: Claude Sonnet 4.6 --- python/commands/schematic.py | 5 +- python/kicad_interface.py | 3 +- tests/test_create_schematic_path.py | 89 +++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 tests/test_create_schematic_path.py diff --git a/python/commands/schematic.py b/python/commands/schematic.py index 19a801a..5273bcb 100644 --- a/python/commands/schematic.py +++ b/python/commands/schematic.py @@ -12,7 +12,7 @@ class SchematicManager: """Core schematic operations using kicad-skip""" @staticmethod - def create_schematic(name, metadata=None): + def create_schematic(name, path=None, metadata=None): """Create a new empty schematic from template""" try: # Determine template path (use template_with_symbols for component cloning support) @@ -24,7 +24,8 @@ class SchematicManager: ) # Determine output path - output_path = name if name.endswith(".kicad_sch") else f"{name}.kicad_sch" + base_name = name if name.endswith(".kicad_sch") else f"{name}.kicad_sch" + output_path = os.path.join(path, base_name) if path else base_name if os.path.exists(template_path): # Copy template to target location diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 1c14710..2a3155e 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -614,7 +614,8 @@ class KiCADInterface: "message": "Schematic name is required. Provide 'name', 'projectName', or 'filename' parameter.", } - schematic = SchematicManager.create_schematic(project_name, metadata) + sch_path = path if path and path != "." else None + schematic = SchematicManager.create_schematic(project_name, path=sch_path, metadata=metadata) file_path = f"{path}/{project_name}.kicad_sch" success = SchematicManager.save_schematic(schematic, file_path) diff --git a/tests/test_create_schematic_path.py b/tests/test_create_schematic_path.py new file mode 100644 index 0000000..b2f911e --- /dev/null +++ b/tests/test_create_schematic_path.py @@ -0,0 +1,89 @@ +""" +Unit tests for create_schematic path parameter bug fix. + +Verifies that create_schematic respects the `path` argument and writes +the schematic file to the correct directory instead of the process cwd. +""" + +import os +import sys +import importlib.util +import tempfile +from unittest.mock import patch, MagicMock + +# pcbnew and skip are only available inside KiCAD — stub them so the +# schematic module can be imported in a plain Python environment. +sys.modules.setdefault("pcbnew", MagicMock()) +sys.modules.setdefault("skip", MagicMock()) + +# Import the module directly (bypasses python/commands/__init__.py which +# would otherwise pull in board/component commands that also need pcbnew). +_spec = importlib.util.spec_from_file_location( + "schematic_module", + os.path.join(os.path.dirname(__file__), "..", "python", "commands", "schematic.py"), +) +_mod = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(_mod) +SchematicManager = _mod.SchematicManager + +_OPEN_MOCK = MagicMock(return_value=MagicMock( + __enter__=MagicMock(return_value=MagicMock( + read=MagicMock(return_value="(uuid 00000000-0000-0000-0000-000000000000)") + )), + __exit__=MagicMock(return_value=False), +)) + + +def test_create_schematic_uses_path_argument(): + """ + create_schematic should write the .kicad_sch file inside `path` + when that argument is provided, not in the process working directory. + """ + with tempfile.TemporaryDirectory() as tmpdir: + with patch.object(_mod, "Schematic") as mock_sch_cls, \ + patch("shutil.copy"), \ + patch("os.path.exists", return_value=True), \ + patch("builtins.open", _OPEN_MOCK): + mock_sch_cls.return_value = MagicMock() + + SchematicManager.create_schematic("myschematic", path=tmpdir) + + used_path = mock_sch_cls.call_args[0][0] + assert used_path.startswith(tmpdir), ( + f"Expected path inside {tmpdir!r}, got {used_path!r}" + ) + assert used_path.endswith("myschematic.kicad_sch") + + +def test_create_schematic_without_path_uses_relative(): + """ + When no path is given, behaviour is unchanged — file goes to cwd-relative name. + """ + with patch.object(_mod, "Schematic") as mock_sch_cls, \ + patch("shutil.copy"), \ + patch("os.path.exists", return_value=True), \ + patch("builtins.open", _OPEN_MOCK): + mock_sch_cls.return_value = MagicMock() + + SchematicManager.create_schematic("myschematic") + + used_path = mock_sch_cls.call_args[0][0] + assert used_path == "myschematic.kicad_sch" + + +def test_create_schematic_accepts_full_sch_filename(): + """ + If name already ends with .kicad_sch, it should not double the suffix. + """ + with tempfile.TemporaryDirectory() as tmpdir: + with patch.object(_mod, "Schematic") as mock_sch_cls, \ + patch("shutil.copy"), \ + patch("os.path.exists", return_value=True), \ + patch("builtins.open", _OPEN_MOCK): + mock_sch_cls.return_value = MagicMock() + + SchematicManager.create_schematic("myschematic.kicad_sch", path=tmpdir) + + used_path = mock_sch_cls.call_args[0][0] + assert used_path.endswith("myschematic.kicad_sch") + assert "myschematic.kicad_sch.kicad_sch" not in used_path From 6e80ccd0131ef47ebf6439b0b28043c966d1c9b6 Mon Sep 17 00:00:00 2001 From: Michael Parment Date: Thu, 9 Apr 2026 09:23:02 +0200 Subject: [PATCH 2/5] fix: rotate_schematic_component mirror parameter no longer crashes Previously passing mirror='x' or mirror='y' to rotate_schematic_component always raised: 'NoneType' object has no attribute 'value' Root cause: kicad-skip has no API for setting (mirror x/y) on a placed symbol instance. The handler tried to use a non-existent kicad-skip attribute, returning None, then calling .value on it. Fix: add _apply_mirror_to_symbol_sexp() which directly patches the (mirror x/y) S-expression token in the .kicad_sch file. The rotate handler now applies rotation via kicad-skip (which works fine) and delegates mirror to this helper. The helper: - Inserts (mirror x/y) immediately after the (at x y rot) token - Removes any existing mirror token before inserting the new one (prevents duplicate tokens when toggling axis) - Returns False gracefully when the reference is not found Tests: 6 unit tests in tests/test_rotate_schematic_mirror.py covering: add x, add y, remove, replace, unknown ref, no-mirror smoke test. Co-Authored-By: Claude Sonnet 4.6 --- python/kicad_interface.py | 112 +++++++++++++++-- tests/test_rotate_schematic_mirror.py | 171 ++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 11 deletions(-) create mode 100644 tests/test_rotate_schematic_mirror.py 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 From dd19828cbab432980fa0767d2196220bfb30c23e Mon Sep 17 00:00:00 2001 From: Michael Parment Date: Thu, 9 Apr 2026 09:25:17 +0200 Subject: [PATCH 3/5] Revert "fix: rotate_schematic_component mirror parameter no longer crashes" This reverts commit 6e80ccd0131ef47ebf6439b0b28043c966d1c9b6. --- python/kicad_interface.py | 112 ++--------------- tests/test_rotate_schematic_mirror.py | 171 -------------------------- 2 files changed, 11 insertions(+), 272 deletions(-) delete mode 100644 tests/test_rotate_schematic_mirror.py diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 7462008..3c853fc 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -2123,87 +2123,6 @@ 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") @@ -2211,7 +2130,7 @@ class KiCADInterface: schematic_path = params.get("schematicPath") reference = params.get("reference") angle = params.get("angle", 0) - mirror = params.get("mirror") # "x", "y", or None + mirror = params.get("mirror") if not schematic_path or not reference: return { @@ -2233,26 +2152,17 @@ 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) - - # 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": True, "reference": reference, "angle": angle} 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 deleted file mode 100644 index 78a8034..0000000 --- a/tests/test_rotate_schematic_mirror.py +++ /dev/null @@ -1,171 +0,0 @@ -""" -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 From 75a0b764d53d5f19117705383127e84698c29c0d Mon Sep 17 00:00:00 2001 From: Tom <145493375+Kletternaut@users.noreply.github.com> Date: Sat, 18 Apr 2026 14:31:09 +0200 Subject: [PATCH 4/5] Update python/kicad_interface.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- python/kicad_interface.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 3c853fc..6b2e21c 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -617,7 +617,9 @@ class KiCADInterface: sch_path = path if path and path != "." else None schematic = SchematicManager.create_schematic(project_name, path=sch_path, metadata=metadata) - file_path = f"{path}/{project_name}.kicad_sch" + base_name = project_name if project_name.endswith(".kicad_sch") else f"{project_name}.kicad_sch" + normalized_path = path or "." + file_path = os.path.join(normalized_path, base_name) success = SchematicManager.save_schematic(schematic, file_path) return {"success": success, "file_path": file_path} From 56e6aead88e878bcf4db0e0be7080a8e11c82ec3 Mon Sep 17 00:00:00 2001 From: Tom <145493375+Kletternaut@users.noreply.github.com> Date: Sat, 18 Apr 2026 14:31:22 +0200 Subject: [PATCH 5/5] Update python/commands/schematic.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- python/commands/schematic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/commands/schematic.py b/python/commands/schematic.py index c236dc2..a435c66 100644 --- a/python/commands/schematic.py +++ b/python/commands/schematic.py @@ -13,7 +13,7 @@ class SchematicManager: """Core schematic operations using kicad-skip""" @staticmethod - def create_schematic(name: str, path: Optional[str] = None, metadata: Optional[Any] = None) -> Any: + def create_schematic(name: str, metadata: Optional[Any] = None, *, path: Optional[str] = None) -> Any: """Create a new empty schematic from template""" try: # Determine template path (use template_with_symbols for component cloning support)