Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
171 changes: 164 additions & 7 deletions crates/konnect-core/src/tools/sch_batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,12 @@ pub fn tools() -> Vec<ToolDef> {
"x": { "type": "number", "description": "X position in mm" },
"y": { "type": "number", "description": "Y position in mm" },
"size": { "type": "number", "description": "Font size in mm", "default": 1.27 },
"rotation": { "type": "number", "description": "Rotation in degrees", "default": 0 }
"rotation": { "type": "number", "description": "Rotation in degrees", "default": 0 },
"justify": {
"type": "string",
"description": "Alignment of the text against x/y: at most one horizontal token (left, right) and one vertical token (top, bottom), space separated. An axis you leave out is centred - KiCad has no 'center' keyword and encodes centring by omission, so 'bottom' means horizontally centred and bottom-aligned. 'center' is shorthand for centring both axes. Defaults to 'left bottom', what KiCad itself writes for a placed annotation; a centred horizontal axis can carry a long line off the page.",
"default": "left bottom"
}
},
"required": ["schematic", "text", "x", "y"]
}),
Expand Down Expand Up @@ -1026,6 +1031,62 @@ async fn handle_connect_passthrough(
})))
}

/// KiCad's default alignment for a placed text annotation.
///
/// Measured against KiCad 10's own demo projects: every text block in the
/// current file format carries `(justify left bottom)`.
const DEFAULT_TEXT_JUSTIFY: &str = "left bottom";

/// Translate a caller's alignment into the `(justify ...)` clause KiCad writes.
///
/// Alignment is per axis, and centring an axis means leaving its token out:
/// `left` is left-aligned and vertically centred, `bottom` is horizontally
/// centred and bottom-aligned. `"center"` centres both and so returns an empty
/// string. There is no token for it — `(justify center)` makes KiCad refuse the
/// whole file, the same way a misplaced item does.
fn schematic_text_justify(value: &str) -> Result<String, String> {
fn claim(
slot: &mut Option<&'static str>,
value: &'static str,
token: &str,
) -> Result<(), String> {
if let Some(existing) = slot {
return Err(format!(
"justify names '{existing}' and '{token}' on the same axis - use at most one horizontal (left, right) and one vertical (top, bottom) token"
));
}
*slot = Some(value);
Ok(())
}

let trimmed = value.trim();
if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("center") {
return Ok(String::new());
}

let mut horizontal: Option<&'static str> = None;
let mut vertical: Option<&'static str> = None;
for token in trimmed.split_whitespace() {
match token.to_ascii_lowercase().as_str() {
"left" => claim(&mut horizontal, "left", token)?,
"right" => claim(&mut horizontal, "right", token)?,
"top" => claim(&mut vertical, "top", token)?,
"bottom" => claim(&mut vertical, "bottom", token)?,
_ => {
return Err(format!(
"unknown justify token '{token}' - use left, right, top, bottom, or center"
))
}
}
}

// KiCad writes the horizontal token first.
let mut parts = Vec::with_capacity(2);
parts.extend(horizontal);
parts.extend(vertical);
Ok(format!(" (justify {})", parts.join(" ")))
}

