From 86e8019bc8a13ea363e3a8fef1d36471d0377689 Mon Sep 17 00:00:00 2001 From: nordic-style <11313330+nordic-style@users.noreply.github.com> Date: Tue, 18 Aug 2026 01:02:41 +0200 Subject: [PATCH] fix(schematic): respect intentional unconnected pins The component validator reported pins declared no-connect, ignored its public power-pin option, and projected every unit's pins through each placed unit. Preserve KiCad electrical types, honor the filter, exempt explicit and intrinsic no-connect pins, and resolve only the selected unit. Refs #182 --- crates/konnect-core/src/tools/sch_batch.rs | 113 +++++++++++++++++++-- crates/konnect-sexp/src/schematic.rs | 18 ++++ 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/crates/konnect-core/src/tools/sch_batch.rs b/crates/konnect-core/src/tools/sch_batch.rs index 6cdacdb0..89c7b375 100644 --- a/crates/konnect-core/src/tools/sch_batch.rs +++ b/crates/konnect-core/src/tools/sch_batch.rs @@ -15,9 +15,9 @@ use konnect_schematic_editor as cse; use konnect_sexp::{ geometry::{point_on_segment, points_coincident, snap_point}, schematic::{ - extract_all_net_labels, extract_labels, extract_lib_pins, extract_symbol_instances, - extract_wires, find_lib_symbol, format_net_label, format_wire, pin_endpoint, - pin_label_rotation, read_schematic, + extract_all_net_labels, extract_labels, extract_lib_pins, extract_lib_pins_for_unit, + extract_symbol_instances, extract_wires, find_lib_symbol, format_net_label, format_wire, + pin_endpoint, pin_label_rotation, read_schematic, }, writer::{ apply_edits, find_block_with_leading_whitespace, find_enclosing_direct_child_block, @@ -278,9 +278,10 @@ pub fn tools() -> Vec { ), tool!( "validate_component_connections", - "Check that every non-passive pin on every component has at least one wire \ - or label connected. Reports unconnected pins with reference, pin number, \ - and schematic position.", + "Check that every connectable pin on every component has at least one wire \ + or label connected. Symbol pins typed no_connect and pins carrying a \ + no-connect marker are exempt. Reports unconnected pins with reference, \ + pin number, electrical type, and schematic position.", json!({ "type": "object", "properties": { @@ -1252,6 +1253,7 @@ async fn handle_validate_component_connections( .collect() }) .unwrap_or_default(); + let ignore_power_pins = args["ignore_power_pins"].as_bool().unwrap_or(false); let tol = 0.01_f64; let (_, tree) = read_schematic(&sch_path)?; @@ -1311,7 +1313,19 @@ async fn handle_validate_component_connections( let lib_sym = find_lib_symbol(&lib_syms, inst); if let Some(sym) = lib_sym { let t = inst.pin_transform(); - for pin in extract_lib_pins(sym) { + for pin in extract_lib_pins_for_unit(sym, inst.unit) { + // A library-declared no-connect pin is intentional by + // definition and does not need a placed X marker. This is + // distinct from an ordinary pin carrying a schematic + // `(no_connect ...)` marker, which is handled below. + if pin.electrical_type == "no_connect" { + continue; + } + if ignore_power_pins + && matches!(pin.electrical_type.as_str(), "power_in" | "power_out") + { + continue; + } let (px, py) = pin_endpoint(&pin, t); // Skip intentional no-connects @@ -1328,6 +1342,7 @@ async fn handle_validate_component_connections( "value": inst.value, "pin": pin.number, "pin_name": pin.name, + "pin_type": pin.electrical_type, "x": px, "y": py })); @@ -1685,6 +1700,90 @@ mod midwire_pin_tests { ); } } + + fn typed_pin_schematic() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("typed-pins.kicad_sch"); + std::fs::write( + &path, + r#"(kicad_sch + (version 20260306) + (generator "eeschema") + (uuid "root") + (lib_symbols + (symbol "Test:Typed" + (symbol "Typed_1_1" + (pin no_connect line (at 0 0 0) (length 0) + (name "NC" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27))))) + (pin power_in line (at 0 2.54 0) (length 2.54) + (name "VDD" (effects (font (size 1.27 1.27)))) + (number "2" (effects (font (size 1.27 1.27))))) + (pin output line (at 0 5.08 0) (length 2.54) + (name "OUT" (effects (font (size 1.27 1.27)))) + (number "3" (effects (font (size 1.27 1.27)))))) + (symbol "Typed_2_1" + (pin input line (at 0 7.62 0) (length 2.54) + (name "OTHER_UNIT" (effects (font (size 1.27 1.27)))) + (number "4" (effects (font (size 1.27 1.27)))))))) + (symbol + (lib_id "Test:Typed") + (at 100 80 0) + (unit 1) + (uuid "u1") + (property "Reference" "U1" (at 100 75 0)) + (property "Value" "Typed" (at 100 77 0))) + (sheet_instances (path "/" (page "1")))) +"#, + ) + .unwrap(); + (dir, path) + } + + async fn validate_components_json( + schematic: &std::path::Path, + ignore_power_pins: bool, + ) -> serde_json::Value { + let result = handle_validate_component_connections( + &json!({ + "schematic": schematic.display().to_string(), + "ignore_power_pins": ignore_power_pins + }), + &test_ctx(), + ) + .await + .unwrap(); + let crate::mcp::protocol::ToolContent::Text { text } = &result.content[0] else { + panic!("expected text content"); + }; + serde_json::from_str(text).unwrap() + } + + #[tokio::test] + async fn declared_no_connect_and_other_unit_pins_are_not_reported() { + let (_dir, path) = typed_pin_schematic(); + let body = validate_components_json(&path, false).await; + let pins = body["unconnected_pins"].as_array().unwrap(); + + assert_eq!(body["unconnected_count"], 2); + assert_eq!(pins[0]["pin"], "2"); + assert_eq!(pins[0]["pin_type"], "power_in"); + assert_eq!(pins[1]["pin"], "3"); + assert_eq!(pins[1]["pin_type"], "output"); + assert!(pins.iter().all(|pin| pin["pin"] != "1")); + assert!(pins.iter().all(|pin| pin["pin"] != "4")); + } + + #[tokio::test] + async fn ignore_power_pins_option_is_effective() { + let (_dir, path) = typed_pin_schematic(); + let body = validate_components_json(&path, true).await; + let pins = body["unconnected_pins"].as_array().unwrap(); + + assert_eq!(body["unconnected_count"], 1); + assert_eq!(pins[0]["pin"], "3"); + assert_eq!(pins[0]["pin_type"], "output"); + } } #[cfg(test)] diff --git a/crates/konnect-sexp/src/schematic.rs b/crates/konnect-sexp/src/schematic.rs index 06055fd2..99273615 100644 --- a/crates/konnect-sexp/src/schematic.rs +++ b/crates/konnect-sexp/src/schematic.rs @@ -936,6 +936,24 @@ mod unit_pin_tests { ); } + #[test] + fn extraction_preserves_the_electrical_pin_type() { + let root = parse_sexp( + r#"(kicad_symbol_lib + (symbol "TEST" + (pin no_connect line (at 0 0 0) (length 0) + (name "NC" (effects (font (size 1.27 1.27)))) + (number "1" (effects (font (size 1.27 1.27)))))))"#, + ) + .unwrap(); + let pin = extract_lib_pins(root.find("symbol").unwrap()) + .into_iter() + .next() + .unwrap(); + + assert_eq!(pin.electrical_type, "no_connect"); + } + #[test] fn underscored_base_names_parse_their_unit_suffix() { assert_eq!(parse_subsymbol_unit("R_Small_1_1"), Some(1));