KiCad - the KiCad Bridge
Public Made by Adomby adom
Drive your own KiCad from an AI, in the background: open every editor, read the whole board as data (the EDA Bridge Standard board model), stack-up, part heights, netlists, place, route, pour, DRC and exports.
1.0.107: kicad_move_footprint refuses unknown arguments (PR #70) without refusing real calls #72
1.0.107: kicad_move_footprint refuses unknown arguments (PR #70) without refusing real calls
PR #70 (Josh Aberdeen) makes kicad_move_footprint refuse a key it does not read, such as rot for rotation, which was silently ignored: the part moved but did not turn. As merged, its allowlist also refused keys that real calls carry, so every move through Adom Bridge would have failed:
- reason (and caller, aiThread), which AI clients put in the arguments of every call;
- _fromFrame, which the bridge adds when it converts a frame:"standard" call to KiCad's frame;
- ipcTimeoutMs, which the placement helpers pass on;
- the arguments of kicad_align_parts (mode, by, to, source, ...), which it handed on to the move whole.
This follow-up:
- accepts reason, caller, aiThread, ipcTimeoutMs and any key starting with _ at the top level; refs[] entries keep PR #70's strict ref/x/y/rotation/side;
- moves the allowed keys to placement::MOVE_KEYS / MOVE_ENTRY_KEYS, and adds placement::move_args_only, which kicad_align_parts now uses to pass on only the move's arguments (live and dryRun);
- counts unknown_argument and invalid_arguments as the caller's own mistake (no _reportIssues line);
- names unknown_argument in kicad_move_footprint's hint.
Tested: PR #70's tests, plus a new test that sends a real call (reason, caller, frame:"standard" converted to _fromFrame, ipcTimeoutMs) through plan_moves, checks rot is still refused with suggestedKey rotation beside them, and checks align's arguments are trimmed to the move's. All workspace tests pass. Laptop check after the insiders build.
🤖 Generated with Claude Code
Diff Skip to comments
@@ -1,1941 +1,1993 @@⋯ 565 unchanged lines ⋯ } } +/// The keys `kicad_move_footprint` reads at the top level (wiki PR #70).+pub const MOVE_KEYS: &[&str] = &[+ "filePath", "socketPath", "expectedRevision", "ref", "x", "y",+ "rotation", "side", "refs", "dryRun", "allowOverlap",+ "allowOutside", "allowLocked", "save", "frame", "ipcTimeoutMs",+];+/// The keys of one `refs[]` entry.+pub const MOVE_ENTRY_KEYS: &[&str] = &["ref", "x", "y", "rotation", "side"];+/// Keys any verb may carry that are not about the move: the asker's reason and identity, which+/// Adom Bridge and AI clients put in the arguments of every call. Keys starting with `_` are the+/// bridge's own notes (`_fromFrame` after a `frame:"standard"` call), also accepted.+const CALL_KEYS: &[&str] = &["reason", "caller", "aiThread"];++fn move_key_allowed(key: &str) -> bool {+ MOVE_KEYS.contains(&key) || CALL_KEYS.contains(&key) || key.starts_with('_')+}++/// Only the arguments a move reads (plus the call's reason and the bridge's `_` notes), for a+/// verb that hands its own arguments on to `kicad_move_footprint` (`kicad_align_parts`).+pub fn move_args_only(args: &Value) -> Value {+ match args.as_object() {+ Some(o) => Value::Object(o.iter().filter(|(k, _)| move_key_allowed(k)).map(|(k, v)| (k.clone(), v.clone())).collect()),+ None => args.clone(),+ }+}+ /// Validate the *whole* move payload before resolving geometry or opening the live /// editor. JSON getters otherwise silently treat typos such as `rot` as an omitted /// `rotation`, which can move copper-connected parts without rotating their pads.⋯ 1 unchanged line ⋯ /// Includes the standard board-frame and IPC fields. Batch entries have their /// own much smaller schema so typos inside `refs` fail the same way. pub fn validate_move_arguments(args: &Value) -> Result<(), RoutingError> {- const ROOT: &[&str] = &[- "filePath", "socketPath", "expectedRevision", "ref", "x", "y",- "rotation", "side", "refs", "dryRun", "allowOverlap",- "allowOutside", "allowLocked", "save", "frame",- ];- const ENTRY: &[&str] = &["ref", "x", "y", "rotation", "side"];- fn check(value: &Value, allowed: &[&str], context: &str) -> Result<(), RoutingError> {+ fn check(value: &Value, allowed: &[&str], top: bool, context: &str) -> Result<(), RoutingError> { let map = value.as_object().ok_or_else(|| { RoutingError::new("invalid_arguments", format!("{context} must be a JSON object")) .with("mutated", json!(false)) })?; for key in map.keys() {- if !allowed.contains(&key.as_str()) {+ let ok = if top { move_key_allowed(key) } else { allowed.contains(&key.as_str()) };+ if !ok { let suggested = if key == "rot" || key == "angle" { Some("rotation") } else {⋯ 18 unchanged lines ⋯ } Ok(()) }- check(args, ROOT, "move")?;+ check(args, MOVE_KEYS, true, "move")?; if let Some(Value::Array(entries)) = args.get("refs") { for (index, entry) in entries.iter().enumerate() {- check(entry, ENTRY, &format!("refs[{index}]"))?;+ check(entry, MOVE_ENTRY_KEYS, false, &format!("refs[{index}]"))?; } } Ok(())⋯ 767 unchanged lines ⋯ } #[test]+ fn a_real_call_with_its_reason_caller_and_frame_note_still_moves() {+ // What reaches plan_moves from Adom Bridge: the reason and caller ride in the arguments,+ // frame:"standard" has become _fromFrame, and helpers pass ipcTimeoutMs.+ let b = board(&[fp("Q2", 110.0, 110.0, 0.0, "F.Cu", RECT, "")]);+ let call = json!({"filePath": "C:/p/b.kicad_pcb", "ref": "Q2", "x": 120, "y": -110, "rotation": 90, "frame": "standard",+ "dryRun": true, "reason": "move Q2 beside the connector", "caller": {"thread": "t"}, "aiThread": "t", "ipcTimeoutMs": 20000});+ let native = crate::board_model::args_to_kicad_frame(&call).unwrap();+ assert_eq!(native["_fromFrame"], json!("standard"));+ let plan = plan_moves(&b, &native).unwrap();+ assert_eq!((plan.moves[0].after.x, plan.moves[0].after.y, plan.moves[0].after.rotation), (120.0, 110.0, 90.0));+ // a misspelling is still caught beside them, and a _ key inside a refs entry is not a bridge note+ let e = plan_moves(&b, &json!({"ref": "Q2", "x": 120, "y": 110, "reason": "r", "_fromFrame": "standard", "rot": 90})).unwrap_err();+ assert_eq!((e.code.as_str(), e.detail["suggestedKey"].clone()), ("unknown_argument", json!("rotation")));+ let e = plan_moves(&b, &json!({"refs": [{"ref": "Q2", "x": 120, "y": 110, "_x": 1}]})).unwrap_err();+ assert_eq!(e.code, "unknown_argument");+ // kicad_align_parts hands its own arguments on: only the move's survive+ let align = json!({"filePath": "C:/p/b.kicad_pcb", "refs": [{"ref": "Q2", "x": 120, "y": 110}], "mode": "top", "by": "anchor", "to": 4,+ "spacing": 2, "source": "live", "dryRun": true, "reason": "line them up", "_fromFrame": "standard"});+ let kept = move_args_only(&align);+ for k in ["mode", "by", "to", "spacing", "source"] {+ assert!(kept.get(k).is_none(), "{k}");+ }+ for k in ["filePath", "refs", "dryRun", "reason", "_fromFrame"] {+ assert!(kept.get(k).is_some(), "{k}");+ }+ assert!(plan_moves(&b, &kept).is_ok());+ assert_eq!(plan_moves(&b, &align).unwrap_err().code, "unknown_argument");+ }++ #[test] fn courtyard_box_follows_rotation_and_falls_back_to_pads() { let b = board(&[fp("U1", 110.0, 110.0, 0.0, "F.Cu", RECT, ""), fp("U2", 120.0, 110.0, 90.0, "F.Cu", RECT, ""), fp("R1", 130.0, 110.0, 0.0, "F.Cu", "", "")]); let p = placements(&b);⋯ 546 unchanged lines ⋯ assert!(!outlines_overlap(&rect(0.0, 0.0, 1.0, 1.0), &rect(1.0, 0.0, 2.0, 1.0))); } }+@@ -1,1019 +1,1022 @@⋯ 775 unchanged lines ⋯ Ok(m) => m, Err(e) => return routing_fail(&e), };- let mut a = args.clone();+ // only the move's own arguments go on: align's mode/by/to would be unknown to it (wiki PR #70)+ let mut a = kicad_core::placement::move_args_only(args); a["refs"] = json!(moves); a.as_object_mut().map(|o| o.remove("ref")); let r = ipc::with_standard_revision(&c, &a, |x| ipc::move_footprint(&c, x));⋯ 27 unchanged lines ⋯ v["mutated"] = json!(false); return v; }- let mut a = args.clone();+ let mut a = kicad_core::placement::move_args_only(args); a["refs"] = json!(moves);+ a.as_object_mut().map(|o| o.remove("ref")); match kicad_core::placement::plan_moves(&loaded.board, &a) { Ok(plan) => { let mut v = json!({"success": true, "dryRun": true, "mutated": false, "source": "file", "boardPath": norm(&loaded.path), "revision": kicad_core::placement::file_revision(&loaded.text), "align": report,⋯ 199 unchanged lines ⋯ let _ = std::fs::remove_dir_all(&d); } }+@@ -1,292 +1,294 @@⋯ 75 unchanged lines ⋯ "unknown_action", "wrong_file_type", "file_exists", "footprint_exists", "ambiguous", "ambiguous_board", "already_there", "already_flagged", "needs_closed_board", "documents_in_use", "text_off_board", "live_required", "board_open", "kicad_running", "project_open", "too_close", "same_point", "zero_length", "plan_invalid", "invalid_path", "invalid_scope", "name_required", "engine_required", "foreground_required", "nothing_to_route", "nothing_in_box",- "nothing_placed", "side_change_not_supported", "live_move_drops_units", "entry_off_bus", "bad_via_geometry"];+ "nothing_placed", "side_change_not_supported", "live_move_drops_units", "entry_off_bus", "bad_via_geometry",+ "unknown_argument", "invalid_arguments"]; EXACT.contains(&code) || code.starts_with("missing_") || code.starts_with("no_such_") || code.ends_with("_not_found") || code.ends_with("_not_open") || code.ends_with("_required") }⋯ 199 unchanged lines ⋯ mod tests { #[test] fn a_callers_own_mistake_is_not_reported_as_a_bridge_bug() {- for c in ["bad_args", "missing_arg", "no_such_probe", "footprint_not_found", "pcb_editor_not_open", "reason_required", "drc_rejected", "no_model"] {+ for c in ["bad_args", "unknown_argument", "missing_arg", "no_such_probe", "footprint_not_found", "pcb_editor_not_open", "reason_required", "drc_rejected", "no_model"] { assert!(super::caller_side(c), "{c}"); } for c in ["internal", "handler_panicked", "write_failed", "simulation_failed", "worker_failed", "bad_layout", "ipc_unavailable"] {⋯ 1 unchanged line ⋯ } } }+@@ -1,210 +1,211 @@⋯ 27 unchanged lines ⋯ mechanism: Mechanism::Ipc, risk: "write", timeout_sec: 130, input: "{\"filePath\", \"expectedRevision\", \"ref\": \"U1\", \"x\": 120.5, \"y\": 91, \"rotation\"?: 90, \"side\"?: \"F.Cu\" | \"B.Cu\" (must equal the current side for now) | \"refs\": [{\"ref\",\"x\",\"y\",\"rotation\"?,\"side\"?}, ...], \"dryRun\"?: false, \"allowOverlap\"?: false, \"allowOutside\"?: false, \"allowLocked\"?: false, \"save\"?: false} frame?: \"standard\" takes y-up coordinates as kicad_routing_state {model:standard} reports them", example: "kicad_move_footprint {\"filePath\":\"C:/p/b.kicad_pcb\",\"expectedRevision\":\"<from placement_state>\",\"ref\":\"C12\",\"x\":132.4,\"y\":88.1,\"rotation\":90}",- hint: "Pass filePath, expectedRevision from placement_state, ref with x and y in mm (rotation in degrees), or refs for a batch that lands as one Undo step. Refuses stale_board, courtyard_overlap (names the offending refs), outside_outline and side_change_not_supported with nothing written; allowOverlap / allowOutside override on purpose. dryRun evaluates the checks and reports without editing. KiCad replaces the whole footprint on an IPC update and takes every child at the absolute coordinates in the proto, so the bridge transforms every pad, field, shape, text, zone and dimension of the proto client-side before sending it. The response carries before and after poses, the courtyard check, undoSteps:1, the new revision and verified, which is true only when the read-back courtyard (or first pad) moved with the anchor; after[].children carries the numbers.",+ hint: "Pass filePath, expectedRevision from placement_state, ref with x and y in mm (rotation in degrees), or refs for a batch that lands as one Undo step. Refuses stale_board, courtyard_overlap (names the offending refs), outside_outline, side_change_not_supported and unknown_argument (a key the move does not read, such as rot: suggestedKey names the right one, allowedKeys lists them) with nothing written; allowOverlap / allowOutside override on purpose. dryRun evaluates the checks and reports without editing. KiCad replaces the whole footprint on an IPC update and takes every child at the absolute coordinates in the proto, so the bridge transforms every pad, field, shape, text, zone and dimension of the proto client-side before sending it. The response carries before and after poses, the courtyard check, undoSteps:1, the new revision and verified, which is true only when the read-back courtyard (or first pad) moved with the anchor; after[].children carries the numbers.", related: &["kicad_placement_state", "kicad_placement_validate", "kicad_route_net"], pitfalls: &["Needs KiCad 10.0.1+ with the IPC API server on and the board open in the PCB editor; dryRun alone works from the file.", "A locked footprint is refused (footprint_locked) unless allowLocked:true.", "Do not blindly retry a timed-out move (mutation_outcome_unknown, ipc_timeout); read placement_state first.", "Moving a footprint leaves its copper where it was; place before routing.", "side changes are refused for now (side_change_not_supported): a flip mirrors every child and swaps its layers, which is separate work. Flip in the PCB editor (F), read placement_state again, then move with the same side or no side.", "The proto children are transformed client-side because KiCad replaces the footprint on update (FOOTPRINT::Deserialize keeps the children's absolute coordinates); a child type without a transform rule refuses the move (unsupported_child_item names the type URL), and a text box or rounded courtyard rectangle only turns by multiples of 90 degrees.", "verified:false with postCommitError 'children did not move' means the anchor moved and the body did not; undo in KiCad and report it, do not move again."], },⋯ 173 unchanged lines ⋯ } } }+@@ -1,27 +1,28 @@⋯ 2 unchanged lines ⋯ members = ["crates/*"] [workspace.package]-version = "1.0.106"+version = "1.0.107" edition = "2021" license = "MIT" publish = false⋯ 15 unchanged lines ⋯ codegen-units = 1 strip = true panic = "abort"+@@ -1,1307 +1,1308 @@⋯ 318 unchanged lines ⋯ [[package]] name = "kicad-bridge"-version = "1.0.106"+version = "1.0.107" dependencies = [ "adom-eda-checks", "kicad-core",⋯ 5 unchanged lines ⋯ [[package]] name = "kicad-core"-version = "1.0.106"+version = "1.0.107" dependencies = [ "kicad-ipc-rs", "kicad-platform",⋯ 20 unchanged lines ⋯ [[package]] name = "kicad-platform"-version = "1.0.106"+version = "1.0.107" dependencies = [ "serde", "serde_json",⋯ 940 unchanged lines ⋯ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b"+
Comments
Log in to comment.
No comments yet.