fix: create_schematic now respects the path parameter
Previously the path argument to create_schematic() was accepted by the tool schema but silently ignored in SchematicManager.create_schematic() (python/commands/schematic.py). The schematic file was always written using only the bare filename, resolving to the MCP server's working directory. On systems where that directory is not writable (e.g. Program Files, OneDrive-synced folders) this caused: [Errno 13] Permission denied: 'myproject.kicad_sch' Fix: - Add path: Optional[str] = None parameter to SchematicManager.create_schematic() - Compute output_path as os.path.join(path, base_name) when path is given - Forward the resolved path from _handle_create_schematic() into create_schematic() Tests: added tests/test_create_schematic_path.py with three unit tests covering: path respected, no-path fallback, and no double-suffix on .kicad_sch names. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -12,7 +12,7 @@ class SchematicManager:
|
||||
"""Core schematic operations using kicad-skip"""
|
||||
|
||||
@staticmethod
|
||||
def create_schematic(name, metadata=None):
|
||||
def create_schematic(name, path=None, metadata=None):
|
||||
"""Create a new empty schematic from template"""
|
||||
try:
|
||||
# Determine template path (use template_with_symbols for component cloning support)
|
||||
@@ -24,7 +24,8 @@ class SchematicManager:
|
||||
)
|
||||
|
||||
# Determine output path
|
||||
output_path = name if name.endswith(".kicad_sch") else f"{name}.kicad_sch"
|
||||
base_name = name if name.endswith(".kicad_sch") else f"{name}.kicad_sch"
|
||||
output_path = os.path.join(path, base_name) if path else base_name
|
||||
|
||||
if os.path.exists(template_path):
|
||||
# Copy template to target location
|
||||
|
||||
@@ -614,7 +614,8 @@ class KiCADInterface:
|
||||
"message": "Schematic name is required. Provide 'name', 'projectName', or 'filename' parameter.",
|
||||
}
|
||||
|
||||
schematic = SchematicManager.create_schematic(project_name, metadata)
|
||||
sch_path = path if path and path != "." else None
|
||||
schematic = SchematicManager.create_schematic(project_name, path=sch_path, metadata=metadata)
|
||||
file_path = f"{path}/{project_name}.kicad_sch"
|
||||
success = SchematicManager.save_schematic(schematic, file_path)
|
||||
|
||||
|
||||
89
tests/test_create_schematic_path.py
Normal file
89
tests/test_create_schematic_path.py
Normal file
@@ -0,0 +1,89 @@
|
||||
"""
|
||||
Unit tests for create_schematic path parameter bug fix.
|
||||
|
||||
Verifies that create_schematic respects the `path` argument and writes
|
||||
the schematic file to the correct directory instead of the process cwd.
|
||||
"""
|
||||
|
||||
import os
|
||||
import sys
|
||||
import importlib.util
|
||||
import tempfile
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
# pcbnew and skip are only available inside KiCAD — stub them so the
|
||||
# schematic module can be imported in a plain Python environment.
|
||||
sys.modules.setdefault("pcbnew", MagicMock())
|
||||
sys.modules.setdefault("skip", MagicMock())
|
||||
|
||||
# Import the module directly (bypasses python/commands/__init__.py which
|
||||
# would otherwise pull in board/component commands that also need pcbnew).
|
||||
_spec = importlib.util.spec_from_file_location(
|
||||
"schematic_module",
|
||||
os.path.join(os.path.dirname(__file__), "..", "python", "commands", "schematic.py"),
|
||||
)
|
||||
_mod = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(_mod)
|
||||
SchematicManager = _mod.SchematicManager
|
||||
|
||||
_OPEN_MOCK = MagicMock(return_value=MagicMock(
|
||||
__enter__=MagicMock(return_value=MagicMock(
|
||||
read=MagicMock(return_value="(uuid 00000000-0000-0000-0000-000000000000)")
|
||||
)),
|
||||
__exit__=MagicMock(return_value=False),
|
||||
))
|
||||
|
||||
|
||||
def test_create_schematic_uses_path_argument():
|
||||
"""
|
||||
create_schematic should write the .kicad_sch file inside `path`
|
||||
when that argument is provided, not in the process working directory.
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
with patch.object(_mod, "Schematic") as mock_sch_cls, \
|
||||
patch("shutil.copy"), \
|
||||
patch("os.path.exists", return_value=True), \
|
||||
patch("builtins.open", _OPEN_MOCK):
|
||||
mock_sch_cls.return_value = MagicMock()
|
||||
|
||||
SchematicManager.create_schematic("myschematic", path=tmpdir)
|
||||
|
||||
used_path = mock_sch_cls.call_args[0][0]
|
||||
assert used_path.startswith(tmpdir), (
|
||||
f"Expected path inside {tmpdir!r}, got {used_path!r}"
|
||||
)
|
||||
assert used_path.endswith("myschematic.kicad_sch")
|
||||
|
||||
|
||||
def test_create_schematic_without_path_uses_relative():
|
||||
"""
|
||||
When no path is given, behaviour is unchanged — file goes to cwd-relative name.
|
||||
"""
|
||||
with patch.object(_mod, "Schematic") as mock_sch_cls, \
|
||||
patch("shutil.copy"), \
|
||||
patch("os.path.exists", return_value=True), \
|
||||
patch("builtins.open", _OPEN_MOCK):
|
||||
mock_sch_cls.return_value = MagicMock()
|
||||
|
||||
SchematicManager.create_schematic("myschematic")
|
||||
|
||||
used_path = mock_sch_cls.call_args[0][0]
|
||||
assert used_path == "myschematic.kicad_sch"
|
||||
|
||||
|
||||
def test_create_schematic_accepts_full_sch_filename():
|
||||
"""
|
||||
If name already ends with .kicad_sch, it should not double the suffix.
|
||||
"""
|
||||
with tempfile.TemporaryDirectory() as tmpdir:
|
||||
with patch.object(_mod, "Schematic") as mock_sch_cls, \
|
||||
patch("shutil.copy"), \
|
||||
patch("os.path.exists", return_value=True), \
|
||||
patch("builtins.open", _OPEN_MOCK):
|
||||
mock_sch_cls.return_value = MagicMock()
|
||||
|
||||
SchematicManager.create_schematic("myschematic.kicad_sch", path=tmpdir)
|
||||
|
||||
used_path = mock_sch_cls.call_args[0][0]
|
||||
assert used_path.endswith("myschematic.kicad_sch")
|
||||
assert "myschematic.kicad_sch.kicad_sch" not in used_path
|
||||
Reference in New Issue
Block a user