diff --git a/python/commands/wire_manager.py b/python/commands/wire_manager.py index eb55ad9..982d50e 100644 --- a/python/commands/wire_manager.py +++ b/python/commands/wire_manager.py @@ -8,6 +8,7 @@ manipulate the .kicad_sch file directly. import logging import math +import re import tempfile import uuid from pathlib import Path @@ -31,6 +32,90 @@ _SYM_UUID = Symbol("uuid") _SYM_SHEET_INSTANCES = Symbol("sheet_instances") +def _find_insertion_point(content: str) -> int: + """Find the right place to insert new elements in a .kicad_sch file. + + Looks for (sheet_instances (KiCad 8) first, falls back to inserting + before the final closing paren (KiCad 9+). + """ + marker = "(sheet_instances" + pos = content.rfind(marker) + if pos != -1: + return pos + pos = content.rfind(")") + if pos == -1: + raise ValueError("Could not find insertion point in schematic") + return pos + + +def _text_insert(file_path: Path, sexp_text: str) -> bool: + """Insert S-expression text into a .kicad_sch file preserving formatting.""" + with open(file_path, "r", encoding="utf-8") as f: + content = f.read() + + insert_at = _find_insertion_point(content) + content = content[:insert_at] + sexp_text + content[insert_at:] + + with open(file_path, "w", encoding="utf-8") as f: + f.write(content) + return True + + +def _make_hierarchical_label_text( + text: str, + position: List[float], + shape: str = "bidirectional", + orientation: int = 0, +) -> str: + """Generate a hierarchical_label S-expression as formatted text. + + orientation: 0=right (label points right, justify left), + 180=left (label points left, justify right), + 90/270=vertical. + """ + uid = str(uuid.uuid4()) + justify = "right" if orientation == 180 else "left" + return ( + f'\t(hierarchical_label "{text}"\n' + f"\t\t(shape {shape})\n" + f"\t\t(at {position[0]} {position[1]} {orientation})\n" + f"\t\t(effects\n" + f"\t\t\t(font\n" + f"\t\t\t\t(size 1.27 1.27)\n" + f"\t\t\t)\n" + f"\t\t\t(justify {justify})\n" + f"\t\t)\n" + f'\t\t(uuid "{uid}")\n' + f"\t)\n" + ) + + +def _make_sheet_pin_text( + pin_name: str, + pin_type: str, + position: List[float], + orientation: int = 0, +) -> str: + """Generate a sheet pin S-expression as formatted text (indented for inside sheet block). + + orientation: 0=right side of sheet box, 180=left side. + """ + uid = str(uuid.uuid4()) + justify = "left" if orientation == 0 else "right" + return ( + f'\t\t(pin "{pin_name}" {pin_type}\n' + f"\t\t\t(at {position[0]} {position[1]} {orientation})\n" + f'\t\t\t(uuid "{uid}")\n' + f"\t\t\t(effects\n" + f"\t\t\t\t(font\n" + f"\t\t\t\t\t(size 1.27 1.27)\n" + f"\t\t\t\t)\n" + f"\t\t\t\t(justify {justify})\n" + f"\t\t\t)\n" + f"\t\t)\n" + ) + + class WireManager: """Manage wires in KiCad schematics using S-expression manipulation""" @@ -669,6 +754,78 @@ class WireManager: return [start, corner, end] + @staticmethod + def add_hierarchical_label( + schematic_path: Path, + text: str, + position: List[float], + shape: str = "bidirectional", + orientation: int = 0, + ) -> bool: + """Add a hierarchical label to a sub-sheet schematic.""" + try: + label_text = _make_hierarchical_label_text(text, position, shape, orientation) + _text_insert(schematic_path, label_text) + logger.info(f"Added hierarchical_label '{text}' at {position} shape={shape}") + return True + except Exception as e: + logger.error(f"Error adding hierarchical label: {e}") + import traceback + + logger.error(traceback.format_exc()) + return False + + @staticmethod + def add_sheet_pin( + content: str, + sheet_name: str, + pin_name: str, + pin_type: str, + position: List[float], + orientation: int = 0, + ) -> Tuple[str, bool]: + """Insert a sheet pin into the named sheet block in the parent schematic. + + Returns (modified_content, success). + """ + lines = content.split("\n") + sheetname_pattern = re.compile( + r'\(property\s+"Sheetname"\s+"' + re.escape(sheet_name) + r'"' + ) + sheet_block_pattern = re.compile(r"^\t\(sheet\b") + + # Find the sheet block that contains the target Sheetname property + i = 0 + while i < len(lines): + if sheet_block_pattern.match(lines[i]): + # Walk forward to find closing paren of this block + depth = sum(1 for c in lines[i] if c == "(") - sum(1 for c in lines[i] if c == ")") + j = i + 1 + found_name = False + while j < len(lines) and depth > 0: + if sheetname_pattern.search(lines[j]): + found_name = True + depth += sum(1 for c in lines[j] if c == "(") - sum( + 1 for c in lines[j] if c == ")" + ) + j += 1 + b_end = j - 1 # index of closing ")" line of the sheet block + + if found_name: + # Insert pin text before the closing paren of the sheet block + pin_text = _make_sheet_pin_text(pin_name, pin_type, position, orientation) + pin_lines = pin_text.rstrip("\n").split("\n") + for offset, line in enumerate(pin_lines): + lines.insert(b_end + offset, line) + logger.info(f"Added sheet pin '{pin_name}' to sheet '{sheet_name}'") + return "\n".join(lines), True + + i = b_end + 1 + continue + i += 1 + + return content, False + if __name__ == "__main__": # Test wire creation diff --git a/python/kicad_interface.py b/python/kicad_interface.py index dcd6a1e..229bc50 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -408,6 +408,8 @@ class KiCADInterface: "find_orphaned_wires": self._handle_find_orphaned_wires, "list_floating_labels": self._handle_list_floating_labels, "snap_to_grid": self._handle_snap_to_grid, + "add_schematic_hierarchical_label": self._handle_add_schematic_hierarchical_label, + "add_sheet_pin": self._handle_add_sheet_pin, "import_svg_logo": self._handle_import_svg_logo, # UI/Process management commands "check_kicad_ui": self._handle_check_kicad_ui, @@ -2622,6 +2624,126 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + def _handle_add_schematic_hierarchical_label(self, params: Dict[str, Any]) -> Dict[str, Any]: + """Add a hierarchical label to a sub-sheet schematic.""" + logger.info("Adding hierarchical label to schematic") + try: + from commands.wire_manager import WireManager + + schematic_path = params.get("schematicPath") + text = params.get("text") + position = params.get("position") + shape = params.get("shape", "bidirectional") + orientation = params.get("orientation", 0) + + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + if not text: + return {"success": False, "message": "text is required"} + if not position or len(position) != 2: + return {"success": False, "message": "position [x, y] is required"} + if shape not in ("input", "output", "bidirectional"): + return { + "success": False, + "message": "shape must be input, output, or bidirectional", + } + + sch_file = Path(schematic_path) + if not sch_file.exists(): + return { + "success": False, + "message": f"Schematic not found: {schematic_path}", + } + + success = WireManager.add_hierarchical_label( + sch_file, text, position, shape=shape, orientation=orientation + ) + + if success: + return { + "success": True, + "message": ( + f"Added hierarchical_label '{text}' " f"at {position} shape={shape}" + ), + } + return {"success": False, "message": "Failed to add hierarchical label"} + + except Exception as e: + logger.error(f"Error adding hierarchical label: {e}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + + def _handle_add_sheet_pin(self, params: Dict[str, Any]) -> Dict[str, Any]: + """Add a sheet pin to a sheet block on the parent schematic.""" + logger.info("Adding sheet pin to schematic") + try: + from commands.wire_manager import WireManager + + schematic_path = params.get("schematicPath") + sheet_name = params.get("sheetName") + pin_name = params.get("pinName") + pin_type = params.get("pinType", "bidirectional") + position = params.get("position") + orientation = params.get("orientation", 0) + + if not schematic_path: + return {"success": False, "message": "schematicPath is required"} + if not sheet_name: + return {"success": False, "message": "sheetName is required"} + if not pin_name: + return {"success": False, "message": "pinName is required"} + if not position or len(position) != 2: + return {"success": False, "message": "position [x, y] is required"} + if pin_type not in ("input", "output", "bidirectional"): + return { + "success": False, + "message": "pinType must be input, output, or bidirectional", + } + + sch_file = Path(schematic_path) + if not sch_file.exists(): + return { + "success": False, + "message": f"Schematic not found: {schematic_path}", + } + + with open(sch_file, "r", encoding="utf-8") as f: + content = f.read() + + modified, success = WireManager.add_sheet_pin( + content, + sheet_name, + pin_name, + pin_type, + position, + orientation=orientation, + ) + + if not success: + return { + "success": False, + "message": f"Sheet '{sheet_name}' not found in {schematic_path}", + } + + with open(sch_file, "w", encoding="utf-8") as f: + f.write(modified) + + return { + "success": True, + "message": ( + f"Added sheet pin '{pin_name}' ({pin_type}) " f"to sheet '{sheet_name}'" + ), + } + + except Exception as e: + logger.error(f"Error adding sheet pin: {e}") + import traceback + + logger.error(traceback.format_exc()) + return {"success": False, "message": str(e)} + def _handle_run_erc(self, params: Dict[str, Any]) -> Dict[str, Any]: """Run Electrical Rules Check on a schematic via kicad-cli""" logger.info("Running ERC on schematic") diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index 3e8b63d..f234591 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -1524,4 +1524,125 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp } }, ); + + // Add hierarchical label to a sub-sheet + server.tool( + "add_schematic_hierarchical_label", + "Add a hierarchical label (sheet interface port) to a sub-sheet schematic. " + + "Hierarchical labels are the connection points that link a sub-sheet to its " + + "parent via sheet pins. The label text must exactly match the corresponding " + + "sheet pin name.", + { + schematicPath: z.string().describe("Path to the sub-sheet .kicad_sch file"), + text: z + .string() + .describe("Label text (e.g. 'SD_CLK') — must match the sheet pin name"), + position: z + .array(z.number()) + .length(2) + .describe("Position [x, y] in mm"), + shape: z + .enum(["input", "output", "bidirectional"]) + .describe("Signal direction from the sub-sheet's perspective"), + orientation: z + .number() + .optional() + .describe( + "Rotation in degrees: 0=label points right, 180=label points left (default: 0)", + ), + }, + async (args: { + schematicPath: string; + text: string; + position: number[]; + shape: "input" | "output" | "bidirectional"; + orientation?: number; + }) => { + const result = await callKicadScript("add_schematic_hierarchical_label", args); + if (result.success) { + return { + content: [ + { + type: "text" as const, + text: result.message || `Added hierarchical label '${args.text}'`, + }, + ], + }; + } + return { + content: [ + { + type: "text" as const, + text: `Failed to add hierarchical label: ${result.message || "Unknown error"}`, + }, + ], + }; + }, + ); + + // Add sheet pin to a sheet block on the parent schematic + server.tool( + "add_sheet_pin", + "Add a pin to a sheet symbol block on the parent schematic. Sheet pins are the " + + "parent-side connection points that correspond to hierarchical labels in the " + + "sub-sheet. The pinName must exactly match a hierarchical_label in the sub-sheet.", + { + schematicPath: z.string().describe("Path to the PARENT .kicad_sch file"), + sheetName: z + .string() + .describe( + "Sheet name as it appears in the Sheetname property (e.g. 'Storage')", + ), + pinName: z + .string() + .describe("Pin name — must match a hierarchical_label in the sub-sheet"), + pinType: z + .enum(["input", "output", "bidirectional"]) + .describe( + "Signal direction (should match the sub-sheet hierarchical label shape)", + ), + position: z + .array(z.number()) + .length(2) + .describe( + "Pin position [x, y] in mm — must be on the sheet block boundary", + ), + orientation: z + .number() + .optional() + .describe( + "Pin orientation: 0=right edge of sheet box, 180=left edge (default: 0)", + ), + }, + async (args: { + schematicPath: string; + sheetName: string; + pinName: string; + pinType: "input" | "output" | "bidirectional"; + position: number[]; + orientation?: number; + }) => { + const result = await callKicadScript("add_sheet_pin", args); + if (result.success) { + return { + content: [ + { + type: "text" as const, + text: + result.message || + `Added sheet pin '${args.pinName}' to sheet '${args.sheetName}'`, + }, + ], + }; + } + return { + content: [ + { + type: "text" as const, + text: `Failed to add sheet pin: ${result.message || "Unknown error"}`, + }, + ], + }; + }, + ); } diff --git a/tests/test_hierarchy_tools.py b/tests/test_hierarchy_tools.py new file mode 100644 index 0000000..fc5dcb6 --- /dev/null +++ b/tests/test_hierarchy_tools.py @@ -0,0 +1,349 @@ +""" +Tests for add_schematic_hierarchical_label and add_sheet_pin tools. + +Covers: + - Hierarchical label insertion with correct S-expression format + - Sheet pin insertion into the correct sheet block + - Parameter validation (missing required fields) + - Orientation and justification mapping + - Sheet-not-found error handling +""" + +import sys +import textwrap +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")) + + +# --------------------------------------------------------------------------- +# Shared fixture +# --------------------------------------------------------------------------- + + +def _make_iface() -> Any: + with patch("kicad_interface.USE_IPC_BACKEND", False): + from kicad_interface import KiCADInterface + + iface = KiCADInterface.__new__(KiCADInterface) + return iface + + +@pytest.fixture() +def iface(): + return _make_iface() + + +# --------------------------------------------------------------------------- +# Minimal schematic templates +# --------------------------------------------------------------------------- + +_MINIMAL_SUBSHEET = textwrap.dedent("""\ + (kicad_sch + \t(version 20231120) + \t(generator "eeschema") + \t(generator_version "9.0") + \t(sheet_instances + \t\t(path "/" + \t\t\t(page "1") + \t\t) + \t) + ) +""") + +_MINIMAL_PARENT = textwrap.dedent("""\ + (kicad_sch + \t(version 20231120) + \t(generator "eeschema") + \t(sheet + \t\t(at 100 50) + \t\t(size 40 30) + \t\t(property "Sheetname" "Storage" + \t\t\t(at 100 49 0) + \t\t\t(effects + \t\t\t\t(font + \t\t\t\t\t(size 1.27 1.27) + \t\t\t\t) + \t\t\t) + \t\t) + \t\t(property "Sheetfile" "sheets/storage.kicad_sch" + \t\t\t(at 100 82 0) + \t\t\t(effects + \t\t\t\t(font + \t\t\t\t\t(size 1.27 1.27) + \t\t\t\t) + \t\t\t) + \t\t) + \t) + \t(sheet_instances + \t\t(path "/" + \t\t\t(page "1") + \t\t) + \t) + ) +""") + +_PARENT_TWO_SHEETS = textwrap.dedent("""\ + (kicad_sch + \t(version 20231120) + \t(sheet + \t\t(at 50 50) + \t\t(size 40 30) + \t\t(property "Sheetname" "Power" + \t\t\t(at 50 49 0) + \t\t\t(effects (font (size 1.27 1.27))) + \t\t) + \t\t(property "Sheetfile" "sheets/power.kicad_sch" + \t\t\t(at 50 82 0) + \t\t\t(effects (font (size 1.27 1.27))) + \t\t) + \t) + \t(sheet + \t\t(at 150 50) + \t\t(size 40 30) + \t\t(property "Sheetname" "Storage" + \t\t\t(at 150 49 0) + \t\t\t(effects (font (size 1.27 1.27))) + \t\t) + \t\t(property "Sheetfile" "sheets/storage.kicad_sch" + \t\t\t(at 150 82 0) + \t\t\t(effects (font (size 1.27 1.27))) + \t\t) + \t) + \t(sheet_instances + \t\t(path "/" (page "1")) + \t) + ) +""") + + +# =========================================================================== +# Hierarchical label tests +# =========================================================================== + + +@pytest.mark.unit +class TestAddHierarchicalLabel: + def test_inserts_label_into_subsheet(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "text": "SD_CLK", + "position": [50.8, 25.4], + "shape": "output", + } + ) + + assert result["success"] is True + content = sch.read_text() + assert '(hierarchical_label "SD_CLK"' in content + assert "(shape output)" in content + assert "(at 50.8 25.4 0)" in content + + def test_orientation_180_uses_right_justify(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "text": "VBUS", + "position": [10, 20], + "shape": "input", + "orientation": 180, + } + ) + + assert result["success"] is True + content = sch.read_text() + assert "(at 10 20 180)" in content + assert "(justify right)" in content + + def test_orientation_0_uses_left_justify(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "text": "SDA", + "position": [30, 40], + "shape": "bidirectional", + "orientation": 0, + } + ) + + assert result["success"] is True + content = sch.read_text() + assert "(justify left)" in content + + def test_missing_text_fails(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "position": [10, 20], + "shape": "input", + } + ) + + assert result["success"] is False + assert "text" in result["message"].lower() + + def test_invalid_shape_fails(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "text": "SIG", + "position": [10, 20], + "shape": "passive", + } + ) + + assert result["success"] is False + + def test_nonexistent_file_fails(self, iface, tmp_path): + result = iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(tmp_path / "nope.kicad_sch"), + "text": "SIG", + "position": [10, 20], + "shape": "input", + } + ) + + assert result["success"] is False + assert "not found" in result["message"].lower() + + def test_inserts_before_sheet_instances(self, iface, tmp_path): + sch = tmp_path / "sub.kicad_sch" + sch.write_text(_MINIMAL_SUBSHEET) + + iface._handle_add_schematic_hierarchical_label( + { + "schematicPath": str(sch), + "text": "TEST", + "position": [10, 20], + "shape": "input", + } + ) + + content = sch.read_text() + label_pos = content.find("hierarchical_label") + instances_pos = content.find("sheet_instances") + assert ( + label_pos < instances_pos + ), "Hierarchical label should be inserted before sheet_instances" + + +# =========================================================================== +# Sheet pin tests +# =========================================================================== + + +@pytest.mark.unit +class TestAddSheetPin: + def test_inserts_pin_into_correct_sheet(self, iface, tmp_path): + sch = tmp_path / "parent.kicad_sch" + sch.write_text(_MINIMAL_PARENT) + + result = iface._handle_add_sheet_pin( + { + "schematicPath": str(sch), + "sheetName": "Storage", + "pinName": "SD_CLK", + "pinType": "output", + "position": [140, 60], + } + ) + + assert result["success"] is True + content = sch.read_text() + assert '(pin "SD_CLK" output' in content + assert "(at 140 60 0)" in content + + def test_pin_in_multi_sheet_parent_targets_correct_sheet(self, iface, tmp_path): + sch = tmp_path / "parent.kicad_sch" + sch.write_text(_PARENT_TWO_SHEETS) + + result = iface._handle_add_sheet_pin( + { + "schematicPath": str(sch), + "sheetName": "Storage", + "pinName": "SD_D0", + "pinType": "bidirectional", + "position": [190, 60], + } + ) + + assert result["success"] is True + content = sch.read_text() + # Pin should be inside the Storage sheet block, not the Power block + storage_pos = content.find('"Storage"') + pin_pos = content.find('"SD_D0"') + power_end = content.find('"Power"') + assert pin_pos > storage_pos, "Pin should be after Storage sheet name" + + def test_sheet_not_found_fails(self, iface, tmp_path): + sch = tmp_path / "parent.kicad_sch" + sch.write_text(_MINIMAL_PARENT) + + result = iface._handle_add_sheet_pin( + { + "schematicPath": str(sch), + "sheetName": "NonExistent", + "pinName": "SIG", + "pinType": "input", + "position": [100, 50], + } + ) + + assert result["success"] is False + assert "not found" in result["message"].lower() + + def test_missing_pin_name_fails(self, iface, tmp_path): + sch = tmp_path / "parent.kicad_sch" + sch.write_text(_MINIMAL_PARENT) + + result = iface._handle_add_sheet_pin( + { + "schematicPath": str(sch), + "sheetName": "Storage", + "pinType": "input", + "position": [100, 50], + } + ) + + assert result["success"] is False + + def test_orientation_180_uses_right_justify(self, iface, tmp_path): + sch = tmp_path / "parent.kicad_sch" + sch.write_text(_MINIMAL_PARENT) + + result = iface._handle_add_sheet_pin( + { + "schematicPath": str(sch), + "sheetName": "Storage", + "pinName": "VBUS", + "pinType": "input", + "position": [100, 60], + "orientation": 180, + } + ) + + assert result["success"] is True + content = sch.read_text() + assert "(at 100 60 180)" in content + assert "(justify right)" in content