diff --git a/python/commands/routing.py b/python/commands/routing.py index 23defb9..2529dc3 100644 --- a/python/commands/routing.py +++ b/python/commands/routing.py @@ -2,9 +2,12 @@ Routing-related command implementations for KiCAD interface """ +import json import logging import math import os +import tempfile +from pathlib import Path from typing import Any, Dict, List, Optional, Tuple import pcbnew @@ -12,6 +15,128 @@ import pcbnew logger = logging.getLogger("kicad_interface") +# --- Net class project-file persistence (KiCad 7+) ------------------------ +# +# Net class *definitions* live in the project file (``.kicad_pro`` -> +# ``net_settings``), not in the ``.kicad_pcb`` board file, since KiCad 7. The +# SWIG board save only writes ``.kicad_pcb``, so a NETCLASS created on the +# in-memory board never survives a reload on its own (issue #185). These +# helpers write the class definition straight into the project JSON, which is +# what KiCad reads on open. Values are millimetres (the +# ``.kicad_pro`` unit). They are pure module functions so they can be unit +# tested without a live KiCad / SWIG round-trip. + +# net_settings.classes numeric fields this tool can set (millimetres). The +# caller passes values already keyed by these names, so the persisted class +# cannot drift from differing request-key spellings (e.g. traceWidth). +_NETCLASS_NUMERIC_FIELDS = ( + "clearance", + "track_width", + "via_diameter", + "via_drill", + "microvia_diameter", + "microvia_drill", + "diff_pair_width", + "diff_pair_gap", +) + +# Fallback field set (KiCad 10 defaults) used only when the project has no +# "Default" class to clone the shape from. +_DEFAULT_NETCLASS_TEMPLATE = { + "bus_width": 12, + "clearance": 0.2, + "diff_pair_gap": 0.25, + "diff_pair_via_gap": 0.25, + "diff_pair_width": 0.2, + "line_style": 0, + "microvia_diameter": 0.3, + "microvia_drill": 0.1, + "pcb_color": "rgba(0, 0, 0, 0.000)", + "priority": 0, + "schematic_color": "rgba(0, 0, 0, 0.000)", + "track_width": 0.2, + "tuning_profile": "", + "via_diameter": 0.6, + "via_drill": 0.3, + "wire_width": 6, +} + + +def apply_netclass_to_project_settings( + data: Dict[str, Any], name: str, props: Dict[str, Any] +) -> Dict[str, Any]: + """Insert or update a net class *definition* in a parsed ``.kicad_pro`` dict. + Pure: mutates and returns ``data``; performs no I/O. + + Net class definitions live in the project file's ``net_settings.classes`` on + KiCad 7+. ``props`` is keyed by ``net_settings.classes`` field names + (``clearance``/``track_width``/...) in millimetres; the caller normalizes the + request keys so the persisted class never diverges from the live board. A + new class is cloned from the project's ``Default`` class (falling back to a + built-in template) so it carries KiCad's full field set. + """ + net_settings = data.setdefault("net_settings", {}) + classes = net_settings.setdefault("classes", []) + + cls = next((c for c in classes if c.get("name") == name), None) + if cls is None: + template = next((c for c in classes if c.get("name") == "Default"), None) + cls = dict(template) if template else dict(_DEFAULT_NETCLASS_TEMPLATE) + cls["name"] = name + cls["priority"] = 0 # custom classes; the Default class keeps its own priority + classes.append(cls) + + for key in _NETCLASS_NUMERIC_FIELDS: + value = props.get(key) + if value is not None: + cls[key] = float(value) + + return data + + +def persist_netclass_to_project( + pro_path: Optional[str], name: str, props: Dict[str, Any] +) -> Dict[str, Any]: + """Read/modify/write ``pro_path`` so the net class definition survives a + reload (KiCad 7+ keeps it in the project file, not the board). + + Returns ``{"persisted": bool, "projectFile"?: str, "warning"?: str}``. Never + raises: a persistence failure is reported, not fatal, so the in-memory net + class still stands. The write is atomic (temp file + ``os.replace``) so a + crash mid-write cannot corrupt the project file. + """ + if not pro_path or not os.path.exists(pro_path): + return { + "persisted": False, + "warning": "no .kicad_pro project file found; net class set in memory " + "only and will not persist across a reload", + } + try: + with open(pro_path, "r", encoding="utf-8") as handle: + data = json.load(handle) + apply_netclass_to_project_settings(data, name, props) + + directory = os.path.dirname(pro_path) or "." + fd, tmp_path = tempfile.mkstemp(dir=directory, prefix=".netclass-", suffix=".kicad_pro") + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + json.dump(data, handle, indent=2, sort_keys=True) + handle.write("\n") + os.replace(tmp_path, pro_path) + except Exception: + try: + os.remove(tmp_path) + except OSError: + pass + raise + return {"persisted": True, "projectFile": pro_path} + except Exception as exc: # report, never fail the command on a persistence error + return { + "persisted": False, + "warning": f"could not persist net class to {pro_path}: {exc}", + } + + class RoutingCommands: """Handles routing-related KiCAD operations""" @@ -1262,76 +1387,88 @@ class RoutingCommands: "errorDetails": "name parameter is required", } - # Get net classes — KiCad 6/7 returns NETCLASSES with .Find/.Add; - # KiCad 9/10 returns a netclasses_map (SWIG-wrapped std::map) that is dict-like. - net_classes = self.board.GetNetClasses() + # Net class DEFINITIONS live in .kicad_pro (net_settings) on + # KiCad 7+, not in the .kicad_pcb the SWIG board save writes. Resolve + # the project file up front so the durable write below runs even if a + # SWIG API (which shifted across KiCad 6->10) throws (issue #185). + pro_path = None + try: + board_path = self.board.GetFileName() + if board_path and board_path.endswith(".kicad_pcb"): + pro_path = str(Path(board_path).with_suffix(".kicad_pro")) + except Exception: + pro_path = None - existing = None - if hasattr(net_classes, "Find"): - existing = net_classes.Find(name) - else: - try: - if name in net_classes: - existing = net_classes[name] - except Exception: - existing = None - - if existing is None: - netclass = pcbnew.NETCLASS(name) - if hasattr(net_classes, "Add"): - net_classes.Add(netclass) - else: - net_classes[name] = netclass - else: - netclass = existing - - # Set properties scale = 1000000 # mm to nm + net_class_values: Dict[str, Any] = {} + in_memory_warning = None - # Defensive setters — KiCad 10's NETCLASS dropped some legacy mutators. - def _safe_set(method_name, value): - if value is None: - return - method = getattr(netclass, method_name, None) - if method is None: - return - try: - method(int(value * scale)) - except Exception: - pass + # Best-effort in-memory NETCLASS for the live session. A SWIG failure + # here is logged but must NOT skip the .kicad_pro persistence below, + # which is the deterministic, SWIG-independent part (issue #185). + try: + # KiCad 6/7 returns NETCLASSES with .Find/.Add; KiCad 9/10 returns + # a netclasses_map (SWIG-wrapped std::map) that is dict-like. + net_classes = self.board.GetNetClasses() - _safe_set("SetClearance", clearance) - _safe_set("SetTrackWidth", track_width) - _safe_set("SetViaDiameter", via_diameter) - _safe_set("SetViaDrill", via_drill) - _safe_set("SetMicroViaDiameter", uvia_diameter) - _safe_set("SetMicroViaDrill", uvia_drill) - _safe_set("SetDiffPairWidth", diff_pair_width) - _safe_set("SetDiffPairGap", diff_pair_gap) + existing = None + if hasattr(net_classes, "Find"): + existing = net_classes.Find(name) + else: + try: + if name in net_classes: + existing = net_classes[name] + except Exception: + existing = None - # Add nets to net class - netinfo = self.board.GetNetInfo() - nets_map = netinfo.NetsByName() - for net_name in nets: - if nets_map.has_key(net_name): - net = nets_map[net_name] - net.SetClass(netclass) + if existing is None: + netclass = pcbnew.NETCLASS(name) + if hasattr(net_classes, "Add"): + net_classes.Add(netclass) + else: + net_classes[name] = netclass + else: + netclass = existing - # Defensive accessors — KiCad 10's NETCLASS dropped some legacy getters. - def _safe_get(method_name): - method = getattr(netclass, method_name, None) - if method is None: - return None - try: - return method() / scale - except Exception: - return None + # Defensive setters — KiCad 10's NETCLASS dropped some legacy mutators. + def _safe_set(method_name, value): + if value is None: + return + method = getattr(netclass, method_name, None) + if method is None: + return + try: + method(int(value * scale)) + except Exception: + pass - return { - "success": True, - "message": f"Created net class: {name}", - "netClass": { - "name": name, + _safe_set("SetClearance", clearance) + _safe_set("SetTrackWidth", track_width) + _safe_set("SetViaDiameter", via_diameter) + _safe_set("SetViaDrill", via_drill) + _safe_set("SetMicroViaDiameter", uvia_diameter) + _safe_set("SetMicroViaDrill", uvia_drill) + _safe_set("SetDiffPairWidth", diff_pair_width) + _safe_set("SetDiffPairGap", diff_pair_gap) + + netinfo = self.board.GetNetInfo() + nets_map = netinfo.NetsByName() + for net_name in nets: + if nets_map.has_key(net_name): + net = nets_map[net_name] + net.SetClass(netclass) + + # Defensive accessors — KiCad 10's NETCLASS dropped some legacy getters. + def _safe_get(method_name): + method = getattr(netclass, method_name, None) + if method is None: + return None + try: + return method() / scale + except Exception: + return None + + net_class_values = { "clearance": _safe_get("GetClearance"), "trackWidth": _safe_get("GetTrackWidth"), "viaDiameter": _safe_get("GetViaDiameter"), @@ -1340,9 +1477,47 @@ class RoutingCommands: "uviaDrill": _safe_get("GetMicroViaDrill"), "diffPairWidth": _safe_get("GetDiffPairWidth"), "diffPairGap": _safe_get("GetDiffPairGap"), - "nets": nets, - }, + } + except Exception as exc: + in_memory_warning = "in-memory NETCLASS update failed: %s" % exc + logger.warning("create_netclass: %s", in_memory_warning) + + # Persist the class DEFINITION (net_settings.classes) using the same + # normalized values as the live path, so the persisted class can never + # diverge from the request. Membership lives in netclass_patterns and + # is out of scope here (create_netclass exposes no `nets` field). + project_props = { + "clearance": clearance, + "track_width": track_width, + "via_diameter": via_diameter, + "via_drill": via_drill, + "microvia_diameter": uvia_diameter, + "microvia_drill": uvia_drill, + "diff_pair_width": diff_pair_width, + "diff_pair_gap": diff_pair_gap, } + persist = persist_netclass_to_project(pro_path, name, project_props) + + warnings = [w for w in (in_memory_warning, persist.get("warning")) if w] + if in_memory_warning and not persist.get("persisted"): + # Neither the live board nor the project file received the class. + return { + "success": False, + "message": "Failed to create net class", + "errorDetails": "; ".join(warnings), + } + + result = { + "success": True, + "message": f"Created net class: {name}", + "netClass": {"name": name, "nets": nets, **net_class_values}, + "persisted": persist.get("persisted", False), + } + if persist.get("projectFile"): + result["projectFile"] = persist["projectFile"] + if warnings: + result["warning"] = "; ".join(warnings) + return result except Exception as e: logger.error(f"Error creating net class: {str(e)}") diff --git a/tests/test_netclass_kicad_pro_persistence.py b/tests/test_netclass_kicad_pro_persistence.py new file mode 100644 index 0000000..022e6f3 --- /dev/null +++ b/tests/test_netclass_kicad_pro_persistence.py @@ -0,0 +1,201 @@ +"""Tests for create_netclass persisting net class definitions to ``.kicad_pro`` (#185). + +Net class *definitions* live in ``.kicad_pro`` (``net_settings.classes``), +not in the ``.kicad_pcb`` board file, on KiCad 7+. These tests exercise the pure +JSON transform, the atomic file round-trip, and the create_netclass wiring without +needing a live KiCad / SWIG board. +""" + +import json +import sys +from pathlib import Path +from unittest.mock import MagicMock + +sys.path.insert(0, str(Path(__file__).parent.parent / "python")) + +from commands.routing import ( # noqa: E402 + RoutingCommands, + apply_netclass_to_project_settings, + persist_netclass_to_project, +) + + +def _project_with_default(): + return { + "net_settings": { + "classes": [ + { + "name": "Default", + "clearance": 0.2, + "track_width": 0.2, + "via_diameter": 0.6, + "via_drill": 0.3, + "priority": 2147483647, + } + ], + "netclass_patterns": [], + } + } + + +# --- pure transform ------------------------------------------------------- + + +def test_apply_adds_class_with_mm_fields(): + data = _project_with_default() + apply_netclass_to_project_settings( + data, "HV", {"clearance": 0.5, "track_width": 0.4, "via_diameter": 1.0, "via_drill": 0.5} + ) + hv = next(c for c in data["net_settings"]["classes"] if c["name"] == "HV") + assert hv["clearance"] == 0.5 + assert hv["track_width"] == 0.4 + assert hv["via_diameter"] == 1.0 + assert hv["via_drill"] == 0.5 + # custom classes get priority 0; the Default class keeps its own + assert hv["priority"] == 0 + + +def test_apply_clones_default_field_shape(): + data = _project_with_default() + apply_netclass_to_project_settings(data, "HV", {"clearance": 0.5}) + hv = next(c for c in data["net_settings"]["classes"] if c["name"] == "HV") + assert "track_width" in hv and "via_diameter" in hv + + +def test_apply_creates_class_template_when_no_default(): + data = {"net_settings": {"classes": []}} + apply_netclass_to_project_settings(data, "HV", {"clearance": 0.5}) + hv = next(c for c in data["net_settings"]["classes"] if c["name"] == "HV") + # full KiCad-10 field set from the fallback template + for key in ("bus_width", "track_width", "via_diameter", "via_drill", "line_style"): + assert key in hv + + +def test_apply_updates_existing_class_without_duplicating(): + data = _project_with_default() + apply_netclass_to_project_settings(data, "HV", {"clearance": 0.5}) + apply_netclass_to_project_settings(data, "HV", {"clearance": 0.8}) + hv = [c for c in data["net_settings"]["classes"] if c["name"] == "HV"] + assert len(hv) == 1 + assert hv[0]["clearance"] == 0.8 + + +def test_apply_creates_net_settings_when_absent(): + data = {} + apply_netclass_to_project_settings(data, "HV", {"clearance": 0.5}) + assert data["net_settings"]["classes"][0]["name"] == "HV" + + +# --- file persistence ----------------------------------------------------- + + +def test_persist_round_trips_through_a_real_file(tmp_path): + pro = tmp_path / "proj.kicad_pro" + pro.write_text(json.dumps(_project_with_default())) + result = persist_netclass_to_project(str(pro), "HV", {"clearance": 0.5, "track_width": 0.4}) + assert result["persisted"] is True + assert result["projectFile"] == str(pro) + hv = next( + c for c in json.loads(pro.read_text())["net_settings"]["classes"] if c["name"] == "HV" + ) + assert hv["clearance"] == 0.5 and hv["track_width"] == 0.4 + + +def test_persist_preserves_unrelated_project_and_net_settings_content(tmp_path): + pro = tmp_path / "proj.kicad_pro" + project = _project_with_default() + project["board"] = {"design_settings": {"rules": {"min_clearance": 0.1}}} + project["net_settings"]["meta"] = {"version": 4} + project["net_settings"]["net_colors"] = {"GND": "rgba(1, 2, 3, 0.5)"} + project["net_settings"]["netclass_assignments"] = {"GND": "Default"} + pro.write_text(json.dumps(project)) + persist_netclass_to_project(str(pro), "HV", {"clearance": 0.5}) + reloaded = json.loads(pro.read_text()) + assert reloaded["board"]["design_settings"]["rules"]["min_clearance"] == 0.1 + assert reloaded["net_settings"]["meta"] == {"version": 4} + assert reloaded["net_settings"]["net_colors"] == {"GND": "rgba(1, 2, 3, 0.5)"} + assert reloaded["net_settings"]["netclass_assignments"] == {"GND": "Default"} + default = next(c for c in reloaded["net_settings"]["classes"] if c["name"] == "Default") + assert default["priority"] == 2147483647 + + +def test_persist_writes_atomically_leaving_no_temp_file(tmp_path): + pro = tmp_path / "proj.kicad_pro" + pro.write_text(json.dumps(_project_with_default())) + persist_netclass_to_project(str(pro), "HV", {"clearance": 0.5}) + assert [p.name for p in tmp_path.iterdir()] == ["proj.kicad_pro"] + + +def test_persist_warns_when_no_project_file(): + result = persist_netclass_to_project(None, "HV", {"clearance": 0.5}) + assert result["persisted"] is False + assert "warning" in result + + +def test_persist_warns_on_malformed_json_and_leaves_file_intact(tmp_path): + pro = tmp_path / "proj.kicad_pro" + pro.write_text("{not valid json") + result = persist_netclass_to_project(str(pro), "HV", {"clearance": 0.5}) + assert result["persisted"] is False + assert str(pro) in result["warning"] + assert pro.read_text() == "{not valid json" # never half-written + + +def test_persist_warns_when_path_is_a_directory(tmp_path): + # a directory passes os.path.exists() but open()-for-read fails -> hits the except + result = persist_netclass_to_project(str(tmp_path), "HV", {"clearance": 0.5}) + assert result["persisted"] is False + assert str(tmp_path) in result["warning"] + + +# --- create_netclass wiring ---------------------------------------------- + + +def test_create_netclass_persists_canonical_trace_width(tmp_path): + # Regression: a schema-conformant call (canonical "traceWidth") must reach + # .kicad_pro as track_width, not the cloned Default's value. + pro = tmp_path / "p.kicad_pro" + pro.write_text(json.dumps(_project_with_default())) + board = MagicMock() + board.GetFileName.return_value = str(tmp_path / "p.kicad_pcb") + result = RoutingCommands(board).create_netclass( + {"name": "HV", "traceWidth": 5.0, "clearance": 0.5} + ) + assert result["success"] is True + assert result["persisted"] is True + hv = next( + c for c in json.loads(pro.read_text())["net_settings"]["classes"] if c["name"] == "HV" + ) + assert hv["track_width"] == 5.0 + assert hv["clearance"] == 0.5 + + +def test_create_netclass_persists_even_when_swig_path_fails(tmp_path): + # The .kicad_pro write is SWIG-independent: a SWIG throw must not skip it. + pro = tmp_path / "p.kicad_pro" + pro.write_text(json.dumps(_project_with_default())) + board = MagicMock() + board.GetNetClasses.side_effect = RuntimeError("swig boom") + board.GetFileName.return_value = str(tmp_path / "p.kicad_pcb") + result = RoutingCommands(board).create_netclass( + {"name": "HV", "traceWidth": 0.4, "clearance": 0.5} + ) + assert result["success"] is True + assert result["persisted"] is True + assert "swig boom" in result.get("warning", "") + hv = next( + c for c in json.loads(pro.read_text())["net_settings"]["classes"] if c["name"] == "HV" + ) + assert hv["track_width"] == 0.4 + + +def test_create_netclass_fails_when_neither_memory_nor_disk_succeeds(tmp_path): + board = MagicMock() + board.GetNetClasses.side_effect = RuntimeError("swig boom") + # sibling .kicad_pro does not exist -> persistence can't happen either + board.GetFileName.return_value = str(tmp_path / "missing.kicad_pcb") + result = RoutingCommands(board).create_netclass( + {"name": "HV", "traceWidth": 0.4, "clearance": 0.5} + ) + assert result["success"] is False + assert "swig boom" in result["errorDetails"]