diff --git a/src/mckicad/utils/freerouting.py b/src/mckicad/utils/freerouting.py index 7a6b249..c46d60e 100644 --- a/src/mckicad/utils/freerouting.py +++ b/src/mckicad/utils/freerouting.py @@ -45,6 +45,50 @@ class FreeRoutingError(Exception): pass +# Cache of {cli_path: supported flags}, so `freeroute --help` is probed at most +# once per CLI path. +_FREEROUTE_FLAGS: dict[str, set[str]] = {} + +_FREEROUTE_OPTIONAL_FLAGS = ("--engine", "--pack", "--shove", "--diagonal") + +# freeroute's engine/option matrix. Its CLI *errors* (exit 2) on an unsupported +# combination rather than ignoring it, so only send a flag the chosen engine +# actually implements. Channel packing is a room-engine feature; 45-degree +# routing lives on the exact track. +_FREEROUTE_ENGINE_FLAGS: dict[str, set[str]] = { + "grid": set(), + "exact": {"--shove", "--diagonal"}, + "room": {"--pack", "--shove"}, +} + + +def freeroute_supported_flags(freeroute_cli: str) -> set[str]: + """Which optional flags the installed freeroute CLI understands. + + freeroute's better engines (the continuous room router, channel packing, + 45-degree routing) landed after its first CLI, so probe ``--help`` once and + only pass flags this build actually advertises. An older freeroute simply + gets the bare ``-de``/``-do`` invocation instead of failing on an unknown + argument. + """ + cached = _FREEROUTE_FLAGS.get(freeroute_cli) + if cached is not None: + return cached + + flags: set[str] = set() + try: + result = subprocess.run( + [freeroute_cli, "--help"], capture_output=True, text=True, timeout=30 + ) + help_text = (result.stdout or "") + (result.stderr or "") + flags = {flag for flag in _FREEROUTE_OPTIONAL_FLAGS if flag in help_text} + except (OSError, subprocess.SubprocessError): + logger.debug("Could not probe freeroute --help; using the bare invocation") + + _FREEROUTE_FLAGS[freeroute_cli] = flags + return flags + + def find_kicad_python() -> str | None: """Locate a Python interpreter that can ``import pcbnew``. @@ -134,7 +178,16 @@ class FreeRoutingEngine: "automatic_neckdown": True, "postroute_optimization": True, "max_iterations": 1000, - "improvement_threshold": 0.01 + "improvement_threshold": 0.01, + # Native freeroute engine selection (ignored by the FreeRouting JAR). + # Default to the continuous room engine with channel packing: it routes + # channels the grid MVP cannot, its output is exactly DRC-clean, and + # packing fits traces a greedy pass drops. Diagonal (45 degree) lives on + # the exact track, not the room track, so it is off by default here. + "freeroute_engine": "room", + "freeroute_pack": True, + "freeroute_shove": False, + "freeroute_diagonal": False, } # Layer configuration @@ -357,7 +410,9 @@ class FreeRoutingEngine: # (not imported) so its GPL license stays cleanly separated from mckicad. freeroute_cli = find_freeroute_cli() if freeroute_cli: - ok, ses_path = self._run_freeroute_cli(freeroute_cli, dsn_path, output_directory) + ok, ses_path = self._run_freeroute_cli( + freeroute_cli, dsn_path, output_directory, routing_config + ) if ok: return True, ses_path logger.warning("freeroute produced no route; falling back to the FreeRouting JAR") @@ -424,19 +479,53 @@ class FreeRoutingEngine: return False, None def _run_freeroute_cli( - self, freeroute_cli: str, dsn_path: str, output_directory: str + self, + freeroute_cli: str, + dsn_path: str, + output_directory: str, + routing_config: dict[str, Any] | None = None, ) -> tuple[bool, str | None]: """Route with the native freeroute CLI (a drop-in for the JAR's -de/-do). + Selects freeroute's best *available* engine rather than accepting its + default grid router: the continuous room engine routes channels the grid + cannot and emits exactly DRC-clean geometry, and channel packing fits + traces a greedy pass drops. Options are only passed when the installed + CLI advertises them, so an older freeroute still routes. + Returns (success, ses_path). Unlike the JAR's ``-do ``, freeroute writes a single ``-do ``, so the SES path is explicit. """ + config = {**self.routing_config, **(routing_config or {})} name = os.path.splitext(os.path.basename(dsn_path))[0] ses_path = os.path.join(output_directory, f"{name}.ses") + + cmd = [freeroute_cli, "-de", dsn_path, "-do", ses_path] + supported = freeroute_supported_flags(freeroute_cli) + + engine = str(config.get("freeroute_engine") or "") + engine_flags = _FREEROUTE_ENGINE_FLAGS.get(engine, set()) + if engine and "--engine" in supported: + cmd += ["--engine", engine] + + for flag, key in ( + ("--pack", "freeroute_pack"), + ("--shove", "freeroute_shove"), + ("--diagonal", "freeroute_diagonal"), + ): + if not config.get(key) or flag not in supported: + continue + if engine and flag not in engine_flags: + logger.warning( + f"freeroute {flag} is not supported by --engine {engine}; skipping it" + ) + continue + cmd.append(flag) + try: - logger.info(f"Routing with native freeroute: {freeroute_cli}") + logger.info(f"Routing with native freeroute: {' '.join(cmd)}") result = subprocess.run( - [freeroute_cli, "-de", dsn_path, "-do", ses_path], + cmd, capture_output=True, text=True, timeout=300, diff --git a/tests/test_freeroute_engines.py b/tests/test_freeroute_engines.py new file mode 100644 index 0000000..75380e7 --- /dev/null +++ b/tests/test_freeroute_engines.py @@ -0,0 +1,82 @@ +"""mckicad selects freeroute's best *available* engine, not its grid default. + +freeroute's CLI exits 2 on an unsupported (engine, flag) combination rather than +ignoring it, and older freeroute builds don't advertise the flags at all — so the +command has to be built from both the CLI's probed capabilities and the engine's +own option matrix. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock, patch + +from mckicad.utils.freerouting import FreeRoutingEngine + +ALL_FLAGS = {"--engine", "--pack", "--shove", "--diagonal"} + + +def _run_capturing(captured): + def _run(cmd, **_kwargs): + captured.append(cmd) + out = cmd[cmd.index("-do") + 1] + with open(out, "w") as f: + f.write("(session board)") + return MagicMock(returncode=0, stdout="", stderr="") + + return _run + + +def _route(tmp_path, supported, config=None): + engine = FreeRoutingEngine() + dsn = tmp_path / "board.dsn" + dsn.write_text("(pcb)") + captured: list[list[str]] = [] + with ( + patch("mckicad.utils.freerouting.freeroute_supported_flags", return_value=supported), + patch("mckicad.utils.freerouting.subprocess.run", side_effect=_run_capturing(captured)), + ): + ok, ses = engine._run_freeroute_cli( + "/usr/bin/freeroute", str(dsn), str(tmp_path), config + ) + assert ok and ses + return captured[0] + + +def test_defaults_to_room_engine_with_packing(tmp_path): + cmd = _route(tmp_path, ALL_FLAGS) + assert "--engine" in cmd and cmd[cmd.index("--engine") + 1] == "room" + assert "--pack" in cmd + # 45-degree routing is an exact-track feature; sending it with room would + # make freeroute exit 2. + assert "--diagonal" not in cmd + + +def test_older_freeroute_gets_the_bare_invocation(tmp_path): + cmd = _route(tmp_path, set()) # a build that advertises no optional flags + assert "--engine" not in cmd + assert "--pack" not in cmd + assert cmd[1] == "-de" # still the plain JAR-compatible drop-in call + + +def test_incompatible_flag_is_skipped_not_sent(tmp_path): + cmd = _route( + tmp_path, + ALL_FLAGS, + {"freeroute_engine": "room", "freeroute_diagonal": True}, + ) + assert "--diagonal" not in cmd # room does not implement it + + +def test_exact_engine_can_use_diagonal(tmp_path): + cmd = _route( + tmp_path, + ALL_FLAGS, + { + "freeroute_engine": "exact", + "freeroute_pack": False, + "freeroute_diagonal": True, + }, + ) + assert cmd[cmd.index("--engine") + 1] == "exact" + assert "--diagonal" in cmd + assert "--pack" not in cmd # exact does not implement packing