← Commit history

Merge PR #168: Native reference/value field geometry by nested UUID, without replacing footprints (astra/native-silk-fields -> master) merge

John Lauer ·6984b95e5b ·18d ago ·parents e6e8803, 08d1515

Showing changes introduced by the merge (against its first parent).

7 files changed +488−1
docs/native-silk-fields.mdadded+16
@@ -0,0 +1,16 @@+# Native reference/value field geometry++`kicad_silk_fields_batch {filePath, expectedRevision, update:[{id,x,y,layer,height,stroke,...}], dryRun?, save?}` updates existing footprint Reference/Value properties by their field UUID. Read that UUID from the current native board snapshot. `x,y,rotation` are board-frame mm/degrees on both faces. `width` defaults to height; `align` is left/center/right; bottom silk defaults to mirrored; `visible` defaults true. Only plain native stroke-font fields and F.SilkS/B.SilkS destinations are supported. Locked footprints/fields, foreign IDs, duplicate IDs, content changes and unsupported styles refuse.++This sends nested Field updates through IPC, never a replacement footprint. It preserves reference/value content and field identity. The candidate converts position into footprint-local file coordinates for native DRC; it retains the footprint, pads and model definitions. All DRC severities participate before one Undo transaction. Native field UUID replies are checked before commit; complete field protobuf readback and board data outside the selected properties are checked after commit. Post-commit mismatches explicitly fail without replay. The native bounds and revision are returned; save occurs only after successful readback.++**Source validation is not native acceptance.** The pinned SDK supports Field payloads and GetItemsById/UpdateItems, but the exact nested-field path must be verified in the running KiCad. If KiCad cannot query an exact field UUID, this refuses; do not replace the whole footprint as a fallback or edit the saved file and claim a live update.++Owner's native acceptance before publication:++- Query and update reference and value fields on top and bottom footprints, including 90/180/270-degree rotations. Verify API board coordinates against saved local coordinates and the visible text; confirm mirroring, justification, size, stroke and visibility.+- Preserve both strings, field UUIDs, footprint UUID, pads, nets, all model paths/transforms and other board data. Complete protobuf equality is intentionally strict: investigate any native normalization before loosening it.+- Batch two fields, inspect bounds and the 2D/3D result, then one Undo restores both. Test inherited collisions versus a newly introduced mask/silk collision, stale revision, locked/foreign UUIDs and no whole-footprint fallback.+- Inspect post-commit errors without blindly replaying an edit. Refresh the exact linked views if necessary, then visually verify; a refresh acknowledgement does not prove rendering completed.++This is a contribution for #104's field-editing portion only. Exact mask/body obstacles, native model editing and reference camera framing remain separate features.
rust/crates/kicad-bridge/src/verbs_routing.rs+13
@@ -17,6 +17,16 @@ use kicad_core::pcb::{self, NetRef}; use kicad_core::{dsn, freerouting, progress};  pub static VERBS: &[Verb] = &[+    Verb {+        name: "kicad_silk_fields_batch",+        summary: "Update native footprint reference/value field geometry by stable field UUID.",+        mechanism: Mechanism::Ipc, risk: "write", timeout_sec: 130,+        input: "{filePath,expectedRevision,update:[{id,x,y,rotation?,layer,height,width?,stroke,align?,mirrored?,visible?}],dryRun?,save?}",+        example: "kicad_silk_fields_batch {filePath,expectedRevision,update:[{id,x:10,y:20,layer:F.SilkS,height:0.8,stroke:0.12}]}",+        hint: "Board-frame mm on both faces. Preserves reference/value content, uses nested field updates, never replaces the whole footprint. Full candidate DRC includes warnings. One Undo, then field and unrelated-board-data readback; inspect postCommit errors before retrying.",+        related: &["kicad_text_bounds", "kicad_silk_text_batch", "kicad_refresh_board_views"],+        pitfalls: &["Only plain stroke-font Reference/Value properties. No field creation/removal or content changes. Refuses if native field IDs cannot be queried. Exact masks and 3D body obstacles remain separate."],+    },     Verb {         name: "kicad_silk_graphics_batch",         summary: "Create/update/remove native silkscreen lines, borders, arcs and Bezier leaders by stable ID.",@@ -210,6 +220,7 @@ pub static VERBS: &[Verb] = &[  pub fn dispatch(state: &mut State, command: &str, args: &Value) -> Option<Value> {     Some(match command {+        "kicad_silk_fields_batch" => live(state,args,Live::SilkFieldsBatch),         "kicad_silk_graphics_batch" => live(state,args,Live::SilkGraphicsBatch),         "kicad_silk_text_batch" => live(state,args,Live::SilkTextBatch),         "kicad_routing_state" => live(state, args, Live::State),@@ -239,6 +250,7 @@ pub fn dispatch(state: &mut State, command: &str, args: &Value) -> Option<Value>  #[derive(Clone, Copy)] enum Live {+    SilkFieldsBatch,     SilkGraphicsBatch,     SilkTextBatch,     State,@@ -263,6 +275,7 @@ fn live(state: &mut State, args: &Value, which: Live) -> Value {         config_dir: info.primary().and_then(|p| detect::config_dir(&p.version)),     };     let result = match which {+        Live::SilkFieldsBatch => ipc::silk_fields_batch(&ctx,args),         Live::SilkGraphicsBatch => ipc::silk_graphics_batch(&ctx,args),         Live::State => ipc::routing_state(&ctx, args),         Live::TextBounds => ipc::text_bounds(&ctx,args),
rust/crates/kicad-core/src/ipc.rs+3
@@ -1930,3 +1930,6 @@ pub fn silk_text_batch(ctx: &Ctx, args: &Value) -> RResult<Value> {     Ok(result) } ++mod silk_fields;+pub use silk_fields::silk_fields_batch;
rust/crates/kicad-core/src/ipc/silk_fields.rsadded+255
@@ -0,0 +1,255 @@+use super::*;+use crate::silk_fields as fields;+use kicad_ipc_rs::FieldItem;++fn id(item: &EditablePcbItem) -> Option<&str> {+    match item {+        EditablePcbItem::Field(f) => f+            .proto()+            .text+            .as_ref()?+            .id+            .as_ref()+            .map(|x| x.value.as_str()),+        _ => None,+    }+}+fn apply_geometry(mut field: FieldItem, e: &fields::Edit) -> RResult<EditablePcbItem> {+    let f = field.proto_mut();+    if f.name != e.name {+        return Err(RoutingError::new(+            "field_identity_mismatch",+            "Native field name does not match saved reference/value property",+        ));+    }+    f.visible = e.visible;+    let b = f+        .text+        .as_mut()+        .ok_or_else(|| RoutingError::new("field_unavailable", "Native field has no board text"))?;+    if b.locked == 2 || b.knockout {+        return Err(RoutingError::new(+            "field_locked_or_knockout",+            "Locked/knockout fields are not supported",+        ));+    }+    b.layer = BoardLayerInfo::id_from_name(&e.text.layer).unwrap();+    let t = b+        .text+        .as_mut()+        .ok_or_else(|| RoutingError::new("field_unavailable", "Native field has no text"))?;+    if t.text != e.text.text {+        return Err(RoutingError::new(+            "field_identity_mismatch",+            "Native field text differs from the candidate",+        ));+    }+    let p = t.position.get_or_insert_with(Default::default);+    p.x_nm = nm(e.text.x);+    p.y_nm = nm(e.text.y);+    let a = t.attributes.get_or_insert_with(Default::default);+    if a.bold+        || a.italic+        || a.underlined+        || (!a.font_name.is_empty() && a.font_name != "KiCad Font")+    {+        return Err(RoutingError::new(+            "field_style_unsupported",+            "Only plain native stroke-font fields are supported",+        ));+    }+    a.horizontal_alignment = match e.text.align.as_str() {+        "left" => 1,+        "right" => 3,+        _ => 2,+    };+    a.vertical_alignment = 2;+    a.angle.get_or_insert_with(Default::default).value_degrees = e.text.rotation;+    a.line_spacing = 1.;+    a.stroke_width.get_or_insert_with(Default::default).value_nm = nm(e.text.stroke);+    let size = a.size.get_or_insert_with(Default::default);+    size.x_nm = nm(e.text.width);+    size.y_nm = nm(e.text.height);+    a.mirrored = e.text.mirrored;+    a.multiline = t.text.contains('\n');+    Ok(EditablePcbItem::Field(field))+}++pub fn silk_fields_batch(ctx: &Ctx, args: &Value) -> RResult<Value> {+    if args["expectedRevision"]+        .as_str()+        .filter(|s| !s.is_empty())+        .is_none()+    {+        return Err(RoutingError::new(+            "revision_required",+            "Read the board revision before editing fields",+        ));+    }+    let s = connect(ctx, args)?;+    let (text, _, revision) = s.snapshot()?;+    check_revision(args, &revision)?;+    let plan = fields::prepare(&text, args)?;+    let ids: Vec<_> = plan.edits.iter().map(|e| e.text.id.clone()).collect();+    let native = s+        .client+        .get_editable_items_by_id(ids.clone())+        .map_err(map_err)?;+    let mut updates = Vec::new();+    for e in &plan.edits {+        let f = native+            .iter()+            .find(|f| id(f) == Some(e.text.id.as_str()))+            .ok_or_else(|| {+                RoutingError::new(+                    "native_field_unavailable",+                    "KiCad did not return the exact nested field UUID; no whole-footprint fallback",+                )+            })?;+        let EditablePcbItem::Field(f) = f else {+            unreachable!()+        };+        updates.push(apply_geometry(f.clone(), e)?);+    }+    let before = drc_snapshot(ctx.kicad_cli.as_deref(), &s.source_path(), &text)?;+    let after = drc_snapshot(ctx.kicad_cli.as_deref(), &s.source_path(), &plan.candidate)?;+    let all = |mut v: Value| {+        if let Some(a) = v["violations"].as_array_mut() {+            for x in a {+                x["severity"] = json!("error");+            }+        }+        v+    };+    let added = new_errors(&all(before), &all(after.clone()))?;+    if !added.is_empty() {+        return Err(+            RoutingError::new("drc_rejected", "Field geometry adds native DRC findings")+                .with("violations", json!(added))+                .with("mutated", json!(false)),+        );+    }+    if args["dryRun"] == true {+        return Ok(+            json!({"success":true,"mutated":false,"revision":revision,"drc":after,"updatedIds":ids}),+        );+    }+    check_revision(args, &s.revision()?)?;+    transaction(&s, "Adom reference/value fields", || {+        let reply = s.client.update_editable_items(updates.clone())?;+        let actual: HashSet<_> = reply.iter().filter_map(id).collect();+        if actual != ids.iter().map(String::as_str).collect() {+            return Err(IpcFailure::from(+                "Native field update did not preserve all field UUIDs".to_string(),+            ));+        }+        Ok(())+    })?;+    let mut out = json!({"success":true,"committed":true,"mutated":true,"undoSteps":1,"updatedIds":ids,"footprintIds":plan.edits.iter().map(|e|&e.footprint_id).collect::<Vec<_>>(),"saved":false,"drc":after,"_hint":"Only nested reference/value fields were updated; no whole footprint replacement. Inspect postCommit errors before any retry, then refresh the exact linked views and inspect the board."});+    match s.client.get_editable_items_by_id(ids.clone()) {+        Ok(actual) => {+            let matches = updates.iter().all(|want| {+                actual.iter().any(|got| {+                    id(got) == id(want) && got.clone().into_any() == want.clone().into_any()+                })+            });+            out["fieldsVerified"] = json!(matches);+            if !matches {+                out["success"] = json!(false);+                out["postCommitFieldError"]=json!("Native field readback differs from requested geometry or preserved content; inspect before retrying");+            }+        }+        Err(e) => {+            out["success"] = json!(false);+            out["postCommitFieldError"] = json!(e.to_string());+        }+    }+    match s.snapshot() {+        Ok((current, _, rev)) => {+            out["revision"] = json!(rev);+            let set = ids.iter().cloned().collect();+            let preserved = match (+                fields::preserved_geometry(&text, &set),+                fields::preserved_geometry(&current, &set),+            ) {+                (Ok(before), Ok(after)) => before == after,+                _ => false,+            };+            out["otherBoardDataVerified"] = json!(preserved);+            if !preserved {+                out["success"] = json!(false);+                out["postCommitPreservationError"]=json!("Board data outside the requested fields changed; this may also indicate concurrent editing");+            }+        }+        Err(e) => {+            out["success"] = json!(false);+            out["postCommitReadbackError"] = json!(e.message);+        }+    }+    match s.client.get_item_bounding_boxes(ids,false){+        Ok(rows)=>out["bounds"]=json!(rows.iter().map(|b|json!({"id":b.item_id,"boxMm":[b.x_nm as f64/1e6,b.y_nm as f64/1e6,(b.x_nm+b.width_nm) as f64/1e6,(b.y_nm+b.height_nm) as f64/1e6]})).collect::<Vec<_>>()),+        Err(e)=>{out["success"]=json!(false);out["postCommitBoundsError"]=json!(e.to_string());}+    }+    if args["save"] == true && out["success"] == true {+        match s.client.save_document() {+            Ok(()) => out["saved"] = json!(true),+            Err(e) => {+                out["success"] = json!(false);+                out["postCommitSaveError"] = json!(e.to_string());+            }+        }+    }+    Ok(out)+}++#[cfg(test)]+mod tests {+    use super::*;+    #[test]+    fn nested_field_uuid_and_content_survive_geometry_update() {+        let mut f = FieldItem::from_proto(Default::default());+        let p = f.proto_mut();+        p.name = "Reference".into();+        p.id = Some(Default::default());+        p.visible = true;+        let b = p.text.get_or_insert_with(Default::default);+        b.id.get_or_insert_with(Default::default).value = "field-id".into();+        b.locked = 1;+        let text = b.text.get_or_insert_with(Default::default);+        text.text = "R1".into();+        text.hyperlink = "preserve-me".into();+        let e=fields::Edit{text:crate::silk_text::Text::parse(&json!({"text":"R1","layer":"B.SilkS","x":12.,"y":19.,"rotation":90.,"height":0.7,"width":0.6,"stroke":0.1,"align":"right"}),"field-id".into()).unwrap(),visible:false,footprint_id:"parent".into(),name:"Reference".into()};+        let result = apply_geometry(f, &e).unwrap();+        assert_eq!(id(&result), Some("field-id"));+        let EditablePcbItem::Field(field) = result else {+            panic!("not a field")+        };+        let p = field.proto();+        assert_eq!(p.name, "Reference");+        assert!(!p.visible);+        assert_eq!(p.id.as_ref().unwrap().id, 0);+        let b = p.text.as_ref().unwrap();+        let t = b.text.as_ref().unwrap();+        assert_eq!(t.text, "R1");+        assert_eq!(t.hyperlink, "preserve-me");+        assert_eq!(t.position.as_ref().unwrap().x_nm, 12_000_000);+        assert_eq!(b.layer, BoardLayerInfo::id_from_name("B.SilkS").unwrap());+        let a = t.attributes.as_ref().unwrap();+        assert_eq!(a.horizontal_alignment, 3);+        assert!(a.mirrored);+        assert_eq!(a.size.as_ref().unwrap().y_nm, 700_000);+        let mut bold = field.clone();+        bold.proto_mut()+            .text+            .as_mut()+            .unwrap()+            .text+            .as_mut()+            .unwrap()+            .attributes+            .as_mut()+            .unwrap()+            .bold = true;+        assert!(apply_geometry(bold, &e).is_err());+    }+}
rust/crates/kicad-core/src/lib.rs+2
@@ -34,3 +34,5 @@ pub mod zone_plan;   pub mod silk_graphics;++pub mod silk_fields;
rust/crates/kicad-core/src/silk_fields.rsadded+198
@@ -0,0 +1,198 @@+//! Reference/value field geometry, with file-local coordinates kept out of the API.+use crate::schematic::{parse_node, Document, Kind, Node};+use crate::{pcb::RoutingError, silk_text::Text};+use serde_json::{json, Value};+use std::collections::HashSet;+fn fail(s: impl Into<String>) -> RoutingError {+    RoutingError::new("silk_field_refused", s).with("mutated", json!(false))+}+pub struct Edit {+    pub text: Text,+    pub visible: bool,+    pub footprint_id: String,+    pub name: String,+}+pub struct Plan {+    pub candidate: String,+    pub edits: Vec<Edit>,+}+fn locked(n: &Node) -> bool {+    n.child("locked")+        .map(|x| x.arg(0).as_deref() != Some("no"))+        .unwrap_or(false)+}+pub fn prepare(source: &str, args: &Value) -> Result<Plan, RoutingError> {+    let rows = args["update"]+        .as_array()+        .filter(|a| !a.is_empty() && a.len() <= 2000)+        .ok_or_else(|| fail("update requires 1..2000 complete geometry definitions"))?;+    for k in ["create", "remove"] {+        if args.get(k).is_some() {+            return Err(fail(+                "fields are updated, never created or removed by this verb",+            ));+        }+    }+    let mut doc = Document::parse(source).map_err(fail)?;+    let mut edits = Vec::new();+    let mut seen = HashSet::new();+    for row in rows {+        let id = row["id"]+            .as_str()+            .ok_or_else(|| fail("field UUID is required"))?;+        if !seen.insert(id.to_string()) {+            return Err(fail("duplicate field UUID"));+        }+        if row.get("text").is_some() {+            return Err(fail(+                "reference/value content is preserved; text is not editable here",+            ));+        }+        let visible = row+            .get("visible")+            .map(Value::as_bool)+            .unwrap_or(Some(true))+            .ok_or_else(|| fail("visible must be boolean"))?;+        let fp = doc+            .root+            .children_mut()+            .unwrap()+            .iter_mut()+            .find(|fp| {+                fp.head() == Some("footprint")+                    && fp.children().iter().any(|p| {+                        p.head() == Some("property") && p.child_value("uuid").as_deref() == Some(id)+                    })+            })+            .ok_or_else(|| fail("UUID is not a footprint reference/value property"))?;+        if locked(fp) {+            return Err(fail("footprint is locked"));+        }+        let fp_id = fp+            .child_value("uuid")+            .ok_or_else(|| fail("footprint has no UUID"))?;+        let at = fp+            .child("at")+            .ok_or_else(|| fail("footprint has no pose"))?;+        let number = |i| at.arg(i).and_then(|x| x.parse::<f64>().ok());+        let (x, y, rot) = (+            number(0).ok_or_else(|| fail("invalid footprint x"))?,+            number(1).ok_or_else(|| fail("invalid footprint y"))?,+            number(2).unwrap_or(0.),+        );+        let p = fp+            .children_mut()+            .unwrap()+            .iter_mut()+            .find(|p| p.head() == Some("property") && p.child_value("uuid").as_deref() == Some(id))+            .unwrap();+        let name = p.arg(0).unwrap_or_default();+        if !matches!(name.as_str(), "Reference" | "Value") || locked(p) {+            return Err(fail("only unlocked Reference/Value fields are supported"));+        }+        let original = p.arg(1).ok_or_else(|| fail("field has no text"))?;+        let mut v = row.clone();+        v.as_object_mut()+            .ok_or_else(|| fail("update row must be an object"))?+            .remove("visible");+        v["text"] = json!(original);+        let t = Text::parse(&v, id.into())?;+        let local = crate::placement::to_board((0., 0.), -rot, (t.x - x, t.y - y));+        let node = t.node();+        let children = p.children_mut().unwrap();+        children.retain(|n| !matches!(n.head(), Some("at" | "layer" | "effects" | "hide")));+        children.push(parse_node(&format!("(at {} {} {})", local.0, local.1, t.rotation)).unwrap());+        children.push(node.child("layer").unwrap().clone());+        children.push(node.child("effects").unwrap().clone());+        if !visible {+            children.push(parse_node("(hide yes)").unwrap());+        }+        edits.push(Edit {+            text: t,+            visible,+            footprint_id: fp_id,+            name,+        });+    }+    Ok(Plan {+        candidate: doc.to_text(),+        edits,+    })+}+/// Ignore only the explicitly edited property nodes, never pads, model data or other text.+pub fn preserved_geometry(source: &str, ids: &HashSet<String>) -> Result<Value, RoutingError> {+    fn visit(n: &Node, ids: &HashSet<String>) -> Value {+        match &n.kind {+            Kind::Atom(a) => json!(a),+            Kind::List { children, .. } => Value::Array(+                children+                    .iter()+                    .filter(|c| {+                        !(c.head() == Some("property")+                            && c.child_value("uuid")+                                .map(|id| ids.contains(&id))+                                .unwrap_or(false))+                    })+                    .map(|c| visit(c, ids))+                    .collect(),+            ),+        }+    }+    Ok(visit(&Document::parse(source).map_err(fail)?.root, ids))+}+#[cfg(test)]+mod tests {+    use super::*;+    fn board(side: &str, angle: i32) -> String {+        format!(+            r#"(kicad_pcb+          (footprint "P" (uuid fp) (layer "{side}") (at 10 20 {angle})+            (property "Reference" "R1" (at 0 0) (layer "F.SilkS") (uuid field) (effects (font (size 1 1))))+            (property "Value" "10k" (at 0 1) (layer "F.Fab") (uuid value))+            (pad "1" smd rect (at 1 2) (size 1 1) (layers "F.Cu" "F.Mask") (uuid pad))+            (model "chosen.step" (offset (xyz 0 0 0)) (scale (xyz 1 1 1))))+          (gr_text "R1" (uuid other)))"#+        )+    }+    #[test]+    fn field_identity_and_rotated_both_face_coordinates() {+        for side in ["F.Cu", "B.Cu"] {+            let b = board(side, 90);+            let p=prepare(&b,&json!({"update":[{"id":"field","x":12,"y":19,"rotation":90,"layer":"B.SilkS","height":0.7,"stroke":0.1,"align":"right","visible":false}]})).unwrap();+            let d = Document::parse(&p.candidate).unwrap();+            let fp = d.root.child("footprint").unwrap();+            let f = fp+                .children()+                .iter()+                .find(|n| n.head() == Some("property"))+                .unwrap();+            let at = f.child("at").unwrap();+            assert!((at.arg(0).unwrap().parse::<f64>().unwrap() - 1.).abs() < 1e-9);+            assert!((at.arg(1).unwrap().parse::<f64>().unwrap() - 2.).abs() < 1e-9);+            assert!(p.edits[0].text.mirrored);+            assert_eq!(f.arg(1).as_deref(), Some("R1"));+            assert_eq!(f.child_value("uuid").as_deref(), Some("field"));+            let ids = HashSet::from(["field".to_string()]);+            assert_eq!(+                preserved_geometry(&b, &ids).unwrap(),+                preserved_geometry(&p.candidate, &ids).unwrap()+            );+        }+    }+    #[test]+    fn refuses_content_changes_foreign_ids_and_duplicates() {+        let b = board("F.Cu", 0);+        let row = json!({"id":"field","x":1,"y":2,"layer":"F.SilkS","height":0.5,"stroke":0.1});+        assert!(prepare(&b, &json!({"update":[row.clone(),row.clone()]})).is_err());+        for (k, v) in [+            ("id", json!("other")),+            ("text", json!("R999")),+            ("layer", json!("F.Cu")),+            ("visible", json!("yes")),+        ] {+            let mut r = row.clone();+            r[k] = v;+            assert!(prepare(&b, &json!({"update":[r]})).is_err());+        }+    }+}
rust/crates/kicad-core/src/silk_text.rs+1−1
@@ -5,7 +5,7 @@ use crate::{pcb::RoutingError,schematic::{Document,Node,parse_node,new_uuid}}; fn fail(s:impl Into<String>)->RoutingError{RoutingError::new("silk_text_refused",s).with("mutated",json!(false))} #[derive(Clone,Debug)]pub struct Text {pub id:String,pub text:String,pub layer:String,pub x:f64,pub y:f64,pub rotation:f64,pub height:f64,pub width:f64,pub stroke:f64,pub align:String,pub mirrored:bool} impl Text {- fn parse(v:&Value,id:String)->Result<Self,RoutingError>{+ pub(crate) fn parse(v:&Value,id:String)->Result<Self,RoutingError>{   const KEYS:&[&str]=&["id","text","layer","x","y","rotation","height","width","stroke","align","mirrored"];   let o=v.as_object().ok_or_else(||fail("text row must be an object"))?;   if let Some(k)=o.keys().find(|k|!KEYS.contains(&k.as_str())){return Err(fail(format!("unsupported text field {k}; native stroke-font text only")));}