async fn handle_add_schematic_text(
args: &serde_json::Value,
_ctx: &crate::tools::ToolContext,
Expand All @@ -1045,6 +1106,14 @@ async fn handle_add_schematic_text(
};
let size = args["size"].as_f64().unwrap_or(1.27);
let rotation = args["rotation"].as_f64().unwrap_or(0.0);
let justify = args["justify"].as_str().unwrap_or(DEFAULT_TEXT_JUSTIFY);
// Without a justify token KiCad centres the text on `x`, so a long line
// crosses the page edge and is dropped from the PDF export while the
// .kicad_sch still looks complete.
let justify_sexp = match schematic_text_justify(justify) {
Ok(v) => v,
Err(e) => return Ok(CallToolResult::error(e)),
};
let uuid = new_uuid();

// Escape for a KiCad quoted string. Newlines and tabs must become their
Expand All @@ -1060,7 +1129,7 @@ async fn handle_add_schematic_text(

let text_sexp = format!(
"\n (text \"{escaped}\"\n (at {x} {y} {rotation})\n \
(effects (font (size {size} {size})))\n (uuid \"{uuid}\")\n )"
(effects (font (size {size} {size})){justify_sexp})\n (uuid \"{uuid}\")\n )"
);

let content = read_consistent(&sch_path)?;
Expand All @@ -1076,6 +1145,7 @@ async fn handle_add_schematic_text(
"x": x, "y": y,
"size": size,
"rotation": rotation,
"justify": justify,
"uuid": uuid
})))
}
Expand Down Expand Up @@ -2095,15 +2165,16 @@ mod multi_unit_field_tests {

#[cfg(test)]
mod add_text_placement_tests {
use super::tools;
use super::{schematic_text_justify, tools};
use crate::mcp::protocol::CallToolResult;
use crate::tools::ToolContext;
use serde_json::json;
use std::io::Write;
use std::sync::Arc;

const SCH: &str = "(kicad_sch\n\t(version 20260306)\n\t(generator \"eeschema\")\n\t(uuid \"root\")\n\t(lib_symbols\n\t\t(symbol \"Device:R\"\n\t\t\t(property \"Reference\" \"R\")\n\t\t)\n\t)\n\t(symbol\n\t\t(lib_id \"Device:R\")\n\t\t(at 100 80 0)\n\t\t(unit 1)\n\t\t(uuid \"u1\")\n\t\t(property \"Reference\" \"R1\"\n\t\t\t(at 100 75 0)\n\t\t)\n\t)\n\t(sheet_instances\n\t\t(path \"/\" (page \"1\"))\n\t)\n)\n";

async fn add_text(text: &str) -> String {
async fn add_text_inner(text: &str, justify: Option<&str>) -> (String, CallToolResult) {
let mut f = tempfile::NamedTempFile::with_suffix(".kicad_sch").unwrap();
f.write_all(SCH.as_bytes()).unwrap();
f.flush().unwrap();
Expand All @@ -2123,12 +2194,27 @@ mod add_text_placement_tests {
};
let router = Arc::new(crate::router::ToolRouter::new());
let ctx = Arc::new(ToolContext::new(cfg, router));
let args = json!({
let mut args = json!({
"schematic": f.path().to_str().unwrap(),
"text": text, "x": 30.0, "y": 114.3
});
(def.handler)(&args, ctx).await.unwrap();
std::fs::read_to_string(f.path()).unwrap()
if let Some(j) = justify {
args["justify"] = json!(j);
}
let result = (def.handler)(&args, ctx).await.unwrap();
(std::fs::read_to_string(f.path()).unwrap(), result)
}

async fn add_text(text: &str) -> String {
add_text_inner(text, None).await.0
}

async fn add_text_justified(text: &str, justify: &str) -> String {
add_text_inner(text, Some(justify)).await.0
}

async fn add_text_result(text: &str, justify: &str) -> CallToolResult {
add_text_inner(text, Some(justify)).await.1
}

/// The regression: the text was spliced in at the file's last `)`, which
Expand Down Expand Up @@ -2168,6 +2254,77 @@ mod add_text_placement_tests {
let out = add_text("a \"b\" c\\d\te").await;
assert!(out.contains(r#"(text "a \"b\" c\\d\te""#), "got:\n{out}");
}

/// The reported defect: with no `(justify ...)` KiCad centres the text on
/// `x`, so a long line crosses the left page edge and vanishes from the PDF
/// export while the .kicad_sch still reads as complete.
#[tokio::test]
async fn text_is_left_aligned_by_default() {
let out = add_text("hello").await;
assert!(
out.contains("(effects (font (size 1.27 1.27)) (justify left bottom))"),
"got:\n{out}"
);
}

#[tokio::test]
async fn justify_is_written_as_kicad_orders_it() {
let out = add_text_justified("hello", "top right").await;
assert!(
out.contains("(justify right top)"),
"horizontal token comes first, got:\n{out}"
);
}

/// KiCad has no `center` token: centred text is text with no justify at
/// all. This is the pre-change behaviour, still reachable on request.
#[tokio::test]
async fn center_writes_no_justify_token() {
let out = add_text_justified("hello", "center").await;
assert!(!out.contains("(justify"), "got:\n{out}");
assert!(
out.contains("(effects (font (size 1.27 1.27)))"),
"got:\n{out}"
);
}

#[tokio::test]
async fn an_unknown_token_is_refused() {
let result = add_text_result("hello", "middle").await;
assert!(
result.is_error,
"an unknown alignment must not be guessed at"
);
}

#[tokio::test]
async fn two_tokens_on_one_axis_are_refused() {
let result = add_text_result("hello", "left right").await;
assert!(result.is_error);
}

#[test]
fn justify_parsing_covers_the_shapes_kicad_writes() {
// The five forms present in KiCad 10's own demo projects.
for (input, expected) in [
("left bottom", " (justify left bottom)"),
("right bottom", " (justify right bottom)"),
("left", " (justify left)"),
// One axis alone: the other is centred by omission.
("bottom", " (justify bottom)"),
("left top", " (justify left top)"),
("right", " (justify right)"),
] {
assert_eq!(schematic_text_justify(input).unwrap(), expected, "{input}");
}
assert_eq!(schematic_text_justify("center").unwrap(), "");
assert_eq!(
schematic_text_justify(" BOTTOM Left ").unwrap(),
" (justify left bottom)"
);
assert!(schematic_text_justify("sideways").is_err());
assert!(schematic_text_justify("top bottom").is_err());
}
}

/// `add_schematic_text` was not the only handler splicing at the file's last
Expand Down
2 changes: 1 addition & 1 deletion tool-directory.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ Six tools, grouped into *discovery/routing* and *observability*.
| `batch_edit_schematic_components` | Apply field updates (Value, Footprint, custom properties) to multiple components in a single atomic write. |
| `batch_delete_schematic_components` | Delete multiple components by reference designator in a single atomic write. |
| `connect_passthrough` | Add a wire stub and matching net label at a point to route a signal through a region without drawing a full path. Direction defaults to `auto`. |
| `add_schematic_text` | Add a text annotation (non-net label) to the schematic at a given position. |
| `add_schematic_text` | Add a text annotation (non-net label) to the schematic at a given position. Aligns the text against that position with `justify`, per axis and defaulting to `left bottom` as KiCad does; an omitted axis is centred, and `center` centres both. |
| `get_schematic_layout` | Return a compact spatial summary of the schematic: component positions, bounding box, optionally wires and labels. |
| `validate_wire_connections` | Check all wire endpoints for floating ends not connected to a pin, label, or another wire. |
| `validate_component_connections` | Check that every non-passive pin has at least one wire or label connected. Reports unconnected pins. |
Expand Down
Loading