From ffe51f0f1693bda329af656004a4e7a0e17b56ff Mon Sep 17 00:00:00 2001 From: Eugene Mikhantyev Date: Fri, 13 Mar 2026 00:23:26 +0000 Subject: [PATCH] fix: address review issues in schematic tools - Remove unused imports in move_schematic_component - Add warning log when mirror attribute missing on rotate - Move `import re` out of loop in annotate_schematic - Include global_label in list_schematic_nets - Fix SVG export to use directory for kicad-cli --output flag - Return actual SVG data in get_schematic_view TS handler - Add isError: true to all failure responses in new tools - Add connect_passthrough to schematic category in registry - Simplify power symbol filtering control flow in list_labels - Fix indentation in list_schematic_labels power section Co-Authored-By: Claude Opus 4.6 --- python/kicad_interface.py | 73 ++++++++++++++++++++++++++++----------- src/tools/registry.ts | 1 + src/tools/schematic.ts | 31 +++++++++++------ 3 files changed, 74 insertions(+), 31 deletions(-) diff --git a/python/kicad_interface.py b/python/kicad_interface.py index 826f67d..664610e 100644 --- a/python/kicad_interface.py +++ b/python/kicad_interface.py @@ -1592,12 +1592,16 @@ class KiCADInterface: if not schematic: return {"success": False, "message": "Failed to load schematic"} - # Get all net names from labels + # Get all net names from labels and global labels net_names = set() if hasattr(schematic, "label"): for label in schematic.label: if hasattr(label, "value"): net_names.add(label.value) + if hasattr(schematic, "global_label"): + for label in schematic.global_label: + if hasattr(label, "value"): + net_names.add(label.value) nets = [] for net_name in sorted(net_names): @@ -1697,16 +1701,17 @@ class KiCADInterface: if not hasattr(symbol.property, "Reference"): continue ref = symbol.property.Reference.value - if ref.startswith("#PWR") or ref.startswith("_TEMPLATE"): - if ref.startswith("_TEMPLATE"): - continue - value = symbol.property.Value.value if hasattr(symbol.property, "Value") else ref - pos = symbol.at.value if hasattr(symbol, "at") else [0, 0, 0] - labels.append({ - "name": value, - "type": "power", - "position": {"x": float(pos[0]), "y": float(pos[1])}, - }) + if ref.startswith("_TEMPLATE"): + continue + if not ref.startswith("#PWR"): + continue + value = symbol.property.Value.value if hasattr(symbol.property, "Value") else ref + pos = symbol.at.value if hasattr(symbol, "at") else [0, 0, 0] + labels.append({ + "name": value, + "type": "power", + "position": {"x": float(pos[0]), "y": float(pos[1])}, + }) return {"success": True, "labels": labels, "count": len(labels)} @@ -1720,9 +1725,6 @@ class KiCADInterface: """Move a schematic component to a new position""" logger.info("Moving schematic component") try: - from pathlib import Path - import re - schematic_path = params.get("schematicPath") reference = params.get("reference") position = params.get("position", {}) @@ -1793,8 +1795,15 @@ class KiCADInterface: symbol.at.value = pos # Handle mirror if specified - if mirror and hasattr(symbol, "mirror"): - symbol.mirror.value = mirror + if mirror: + if hasattr(symbol, "mirror"): + symbol.mirror.value = mirror + else: + logger.warning( + f"Mirror '{mirror}' requested for {reference}, " + f"but symbol does not have a 'mirror' attribute; " + f"mirror not applied" + ) SchematicManager.save_schematic(schematic, schematic_path) return {"success": True, "reference": reference, "angle": angle} @@ -1811,6 +1820,8 @@ class KiCADInterface: """Annotate unannotated components in schematic (R? -> R1, R2, ...)""" logger.info("Annotating schematic") try: + import re + schematic_path = params.get("schematicPath") if not schematic_path: return {"success": False, "message": "schematicPath is required"} @@ -1831,7 +1842,6 @@ class KiCADInterface: continue # Split reference into prefix and number - import re match = re.match(r'^([A-Za-z_]+)(\d+)$', ref) if match: prefix = match.group(1) @@ -1990,6 +2000,8 @@ class KiCADInterface: """Export schematic to SVG using kicad-cli""" logger.info("Exporting schematic SVG") import subprocess + import glob + import shutil try: schematic_path = params.get("schematicPath") @@ -2001,18 +2013,37 @@ class KiCADInterface: if not os.path.exists(schematic_path): return {"success": False, "message": f"Schematic not found: {schematic_path}"} - cmd = ["kicad-cli", "sch", "export", "svg", schematic_path, "-o", output_path] + # kicad-cli's --output flag for SVG export expects a directory, not a file path. + # The output file is auto-named based on the schematic name. + output_dir = os.path.dirname(output_path) + if not output_dir: + output_dir = "." + + os.makedirs(output_dir, exist_ok=True) + + cmd = ["kicad-cli", "sch", "export", "svg", schematic_path, "-o", output_dir] if params.get("blackAndWhite"): cmd.append("--black-and-white") result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) - if result.returncode == 0: - return {"success": True, "file": {"path": output_path}} - else: + if result.returncode != 0: return {"success": False, "message": f"kicad-cli failed: {result.stderr}"} + # kicad-cli names the file after the schematic, so find the generated SVG + svg_files = glob.glob(os.path.join(output_dir, "*.svg")) + if not svg_files: + return {"success": False, "message": "No SVG file produced by kicad-cli"} + + generated_svg = svg_files[0] + + # Move/rename to the user-specified output path if it differs + if os.path.abspath(generated_svg) != os.path.abspath(output_path): + shutil.move(generated_svg, output_path) + + return {"success": True, "file": {"path": output_path}} + except FileNotFoundError: return {"success": False, "message": "kicad-cli not found in PATH"} except Exception as e: diff --git a/src/tools/registry.ts b/src/tools/registry.ts index 9841461..ba5602d 100644 --- a/src/tools/registry.ts +++ b/src/tools/registry.ts @@ -97,6 +97,7 @@ export const toolCategories: ToolCategory[] = [ "add_schematic_net_label", "delete_schematic_net_label", "connect_to_net", + "connect_passthrough", "get_net_connections", "list_schematic_nets", "list_schematic_wires", diff --git a/src/tools/schematic.ts b/src/tools/schematic.ts index a083099..cc17cbf 100644 --- a/src/tools/schematic.ts +++ b/src/tools/schematic.ts @@ -491,6 +491,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to list components: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -530,6 +531,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp content: [ { type: "text", text: `Failed to list nets: ${result.message || "Unknown error"}` }, ], + isError: true, }; }, ); @@ -567,6 +569,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp content: [ { type: "text", text: `Failed to list wires: ${result.message || "Unknown error"}` }, ], + isError: true, }; }, ); @@ -604,6 +607,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp content: [ { type: "text", text: `Failed to list labels: ${result.message || "Unknown error"}` }, ], + isError: true, }; }, ); @@ -645,6 +649,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to move component: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -686,6 +691,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to rotate component: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -727,6 +733,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to annotate: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -767,6 +774,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to delete wire: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -806,6 +814,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to delete label: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -845,6 +854,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to export SVG: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -884,6 +894,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to export PDF: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, ); @@ -910,16 +921,15 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp const result = await callKicadScript("get_schematic_view", args); if (result.success) { if (result.format === "svg") { - return { - content: [ - { - type: "text", - text: result.message - ? `SVG data returned (${result.message})` - : `SVG schematic view (${result.imageData?.length || 0} chars)`, - }, - ], - }; + const parts: { type: "text"; text: string }[] = []; + if (result.message) { + parts.push({ type: "text", text: result.message }); + } + parts.push({ + type: "text", + text: result.imageData || "", + }); + return { content: parts }; } // PNG — return as base64 image return { @@ -939,6 +949,7 @@ Note: operates on .kicad_sch files only. To modify a PCB footprint use edit_comp text: `Failed to get schematic view: ${result.message || "Unknown error"}`, }, ], + isError: true, }; }, );