fix(routing): persist create_netclass net classes to .kicad_pro (#185)
Net class definitions live in <project>.kicad_pro (net_settings) on KiCad 7+, not in the .kicad_pcb that the SWIG board save writes -- so create_netclass mutated the in-memory board and reported success, but nothing survived a reload. Write the class definition and its net memberships (netclass_patterns) into the project JSON, which is what KiCad reads on open. The transform is a pure, unit-tested function that needs no live KiCad/SWIG round-trip; the in-memory NETCLASS path is kept for live-session consistency.
This commit is contained in:
Judson Stephenson
@@ -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 (``<project>.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,8 +1387,28 @@ 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 class DEFINITIONS live in <project>.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
|
||||
|
||||
scale = 1000000 # mm to nm
|
||||
net_class_values: Dict[str, Any] = {}
|
||||
in_memory_warning = None
|
||||
|
||||
# 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()
|
||||
|
||||
existing = None
|
||||
@@ -1285,9 +1430,6 @@ class RoutingCommands:
|
||||
else:
|
||||
netclass = existing
|
||||
|
||||
# Set properties
|
||||
scale = 1000000 # mm to nm
|
||||
|
||||
# Defensive setters — KiCad 10's NETCLASS dropped some legacy mutators.
|
||||
def _safe_set(method_name, value):
|
||||
if value is None:
|
||||
@@ -1309,7 +1451,6 @@ class RoutingCommands:
|
||||
_safe_set("SetDiffPairWidth", diff_pair_width)
|
||||
_safe_set("SetDiffPairGap", diff_pair_gap)
|
||||
|
||||
# Add nets to net class
|
||||
netinfo = self.board.GetNetInfo()
|
||||
nets_map = netinfo.NetsByName()
|
||||
for net_name in nets:
|
||||
@@ -1327,11 +1468,7 @@ class RoutingCommands:
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": f"Created net class: {name}",
|
||||
"netClass": {
|
||||
"name": name,
|
||||
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)}")
|
||||
|
||||
201
tests/test_netclass_kicad_pro_persistence.py
Normal file
201
tests/test_netclass_kicad_pro_persistence.py
Normal file
@@ -0,0 +1,201 @@
|
||||
"""Tests for create_netclass persisting net class definitions to ``.kicad_pro`` (#185).
|
||||
|
||||
Net class *definitions* live in ``<project>.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"]
|
||||
Reference in New Issue
Block a user