From ed98f12c51a26887a4a18f39f5ccde6208f5df97 Mon Sep 17 00:00:00 2001 From: Milind Sharma Date: Sun, 22 Feb 2026 18:20:04 +0800 Subject: [PATCH] feat: expose via layer spans in typed model and CLI --- docs/TEST_CLI.md | 6 + docs/slash-commands/StandardChecks.md | 32 +++++ src/blocking.rs | 2 + src/client.rs | 196 ++++++++++++++++++++++++-- src/lib.rs | 6 +- src/model/board.rs | 8 ++ test-scripts/kicad-ipc-cli.rs | 61 ++++++++ 7 files changed, 295 insertions(+), 16 deletions(-) create mode 100644 docs/slash-commands/StandardChecks.md diff --git a/docs/TEST_CLI.md b/docs/TEST_CLI.md index e48e067..f11d229 100644 --- a/docs/TEST_CLI.md +++ b/docs/TEST_CLI.md @@ -61,6 +61,12 @@ List nets: cargo run --features blocking --bin kicad-ipc-cli -- nets ``` +List vias with typed via kind and layer span: + +```bash +cargo run --features blocking --bin kicad-ipc-cli -- vias +``` + List project net classes: ```bash diff --git a/docs/slash-commands/StandardChecks.md b/docs/slash-commands/StandardChecks.md new file mode 100644 index 0000000..f36e30d --- /dev/null +++ b/docs/slash-commands/StandardChecks.md @@ -0,0 +1,32 @@ +# /StandardChecks + +Use for any feature/change in this repo. + +## Required Flow + +1. Confirm low-level proto fields and comments for new/changed behavior. +2. Map into typed model structs (async path) with explicit absence handling (`Option`) and unknown-enum handling (`Unknown(...)` / `UNKNOWN_LAYER(...)`). +3. Update sync/blocking parity (`KiCadClientBlocking`) for any new async methods. +4. Update CLI surface (`test-scripts/kicad-ipc-cli.rs`) so feature is observable from terminal. +5. Add regression tests: + - decode/mapping test(s) + - CLI arg parse test(s) for new command/flags + - detail/format test(s) if output changed +6. Update docs touched by behavior/API (`README.md`, `docs/TEST_CLI.md`, runbooks as needed). +7. Run checks: + - `cargo fmt --all` + - `cargo test` + - `cargo test --features blocking` +8. Live verify with KiCad open (real socket): run CLI command(s) showing new data path and confirm expected fields. +9. Ship: + - `git status` + - commit with Conventional Commit + - push branch + - open PR + - request review with `@codex review`. + +## Output Expectations + +- Include exact file refs + what changed. +- Include command outputs summary for tests/live run. +- Call out any skipped checks with reason. diff --git a/src/blocking.rs b/src/blocking.rs index 31f78c7..d826d9a 100644 --- a/src/blocking.rs +++ b/src/blocking.rs @@ -441,6 +441,8 @@ impl KiCadClientBlocking { fn remove_from_selection_raw(&self, item_ids: Vec) -> Result, KiCadError>; fn remove_from_selection(&self, item_ids: Vec) -> Result; fn get_pad_netlist(&self) -> Result, KiCadError>; + fn get_vias_raw(&self) -> Result, KiCadError>; + fn get_vias(&self) -> Result, KiCadError>; fn get_items_raw_by_type_codes(&self, type_codes: Vec) -> Result, KiCadError>; fn get_items_details_by_type_codes(&self, type_codes: Vec) -> Result, KiCadError>; fn get_items_by_type_codes(&self, type_codes: Vec) -> Result, KiCadError>; diff --git a/src/client.rs b/src/client.rs index 10d7a42..38a403b 100644 --- a/src/client.rs +++ b/src/client.rs @@ -13,9 +13,9 @@ use crate::model::board::{ NetClassForNetEntry, NetClassInfo, NetClassType, NetColorDisplayMode, PadNetEntry, PadShapeAsPolygonEntry, PadstackPresenceEntry, PadstackPresenceState, PcbArc, PcbBoardGraphicShape, PcbBoardText, PcbBoardTextBox, PcbDimension, PcbField, PcbFootprint, - PcbGroup, PcbItem, PcbPad, PcbPadType, PcbTrack, PcbUnknownItem, PcbVia, PcbViaType, PcbZone, - PcbZoneType, PolyLineNm, PolyLineNodeGeometryNm, PolygonWithHolesNm, RatsnestDisplayMode, - Vector2Nm, + PcbGroup, PcbItem, PcbPad, PcbPadType, PcbTrack, PcbUnknownItem, PcbVia, PcbViaLayers, + PcbViaType, PcbZone, PcbZoneType, PolyLineNm, PolyLineNodeGeometryNm, PolygonWithHolesNm, + RatsnestDisplayMode, Vector2Nm, }; use crate::model::common::{ CommitAction, CommitSession, DocumentSpecifier, DocumentType, EditorFrameType, ItemBoundingBox, @@ -1136,6 +1136,24 @@ impl KiCadClient { pad_netlist_from_footprint_items(footprint_items) } + pub async fn get_vias_raw(&self) -> Result, KiCadError> { + self.get_items_raw(vec![common_types::KiCadObjectType::KotPcbVia as i32]) + .await + } + + pub async fn get_vias(&self) -> Result, KiCadError> { + let items = self + .get_items_by_type_codes(vec![common_types::KiCadObjectType::KotPcbVia as i32]) + .await?; + Ok(items + .into_iter() + .filter_map(|item| match item { + PcbItem::Via(via) => Some(via), + _ => None, + }) + .collect()) + } + pub fn pcb_object_type_codes() -> &'static [PcbObjectTypeCode] { &PCB_OBJECT_TYPES } @@ -2962,6 +2980,25 @@ fn map_via_type(value: i32) -> PcbViaType { } } +fn map_via_layers(pad_stack: Option) -> Option { + let pad_stack = pad_stack?; + + let (drill_start_layer, drill_end_layer) = if let Some(drill) = pad_stack.drill { + ( + Some(layer_to_model(drill.start_layer)), + Some(layer_to_model(drill.end_layer)), + ) + } else { + (None, None) + }; + + Some(PcbViaLayers { + padstack_layers: pad_stack.layers.into_iter().map(layer_to_model).collect(), + drill_start_layer, + drill_end_layer, + }) +} + fn map_pad_type(value: i32) -> PcbPadType { match board_types::PadType::try_from(value) { Ok(board_types::PadType::PtPth) => PcbPadType::Pth, @@ -3018,6 +3055,7 @@ fn decode_pcb_item(item: prost_types::Any) -> Result { id: via.id.map(|id| id.value), position_nm: via.position.map(map_vector2_nm), via_type: map_via_type(via.r#type), + layers: map_via_layers(via.pad_stack), net: map_optional_net(via.net), })); } @@ -3338,7 +3376,37 @@ fn format_via_selection_detail(via: board_types::Via) -> String { let via_type = board_types::ViaType::try_from(via.r#type) .map(|value| value.as_str_name().to_string()) .unwrap_or_else(|_| format!("UNKNOWN({})", via.r#type)); - format!("via id={id} pos_nm={position} type={via_type} net={net}") + let layers = map_via_layers(via.pad_stack); + let pad_layers = layers + .as_ref() + .map(|row| format_layer_names(&row.padstack_layers)) + .unwrap_or_else(|| "-".to_string()); + let drill_start = layers + .as_ref() + .and_then(|row| row.drill_start_layer.as_ref()) + .map(|layer| layer.name.as_str()) + .unwrap_or("-"); + let drill_end = layers + .as_ref() + .and_then(|row| row.drill_end_layer.as_ref()) + .map(|layer| layer.name.as_str()) + .unwrap_or("-"); + + format!( + "via id={id} pos_nm={position} type={via_type} net={net} pad_layers={pad_layers} drill_span={drill_start}->{drill_end}" + ) +} + +fn format_layer_names(layers: &[BoardLayerInfo]) -> String { + if layers.is_empty() { + return "-".to_string(); + } + + layers + .iter() + .map(|layer| layer.name.as_str()) + .collect::>() + .join(",") } fn format_footprint_selection_detail(footprint: board_types::FootprintInstance) -> String { @@ -3639,18 +3707,19 @@ fn default_client_name() -> String { mod tests { use super::{ any_to_pretty_debug, board_editor_appearance_settings_to_proto, board_stackup_to_proto, - commit_action_to_proto, drc_severity_to_proto, ensure_item_deletion_status_ok, - ensure_item_request_ok, ensure_item_status_ok, layer_to_model, map_board_stackup, - map_commit_session, map_hit_test_result, map_item_bounding_boxes, map_merge_mode_to_proto, - map_polygon_with_holes, map_run_action_status, model_document_to_proto, - normalize_socket_uri, pad_netlist_from_footprint_items, response_payload_as_any, - select_single_board_document, select_single_project_path, selection_item_detail, - summarize_item_details, summarize_selection, text_horizontal_alignment_to_proto, - text_spec_to_proto, PCB_OBJECT_TYPES, + commit_action_to_proto, decode_pcb_item, drc_severity_to_proto, + ensure_item_deletion_status_ok, ensure_item_request_ok, ensure_item_status_ok, + layer_to_model, map_board_stackup, map_commit_session, map_hit_test_result, + map_item_bounding_boxes, map_merge_mode_to_proto, map_polygon_with_holes, + map_run_action_status, model_document_to_proto, normalize_socket_uri, + pad_netlist_from_footprint_items, response_payload_as_any, select_single_board_document, + select_single_project_path, selection_item_detail, summarize_item_details, + summarize_selection, text_horizontal_alignment_to_proto, text_spec_to_proto, + PCB_OBJECT_TYPES, }; use crate::error::KiCadError; use crate::model::board::{ - BoardLayerInfo, BoardStackup, BoardStackupLayer, BoardStackupLayerType, + BoardLayerInfo, BoardStackup, BoardStackupLayer, BoardStackupLayerType, PcbItem, PcbViaType, }; use crate::model::common::{ CommitAction, DocumentSpecifier, DocumentType, ProjectInfo, TextAttributesSpec, @@ -4081,6 +4150,107 @@ mod tests { assert!(detail.contains("net=12:GND")); } + #[test] + fn decode_pcb_item_maps_via_layers() { + let via = crate::proto::kiapi::board::types::Via { + id: Some(crate::proto::kiapi::common::types::Kiid { + value: "via-id".to_string(), + }), + position: Some(crate::proto::kiapi::common::types::Vector2 { + x_nm: 100, + y_nm: 200, + }), + pad_stack: Some(crate::proto::kiapi::board::types::PadStack { + layers: vec![ + crate::proto::kiapi::board::types::BoardLayer::BlFCu as i32, + crate::proto::kiapi::board::types::BoardLayer::BlBCu as i32, + ], + drill: Some(crate::proto::kiapi::board::types::DrillProperties { + start_layer: crate::proto::kiapi::board::types::BoardLayer::BlFCu as i32, + end_layer: crate::proto::kiapi::board::types::BoardLayer::BlBCu as i32, + ..Default::default() + }), + ..Default::default() + }), + locked: 0, + net: Some(crate::proto::kiapi::board::types::Net { + code: Some(crate::proto::kiapi::board::types::NetCode { value: 7 }), + name: "VCC".to_string(), + }), + r#type: crate::proto::kiapi::board::types::ViaType::VtBlindBuried as i32, + }; + + let item = prost_types::Any { + type_url: super::envelope::type_url("kiapi.board.types.Via"), + value: via.encode_to_vec(), + }; + + let parsed = decode_pcb_item(item).expect("via payload should decode"); + match parsed { + PcbItem::Via(via) => { + assert_eq!(via.id.as_deref(), Some("via-id")); + assert_eq!(via.via_type, PcbViaType::BlindBuried); + let layers = via.layers.expect("via layers should decode"); + assert_eq!(layers.padstack_layers.len(), 2); + assert_eq!(layers.padstack_layers[0].name, "BL_F_Cu"); + assert_eq!(layers.padstack_layers[1].name, "BL_B_Cu"); + assert_eq!( + layers + .drill_start_layer + .as_ref() + .map(|layer| layer.name.as_str()), + Some("BL_F_Cu") + ); + assert_eq!( + layers + .drill_end_layer + .as_ref() + .map(|layer| layer.name.as_str()), + Some("BL_B_Cu") + ); + } + other => panic!("expected via item, got {other:?}"), + } + } + + #[test] + fn selection_item_detail_reports_via_layers() { + let via = crate::proto::kiapi::board::types::Via { + id: Some(crate::proto::kiapi::common::types::Kiid { + value: "via-id".to_string(), + }), + position: Some(crate::proto::kiapi::common::types::Vector2 { + x_nm: 100, + y_nm: 200, + }), + pad_stack: Some(crate::proto::kiapi::board::types::PadStack { + layers: vec![ + crate::proto::kiapi::board::types::BoardLayer::BlFCu as i32, + crate::proto::kiapi::board::types::BoardLayer::BlBCu as i32, + ], + drill: Some(crate::proto::kiapi::board::types::DrillProperties { + start_layer: crate::proto::kiapi::board::types::BoardLayer::BlFCu as i32, + end_layer: crate::proto::kiapi::board::types::BoardLayer::BlBCu as i32, + ..Default::default() + }), + ..Default::default() + }), + locked: 0, + net: None, + r#type: crate::proto::kiapi::board::types::ViaType::VtThrough as i32, + }; + + let item = prost_types::Any { + type_url: super::envelope::type_url("kiapi.board.types.Via"), + value: via.encode_to_vec(), + }; + + let detail = selection_item_detail(&item).expect("via detail should decode"); + assert!(detail.contains("type=VT_THROUGH")); + assert!(detail.contains("pad_layers=BL_F_Cu,BL_B_Cu")); + assert!(detail.contains("drill_span=BL_F_Cu->BL_B_Cu")); + } + #[test] fn pad_netlist_from_footprint_items_extracts_pad_entries() { let pad = crate::proto::kiapi::board::types::Pad { diff --git a/src/lib.rs b/src/lib.rs index 2665109..e5faea6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,9 +32,9 @@ pub use crate::model::board::{ NetClassForNetEntry, NetClassInfo, NetClassType, NetColorDisplayMode, PadNetEntry, PadShapeAsPolygonEntry, PadstackPresenceEntry, PadstackPresenceState, PcbArc, PcbBoardGraphicShape, PcbBoardText, PcbBoardTextBox, PcbDimension, PcbField, PcbFootprint, - PcbGroup, PcbItem, PcbPad, PcbPadType, PcbTrack, PcbUnknownItem, PcbVia, PcbViaType, PcbZone, - PcbZoneType, PolyLineNm, PolyLineNodeGeometryNm, PolygonWithHolesNm, RatsnestDisplayMode, - Vector2Nm, + PcbGroup, PcbItem, PcbPad, PcbPadType, PcbTrack, PcbUnknownItem, PcbVia, PcbViaLayers, + PcbViaType, PcbZone, PcbZoneType, PolyLineNm, PolyLineNodeGeometryNm, PolygonWithHolesNm, + RatsnestDisplayMode, Vector2Nm, }; pub use crate::model::common::{ CommitAction, CommitSession, DocumentSpecifier, DocumentType, EditorFrameType, ItemBoundingBox, diff --git a/src/model/board.rs b/src/model/board.rs index 86b5d36..a78dec6 100644 --- a/src/model/board.rs +++ b/src/model/board.rs @@ -326,6 +326,13 @@ pub enum PcbViaType { Unknown(i32), } +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct PcbViaLayers { + pub padstack_layers: Vec, + pub drill_start_layer: Option, + pub drill_end_layer: Option, +} + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum PcbPadType { Pth, @@ -370,6 +377,7 @@ pub struct PcbVia { pub id: Option, pub position_nm: Option, pub via_type: PcbViaType, + pub layers: Option, pub net: Option, } diff --git a/test-scripts/kicad-ipc-cli.rs b/test-scripts/kicad-ipc-cli.rs index d68f373..75c349a 100644 --- a/test-scripts/kicad-ipc-cli.rs +++ b/test-scripts/kicad-ipc-cli.rs @@ -60,6 +60,7 @@ enum Command { text: Vec, }, Nets, + Vias, EnabledLayers, SetEnabledLayers { copper_layer_count: u32, @@ -400,6 +401,46 @@ fn run() -> Result<(), KiCadError> { } } } + Command::Vias => { + let vias = client.get_vias()?; + println!("via_count={}", vias.len()); + for via in vias { + let net = via + .net + .as_ref() + .map(|row| format!("{}:{}", row.code, row.name)) + .unwrap_or_else(|| "-".to_string()); + let pad_layers = via + .layers + .as_ref() + .map(|row| format_layer_names_for_cli(&row.padstack_layers)) + .unwrap_or_else(|| "-".to_string()); + let drill_start = via + .layers + .as_ref() + .and_then(|row| row.drill_start_layer.as_ref()) + .map(|layer| layer.name.as_str()) + .unwrap_or("-"); + let drill_end = via + .layers + .as_ref() + .and_then(|row| row.drill_end_layer.as_ref()) + .map(|layer| layer.name.as_str()) + .unwrap_or("-"); + println!( + "id={} pos_nm={} type={:?} net={} pad_layers={} drill_span={}->{}", + via.id.as_deref().unwrap_or("-"), + via.position_nm + .map(|point| format!("{},{}", point.x_nm, point.y_nm)) + .unwrap_or_else(|| "-".to_string()), + via.via_type, + net, + pad_layers, + drill_start, + drill_end + ); + } + } Command::EnabledLayers => { let enabled = client.get_board_enabled_layers()?; println!("copper_layer_count={}", enabled.copper_layer_count); @@ -1108,6 +1149,7 @@ fn parse_args_from(mut args: Vec) -> Result<(CliConfig, Command), KiCadE Command::TextAsShapes { text } } "nets" => Command::Nets, + "vias" => Command::Vias, "enabled-layers" => Command::EnabledLayers, "set-enabled-layers" => { let mut copper_layer_count = None; @@ -2083,6 +2125,7 @@ COMMANDS: text-as-shapes Convert text to rendered shapes Options: --text (repeatable) nets List board nets (requires one open PCB) + vias List typed vias with via type + layer span netlist-pads Emit pad-level netlist data (with footprint context) items-by-id --id ... Show parsed details for specific item IDs item-bbox --id ... Show bounding boxes for item IDs @@ -2655,6 +2698,18 @@ fn polygon_geometry_summary(polygon: &kicad_ipc_rs::PolygonWithHolesNm) -> Polyg summary } +fn format_layer_names_for_cli(layers: &[kicad_ipc_rs::BoardLayerInfo]) -> String { + if layers.is_empty() { + return "-".to_string(); + } + + layers + .iter() + .map(|layer| layer.name.as_str()) + .collect::>() + .join(",") +} + fn parse_item_ids(args: &[String], command_name: &str) -> Result, KiCadError> { let mut item_ids = Vec::new(); let mut i = 0; @@ -2848,6 +2903,12 @@ mod tests { } } + #[test] + fn parse_args_parses_vias() { + let (_, command) = parse_args_from(vec!["vias".to_string()]).expect("vias should parse"); + assert!(matches!(command, Command::Vias)); + } + #[test] fn parse_args_parses_kicad_binary_path() { let (_, command) = parse_args_from(vec![