Merge pull request #115 from thesamprice/test/add-schematic-component-unit

test: add unit parameter tests for add_schematic_component

python/commands/dynamic_symbol_loader.py

@@ -402,15 +402,17 @@ class DynamicSymbolLoader:
         footprint: str = "",
         x: float = 0,
         y: float = 0,
+        unit: int = 1,
     ) -> bool:
         """
         Add a component instance to the schematic.
         This creates the (symbol ...) block with lib_id reference.
+        For multi-unit symbols, set unit to 1–N to place a specific unit.
         """
         full_lib_id = f"{library_name}:{symbol_name}"
         new_uuid = str(uuid.uuid4())
 
-        instance_block = f"""  (symbol (lib_id "{full_lib_id}") (at {x} {y} 0) (unit 1)
+        instance_block = f"""  (symbol (lib_id "{full_lib_id}") (at {x} {y} 0) (unit {unit})
     (in_bom yes) (on_board yes) (dnp no)
     (uuid "{new_uuid}")
     (property "Reference" "{reference}" (at {x} {y - 2.54} 0)
@@ -429,7 +431,7 @@ class DynamicSymbolLoader:
       (project "project"
         (path "/"
           (reference "{reference}")
-          (unit 1)
+          (unit {unit})
         )
       )
     )
@@ -493,6 +495,7 @@ class DynamicSymbolLoader:
         footprint: str = "",
         x: float = 0,
         y: float = 0,
+        unit: int = 1,
         project_path: Optional[Path] = None,
     ) -> bool:
         """
@@ -500,6 +503,7 @@ class DynamicSymbolLoader:
         This is the main entry point for adding components.
 
         Args:
+            unit: For multi-unit symbols, which unit to place (1=A, 2=B, …). Default 1.
             project_path: Optional project directory. When set, project-specific
                           sym-lib-table is also searched for the library file.
         """
@@ -518,6 +522,7 @@ class DynamicSymbolLoader:
             footprint=footprint,
             x=x,
             y=y,
+            unit=unit,
         )
 
 

python/kicad_interface.py

@@ -725,6 +725,7 @@ class KiCADInterface:
             footprint = component.get("footprint", "")
             x = component.get("x", 0)
             y = component.get("y", 0)
+            unit = component.get("unit", 1)
 
             # Derive project path from schematic path for project-local library resolution
             schematic_file = Path(schematic_path)
@@ -740,6 +741,7 @@ class KiCADInterface:
                 footprint=footprint,
                 x=x,
                 y=y,
+                unit=unit,
                 project_path=derived_project_path,
             )
 

src/tools/schematic.ts

@@ -49,6 +49,12 @@ export function registerSchematicTools(server: McpServer, callKicadScript: Funct
         })
         .optional()
         .describe("Position on schematic"),
