← Commit history

1.0.51: 1.0.51 (wiki #115): kicad_add_zone waits for KiCad's asynchronous zone fill before answering. It used to read the board revision straight after requesting the refill, so it returned the pre-fill revision; the fill then changed the board and the caller's next edit was refused as stale_board (intermittent, seen on arav-rog). It now waits until the revision has moved and holds still (bounded at 12 s, busy reads retried) and answers revisionSettled; if the fill never settles it says so and points at kicad_zone_state instead of handing out a revision it knows may be stale.

John Lauer ·deb509f9e0 ·1mo ago ·parent f0d79f0
3 files changed +68−5
rust/Cargo.lock+3−3
@@ -244,7 +244,7 @@ dependencies = [  [[package]] name = "kicad-bridge"-version = "1.0.50"+version = "1.0.51" dependencies = [  "kicad-core",  "kicad-platform",@@ -255,7 +255,7 @@ dependencies = [  [[package]] name = "kicad-core"-version = "1.0.50"+version = "1.0.51" dependencies = [  "kicad-ipc-rs",  "kicad-platform",@@ -281,7 +281,7 @@ dependencies = [  [[package]] name = "kicad-platform"-version = "1.0.50"+version = "1.0.51" dependencies = [  "serde",  "serde_json",
rust/Cargo.toml+1−1
@@ -3,7 +3,7 @@ resolver = "2" members = ["crates/*"]  [workspace.package]-version = "1.0.50"+version = "1.0.51" edition = "2021" license = "MIT" publish = false
rust/crates/kicad-core/src/ipc.rs+64−1
@@ -888,12 +888,26 @@ pub fn add_zone(ctx: &Ctx, args: &Value) -> RResult<Value> {     }     // KiCad 10.0.x answers AS_UNIMPLEMENTED to RefillZones with a list of zone ids; an empty     // list means "every zone" and works, so that is what the bridge sends.+    let mut settled = true;     if args.get("refill") != Some(&json!(false)) && !ids.is_empty() {+        let before = s.revision().ok();         match s.client.refill_zones(Vec::new()) {-            Ok(()) => result["refilled"] = json!(true),+            Ok(()) => {+                // Wiki #115: KiCad fills ASYNCHRONOUSLY. Reading the revision straight after the+                // request returned the pre-fill board, the fill then changed it, and the caller's+                // next edit was refused as stale_board. Wait for the fill to land and the revision+                // to hold still, bounded; say so honestly if it never does.+                let (_, ok) = settle_revision(before.as_deref(), || s.revision().map_err(|e| e.message));+                settled = ok;+                result["refilled"] = json!(ok);+            }             Err(e) => result["postCommitError"] = json!(format!("zones created but not filled: {e}")),         }     }+    result["revisionSettled"] = json!(settled);+    if !settled {+        result["_hint"] = json!("Zones committed as one Undo step, but KiCad was still filling when this reply was written, so the revision below may already be stale. Call kicad_zone_state (refill:false) for a settled revision before the next edit; a stale one is refused with stale_board and changes nothing.");+    }     match s.revision() {         Ok(r) => {             result["revision"] = json!(r);@@ -1095,6 +1109,37 @@ pub fn remove_zone(ctx: &Ctx, args: &Value) -> RResult<Value> {     Ok(result) } +/// Wait for an asynchronous zone fill to land: the revision must first move away from `before`+/// (a new zone always changes the board when it fills), then read identically three times in a+/// row. A fill that changes nothing is accepted once the revision has stayed at `before` for 3 s.+/// Read errors (KiCad busy mid-fill) count as "not yet". Bounded at 12 s; returns+/// (last revision, settled).+pub fn settle_revision(before: Option<&str>, mut read: impl FnMut() -> Result<String, String>) -> (Option<String>, bool) {+    settle_revision_with(before, &mut read, std::time::Duration::from_millis(300), std::time::Duration::from_secs(3), std::time::Duration::from_secs(12))+}++fn settle_revision_with(before: Option<&str>, read: &mut dyn FnMut() -> Result<String, String>, step: std::time::Duration, quiet: std::time::Duration, limit: std::time::Duration) -> (Option<String>, bool) {+    let start = std::time::Instant::now();+    let (mut last, mut same, mut moved): (Option<String>, u32, bool) = (None, 0, false);+    loop {+        if let Ok(r) = read() {+            moved |= before != Some(r.as_str());+            if last.as_deref() == Some(r.as_str()) { same += 1 } else { same = 0 }+            last = Some(r);+            if moved && same >= 2 {+                return (last, true);+            }+            if !moved && same >= 2 && start.elapsed() >= quiet {+                return (last, true);+            }+        }+        if start.elapsed() >= limit {+            return (last, false);+        }+        std::thread::sleep(step);+    }+}+ fn refill_requested(board_path: &str) -> Value {     json!({         "success": true, "boardPath": board_path, "source": "live-editor",@@ -1353,6 +1398,24 @@ fn merge(into: &mut Value, from: &Value) {  #[cfg(test)] mod tests {+    #[test]+    fn a_fill_that_lands_late_is_waited_for_and_a_stuck_one_is_reported() {+        use std::time::Duration;+        let ms = Duration::from_millis;+        // the pre-fill revision twice, a busy read, then the filled board that holds still+        let mut seq = vec![Ok("A".to_string()), Ok("A".to_string()), Err("busy".to_string()),+                           Ok("B".to_string()), Ok("B".to_string()), Ok("B".to_string())].into_iter();+        let (r, ok) = settle_revision_with(Some("A"), &mut || seq.next().unwrap_or(Ok("B".into())), ms(1), ms(50), ms(500));+        assert_eq!((r.as_deref(), ok), (Some("B"), true)); // never the stale "A"+        // a fill that changes nothing: accepted once quiet long enough+        let (r, ok) = settle_revision_with(Some("A"), &mut || Ok("A".to_string()), ms(1), ms(20), ms(500));+        assert_eq!((r.as_deref(), ok), (Some("A"), true));+        // a board that never stops changing: reported unsettled at the limit+        let mut n = 0;+        let (_, ok) = settle_revision_with(Some("A"), &mut || { n += 1; Ok(format!("R{n}")) }, ms(1), ms(20), ms(60));+        assert!(!ok);+    }+     #[test]     fn refill_acknowledgement_never_certifies_a_stale_snapshot() {         let reply = super::refill_requested("board.kicad_pcb");