diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 46b4e5d..3e9a5d1 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -4181,6 +4181,15 @@ class KiCADInterface: # Build hierarchical pad→net map (walks all sub-sheets) pad_net_map, net_names = self._build_hierarchical_pad_net_map(schematic_path) + # Add missing footprints from the schematic to the board *before* + # we add nets and assign pads — F8 in KiCad does this implicitly + # ("Update PCB from Schematic"), but our previous implementation + # only mutated nets, leaving newly-added schematic symbols with no + # PCB footprint at all. + added_footprints, skipped_footprints = self._add_missing_footprints_from_schematic( + board, schematic_path + ) + # Add all nets to board netinfo = board.GetNetInfo() nets_by_name = netinfo.NetsByName() @@ -4195,7 +4204,7 @@ class KiCADInterface: netinfo = board.GetNetInfo() nets_by_name = netinfo.NetsByName() - # Assign nets to pads + # Assign nets to pads (now also covers any footprints we just added) assigned_pads = 0 unmatched = [] for fp in board.GetFootprints(): @@ -4219,15 +4228,21 @@ class KiCADInterface: self._update_command_handlers() logger.info( - f"sync_schematic_to_board: {len(added_nets)} nets added, {assigned_pads} pads assigned" + f"sync_schematic_to_board: {len(added_nets)} nets added, " + f"{len(added_footprints)} footprints added, {assigned_pads} pads assigned" ) return { "success": True, - "message": f"PCB nets synced from schematic: {len(added_nets)} nets added, {assigned_pads} pads assigned", + "message": ( + f"PCB updated from schematic: {len(added_footprints)} footprints added, " + f"{len(added_nets)} nets added, {assigned_pads} pads assigned" + ), "nets_added": added_nets, "nets_total": len(net_names), "pads_assigned": assigned_pads, "unmatched_pads_sample": unmatched[:10], + "footprints_added": added_footprints, + "footprints_skipped": skipped_footprints, } except Exception as e: @@ -4237,6 +4252,155 @@ class KiCADInterface: logger.error(traceback.format_exc()) return {"success": False, "message": str(e)} + def _extract_components_from_schematic(self, schematic_path: str) -> List[Dict[str, str]]: + """Run kicad-cli netlist export and return the flat list of components. + + Each entry: {"reference": str, "value": str, "footprint": str} + Empty list on any failure (kicad-cli missing, parse error, etc.) — the + caller treats that as "no missing footprints to add". + """ + import subprocess + import tempfile + import xml.etree.ElementTree as ET + + kicad_cli = self._find_kicad_cli_static() + if not kicad_cli: + logger.warning("kicad-cli not found — sync will not add new footprints") + return [] + + with tempfile.NamedTemporaryFile(suffix=".xml", delete=False) as tmp: + tmp_path = tmp.name + try: + cmd = [ + kicad_cli, + "sch", + "export", + "netlist", + "--format", + "kicadxml", + "--output", + tmp_path, + schematic_path, + ] + result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) + if result.returncode != 0: + logger.warning( + f"kicad-cli netlist export failed (exit {result.returncode}): " + f"{result.stderr.strip()}" + ) + return [] + + tree = ET.parse(tmp_path) + root = tree.getroot() + components = [] + for comp in root.findall("./components/comp"): + components.append( + { + "reference": comp.get("ref", ""), + "value": comp.findtext("value", ""), + "footprint": comp.findtext("footprint", ""), + } + ) + return components + except Exception as e: + logger.warning(f"Failed to extract components from schematic: {e}") + return [] + finally: + try: + os.unlink(tmp_path) + except OSError: + pass + + def _add_missing_footprints_from_schematic( + self, board: Any, schematic_path: str + ) -> Tuple[List[Dict[str, str]], List[Dict[str, str]]]: + """Add footprints to ``board`` for any schematic component not yet present. + + New footprints are placed at the board origin so the user can move them + into position. Power/flag references (``#PWR``, ``#FLG``) are skipped — + they have no PCB representation. + + Returns ``(added, skipped)``: each entry is + ``{"reference": str, "footprint": str, "reason": str?}``. + """ + from pathlib import Path + + from commands.library import LibraryManager + + added: List[Dict[str, str]] = [] + skipped: List[Dict[str, str]] = [] + + components = self._extract_components_from_schematic(schematic_path) + if not components: + return added, skipped + + existing_refs = {fp.GetReference() for fp in board.GetFootprints()} + project_dir = Path(schematic_path).parent + library_manager = LibraryManager(project_path=project_dir) + + for comp in components: + ref = comp["reference"] + fp_str = comp["footprint"] + if not ref or ref.startswith("#"): + # Power flags / global indicators — no PCB footprint expected. + continue + if ref in existing_refs: + continue + if not fp_str or ":" not in fp_str: + skipped.append( + { + "reference": ref, + "footprint": fp_str, + "reason": "no Library:Name footprint set on schematic symbol", + } + ) + continue + + lib_name, fp_name = fp_str.split(":", 1) + library_path = library_manager.libraries.get(lib_name) + if not library_path: + skipped.append( + { + "reference": ref, + "footprint": fp_str, + "reason": f"library '{lib_name}' not in fp-lib-table", + } + ) + continue + + try: + module = pcbnew.FootprintLoad(library_path, fp_name) + except Exception as e: + skipped.append( + {"reference": ref, "footprint": fp_str, "reason": f"FootprintLoad failed: {e}"} + ) + continue + + if not module: + skipped.append( + { + "reference": ref, + "footprint": fp_str, + "reason": f"footprint '{fp_name}' not in '{lib_name}'", + } + ) + continue + + module.SetReference(ref) + if comp["value"]: + module.SetValue(comp["value"]) + module.SetFPID(pcbnew.LIB_ID(lib_name, fp_name)) + # Place at board origin; user / autoplacer can position from there. + module.SetPosition(pcbnew.VECTOR2I(0, 0)) + + board.Add(module) + existing_refs.add(ref) + added.append({"reference": ref, "footprint": fp_str}) + + if added: + logger.info(f"_add_missing_footprints_from_schematic: added {len(added)} footprints") + return added, skipped + # =================================================================== # Schematic analysis tools (read-only) # =================================================================== diff --git a/tests/test_sync_schematic_to_board_footprints.py b/tests/test_sync_schematic_to_board_footprints.py new file mode 100644 index 0000000..1dc1c04 --- /dev/null +++ b/tests/test_sync_schematic_to_board_footprints.py @@ -0,0 +1,322 @@ +""" +Regression tests for sync_schematic_to_board's footprint-add path. + +Before the fix, _handle_sync_schematic_to_board only mutated nets and pad +assignments — it iterated board.GetFootprints() and never added new ones. +A schematic symbol whose Reference was not yet on the PCB was therefore +silently dropped on the floor: no footprint added, no rats nest reaching +the missing component. + +These tests cover _add_missing_footprints_from_schematic and its kicad-cli +helper _extract_components_from_schematic. +""" + +import sys +from pathlib import Path +from typing import Any, List +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_existing_fp(reference: str) -> MagicMock: + fp = MagicMock(name=f"existing_fp_{reference}") + fp.GetReference.return_value = reference + return fp + + +def _interface() -> Any: + from kicad_interface import KiCADInterface + + return KiCADInterface() + + +# --------------------------------------------------------------------------- +# _add_missing_footprints_from_schematic +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestAddMissingFootprintsFromSchematic: + """The fix path: walk netlist, add footprints for refs not yet on the board.""" + + def _patch_extract(self, components: List[dict]) -> Any: + return patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=components, + ) + + def test_adds_footprint_for_missing_reference(self, tmp_path: Any) -> None: + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [] # nothing on the board yet + + loaded_module = MagicMock(name="loaded_R0603") + with ( + patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[ + { + "reference": "R99", + "value": "10k", + "footprint": "Resistor_SMD:R_0603_1608Metric", + } + ], + ), + patch("kicad_interface.pcbnew") as mock_pcbnew, + patch("commands.library.LibraryManager") as mock_lm_cls, + ): + mock_pcbnew.FootprintLoad.return_value = loaded_module + lm = MagicMock() + lm.libraries = {"Resistor_SMD": "/fake/Resistor_SMD.pretty"} + mock_lm_cls.return_value = lm + + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert len(added) == 1 + assert added[0]["reference"] == "R99" + assert added[0]["footprint"] == "Resistor_SMD:R_0603_1608Metric" + assert skipped == [] + # Footprint was added to the board. + board.Add.assert_called_once_with(loaded_module) + loaded_module.SetReference.assert_called_with("R99") + loaded_module.SetValue.assert_called_with("10k") + + def test_skips_reference_already_on_board(self, tmp_path: Any) -> None: + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [_make_existing_fp("R1")] + + with ( + patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[ + { + "reference": "R1", + "value": "10k", + "footprint": "Resistor_SMD:R_0603_1608Metric", + } + ], + ), + patch("kicad_interface.pcbnew"), + patch("commands.library.LibraryManager") as mock_lm_cls, + ): + lm = MagicMock() + lm.libraries = {"Resistor_SMD": "/fake/Resistor_SMD.pretty"} + mock_lm_cls.return_value = lm + + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert added == [] + assert skipped == [] + board.Add.assert_not_called() + + def test_skips_power_symbols(self, tmp_path: Any) -> None: + """References starting with # (e.g. #PWR, #FLG) have no PCB footprint.""" + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [] + + with ( + patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[ + {"reference": "#PWR0001", "value": "GND", "footprint": ""}, + {"reference": "#FLG0001", "value": "PWR_FLAG", "footprint": ""}, + ], + ), + patch("kicad_interface.pcbnew"), + patch("commands.library.LibraryManager") as mock_lm_cls, + ): + mock_lm_cls.return_value = MagicMock(libraries={}) + + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert added == [] + # Power refs are excluded entirely — they don't show up in the skipped + # diagnostic list either, since "no PCB footprint" is the right answer. + assert skipped == [] + board.Add.assert_not_called() + + def test_records_skip_reason_for_missing_footprint_property(self, tmp_path: Any) -> None: + """A schematic symbol with no Footprint property is reported as skipped.""" + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [] + + with ( + patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[{"reference": "R1", "value": "10k", "footprint": ""}], + ), + patch("kicad_interface.pcbnew"), + patch("commands.library.LibraryManager") as mock_lm_cls, + ): + mock_lm_cls.return_value = MagicMock(libraries={}) + + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert added == [] + assert len(skipped) == 1 + assert skipped[0]["reference"] == "R1" + assert "no Library:Name" in skipped[0]["reason"] + + def test_records_skip_reason_for_unknown_library(self, tmp_path: Any) -> None: + """If the footprint's library nickname isn't in fp-lib-table, skip with reason.""" + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [] + + with ( + patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[ + { + "reference": "U1", + "value": "MyChip", + "footprint": "MyVendor:MyChip_QFN24", + } + ], + ), + patch("kicad_interface.pcbnew"), + patch("commands.library.LibraryManager") as mock_lm_cls, + ): + mock_lm_cls.return_value = MagicMock(libraries={}) # MyVendor not present + + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert added == [] + assert len(skipped) == 1 + assert skipped[0]["reference"] == "U1" + assert "MyVendor" in skipped[0]["reason"] + + def test_no_op_when_kicad_cli_returns_empty(self, tmp_path: Any) -> None: + """If the netlist extractor returns nothing, the helper is a no-op.""" + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + board = MagicMock(name="board") + board.GetFootprints.return_value = [] + + with patch.object( + _interface().__class__, + "_extract_components_from_schematic", + return_value=[], + ): + iface = _interface() + added, skipped = iface._add_missing_footprints_from_schematic(board, str(sch)) + + assert added == [] + assert skipped == [] + board.Add.assert_not_called() + + +# --------------------------------------------------------------------------- +# _extract_components_from_schematic +# --------------------------------------------------------------------------- + + +@pytest.mark.unit +class TestExtractComponentsFromSchematic: + """The kicad-cli helper that produces (reference, value, footprint) records.""" + + def test_parses_kicad_xml_netlist(self, tmp_path: Any) -> None: + netlist_xml = """ + + + + + 10k + Resistor_SMD:R_0603_1608Metric + + + 0.1uF + Capacitor_SMD:C_0603_1608Metric + + + MyChip + + + + + +""" + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + def fake_run(cmd: Any, **kwargs: Any) -> Any: + output_idx = cmd.index("--output") + 1 + Path(cmd[output_idx]).write_text(netlist_xml) + return MagicMock(returncode=0, stderr="", stdout="") + + with ( + patch.object( + _interface().__class__, "_find_kicad_cli_static", return_value="/fake/kicad-cli" + ), + patch("subprocess.run", side_effect=fake_run), + ): + iface = _interface() + comps = iface._extract_components_from_schematic(str(sch)) + + assert len(comps) == 3 + refs = [c["reference"] for c in comps] + assert refs == ["R1", "C1", "U1"] + # Empty resolves to "" + u1 = next(c for c in comps if c["reference"] == "U1") + assert u1["footprint"] == "" + + def test_returns_empty_when_kicad_cli_missing(self, tmp_path: Any) -> None: + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + with patch.object(_interface().__class__, "_find_kicad_cli_static", return_value=None): + iface = _interface() + comps = iface._extract_components_from_schematic(str(sch)) + + assert comps == [] + + def test_returns_empty_when_kicad_cli_fails(self, tmp_path: Any) -> None: + sch = tmp_path / "test.kicad_sch" + sch.write_text("(kicad_sch)\n") + + with ( + patch.object( + _interface().__class__, "_find_kicad_cli_static", return_value="/fake/kicad-cli" + ), + patch( + "subprocess.run", + return_value=MagicMock(returncode=1, stderr="boom", stdout=""), + ), + ): + iface = _interface() + comps = iface._extract_components_from_schematic(str(sch)) + + assert comps == []