+      unit: z
+        .number()
+        .int()
+        .min(1)
+        .optional()
+        .describe("Unit number for multi-unit symbols (1=A, 2=B, 3=C, …). Defaults to 1."),
     },
     async (args: {
       schematicPath: string;
@@ -57,6 +63,7 @@ export function registerSchematicTools(server: McpServer, callKicadScript: Funct
       value?: string;
       footprint?: string;
       position?: { x: number; y: number };
+      unit?: number;
     }) => {
       // Transform to what Python backend expects
       const [library, symbolName] = args.symbol.includes(":")
@@ -74,6 +81,7 @@ export function registerSchematicTools(server: McpServer, callKicadScript: Funct
           // Python expects flat x, y not nested position
           x: args.position?.x ?? 0,
           y: args.position?.y ?? 0,
+          unit: args.unit ?? 1,
         },
       };
 

tests/test_add_schematic_component.py

@@ -0,0 +1,191 @@
+"""
+Tests for add_schematic_component handler, focusing on the unit parameter
+for multi-unit symbols (e.g. quad optocouplers, dual op-amps).
+"""
+
+import re
+import shutil
+import sys
+import tempfile
+from pathlib import Path
+from typing import Any
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).parent.parent / "python"))
+
+TEMPLATES_DIR = Path(__file__).parent.parent / "python" / "templates"
+EMPTY_SCH = TEMPLATES_DIR / "empty.kicad_sch"
+
+
+def _write_temp_sch(content: str) -> Path:
+    tmp = tempfile.NamedTemporaryFile(
+        suffix=".kicad_sch", delete=False, mode="w", encoding="utf-8"
+    )
+    tmp.write(content)
+    tmp.close()
+    return Path(tmp.name)
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _unit_values_in_file(path: Path) -> list[int]:
+    """Return all (unit N) values written for symbol instances in the schematic."""
+    content = path.read_text()
+    # Match top-level symbol instances: (symbol (lib_id ...) (at ...) (unit N) ...)
+    return [int(n) for n in re.findall(r"\(symbol \(lib_id [^)]+\) \(at [^)]+\) \(unit (\d+)\)", content)]
+
+
+# ---------------------------------------------------------------------------
+# Unit tests – create_component_instance
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.unit
+class TestCreateComponentInstanceUnit:
+    """Tests for DynamicSymbolLoader.create_component_instance unit parameter."""
+
+    def setup_method(self) -> None:
+        from commands.dynamic_symbol_loader import DynamicSymbolLoader
+
+        self.DynamicSymbolLoader = DynamicSymbolLoader
+
+    def _loader(self) -> Any:
+        return self.DynamicSymbolLoader()
+
+    def test_default_unit_is_1(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="R1", value="10k", x=10, y=10
+        )
+        units = _unit_values_in_file(sch)
+        assert 1 in units
+
+    def test_explicit_unit_1(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="R1", value="10k", x=10, y=10, unit=1
+        )
+        units = _unit_values_in_file(sch)
+        assert units.count(1) >= 1
+
+    def test_unit_2_written_correctly(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="U1", value="TLP291-4", x=10, y=10, unit=2
+        )
+        units = _unit_values_in_file(sch)
+        assert 2 in units
+
+    def test_unit_4_written_correctly(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="U1", value="TLP291-4", x=10, y=10, unit=4
+        )
+        units = _unit_values_in_file(sch)
+        assert 4 in units
+
+    def test_instances_block_uses_same_unit(self, tmp_path: Any) -> None:
+        """The (instances ...) path block must also record the correct unit number."""
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="U1", value="val", x=5, y=5, unit=3
+        )
+        content = sch.read_text()
+        # The (unit 3) inside the (instances ...) block
+        assert "(unit 3)" in content
+        # Count occurrences — should appear at least twice (symbol header + instances)
+        assert content.count("(unit 3)") >= 2
+
+    def test_multiple_units_same_reference(self, tmp_path: Any) -> None:
+        """Placing units A and B of the same reference produces two distinct unit entries."""
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        loader = self._loader()
+        loader.create_component_instance(
+            sch, "Device", "R", reference="U10", value="TLP291-4", x=10, y=10, unit=1
+        )
+        loader.create_component_instance(
+            sch, "Device", "R", reference="U10", value="TLP291-4", x=10, y=35, unit=2
+        )
+        units = _unit_values_in_file(sch)
+        assert 1 in units
+        assert 2 in units
+
+
+# ---------------------------------------------------------------------------
+# Handler-level tests – _handle_add_schematic_component
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.unit
+class TestHandlerAddSchematicComponent:
+    """Tests for KiCADInterface._handle_add_schematic_component unit plumbing."""
+
+    def _call_handler(self, params: dict) -> dict:
+        from kicad_interface import KiCADInterface
+
+        iface = KiCADInterface()
+        return iface._handle_add_schematic_component(params)
+
+    def test_missing_schematic_path_returns_error(self) -> None:
+        result = self._call_handler({"component": {"type": "R", "library": "Device"}})
+        assert result["success"] is False
+        assert "path" in result["message"].lower() or "schematic" in result["message"].lower()
+
+    def test_missing_component_returns_error(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        result = self._call_handler({"schematicPath": str(sch)})
+        assert result["success"] is False
+
+    def test_unit_defaults_to_1_in_handler(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        result = self._call_handler({
+            "schematicPath": str(sch),
+            "component": {
+                "library": "Device",
+                "type": "R",
+                "reference": "R99",
+                "value": "1k",
+                "x": 10,
+                "y": 10,
+                # no "unit" key — should default to 1
+            },
+        })
+        assert result["success"] is True
+        units = _unit_values_in_file(sch)
+        assert 1 in units
+
+    def test_unit_2_passed_through_handler(self, tmp_path: Any) -> None:
+        sch = tmp_path / "test.kicad_sch"
+        shutil.copy(EMPTY_SCH, sch)
+        result = self._call_handler({
+            "schematicPath": str(sch),
+            "component": {
+                "library": "Device",
+                "type": "R",
+                "reference": "U10",
+                "value": "TLP291-4",
+                "x": 25,
+                "y": 35,
+                "unit": 2,
+            },
+        })
+        assert result["success"] is True
+        units = _unit_values_in_file(sch)
+        assert 2 in units