From 2385f5782b4dd9f6373cb8bfa0b4dda6bba465a1 Mon Sep 17 00:00:00 2001 From: DarkBiscuits Date: Mon, 9 Nov 2026 15:53:37 -0500 Subject: [PATCH] Accept G91.1 in post read-back and mill audits --- CHANGELOG.md | 4 + crates/post/README.md | 2 + crates/post/src/audit.rs | 6 ++ crates/post/src/audit_5ax.rs | 6 ++ crates/post/src/read.rs | 5 ++ crates/post/tests/arc_centre_mode.rs | 116 +++++++++++++++++++++++++++ 6 files changed, 139 insertions(+) create mode 100644 crates/post/tests/arc_centre_mode.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c2f14..7b14368 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Post read-back, mill audit and five-axis audit accept `G91.1` as an explicit declaration of incremental IJK arc centres. `G90.1` is refused with an explanation that absolute arc centres are not supported. Custom post headers can now declare `G91.1` without failing their own read-back (#144). + ## 0.10.0 — 2026-11-09 Router gaps from the processes plan: cutter sense on diameter tools, onion-skin depth passes, a deterministic sheet packer with nesting checks, and a LinuxCNC router dialect with a cited probe cycle and vacuum/dust auxiliaries. This output has not been proven on a machine. diff --git a/crates/post/README.md b/crates/post/README.md index 98d8f59..425a5b7 100644 --- a/crates/post/README.md +++ b/crates/post/README.md @@ -24,6 +24,8 @@ A Probe event with a contact pose and a feed is one `G38.2` block: the three axi ## Modal audit +`read_back`, `audit` and `audit_five` accept `G91.1` as an explicit declaration of incremental `I`/`J`/`K` arc centres, including in a custom post header or on a motion block. It does not change the `G90` positioning mode. `G90.1` is rejected with a specific diagnostic because absolute arc centres are not supported. Built-in post headers are unchanged. + `audit` reads posted mill G-code and returns the modal state a control would keep at each motion block: motion mode (`G0`–`G3`), plane (`G17`–`G19`), distance (`G90`/`G91`), units (`G20`/`G21`), feed mode (`G93`/`G94`), the active `F`, work offset (`G54`–`G59`), the tool in the spindle (`T` and `M6`, or the post's tool-change comment), tool-length offset (`G43`/`G49`), spindle or beam (`M3`/`M4`/`M5`, including a laser `S` word), coolant, and a modal rotary word when the post has `rotary`. A word these posts do not write is an error, so a typo in a template fails the replay. On a post with `probe`, `G38.2` or `G38.3` is not a motion: position and motion mode stay put, and `F` on that block updates the modal feed. On a post with `digital_out`, `M64` and `M65` are accepted when `P` equals `vacuum_pin` or `dust_pin`. Other posts still reject those codes. A `G2` or `G3` needs a known plane and a full centre: both in-plane letters (`I`/`J`, `I`/`K`, or `J`/`K`) or an `R`. A feed move needs a positive `F` already set. An inverse-time feed block must carry its own `F`. When line numbers are enabled, every numbered line follows the post's start and step, and each number stays at or below 1000000000. Each line must fit `max_line_length`. On a `decimal_point` post, dimensional words include a decimal point, and that includes rotary `A`, `B`, and `C`. A negative zero, scientific notation, and a word whose absolute value is above `1e9` are errors. diff --git a/crates/post/src/audit.rs b/crates/post/src/audit.rs index bdb2ce7..5b44dbb 100644 --- a/crates/post/src/audit.rs +++ b/crates/post/src/audit.rs @@ -6,6 +6,7 @@ //! //! A word this checker does not know is an error, so a typo in a template fails here. Geometry //! is also checked against [`crate::read_back`] so the two readers stay aligned. +//! `G91.1` confirms incremental IJK without changing positioning; `G90.1` is rejected. //! //! Lathe posts are rejected; lathe and mill-turn text is replayed by [`crate::audit_lathe`]. //! Five-axis posts are replayed by [`crate::audit_five`]. Work offset @@ -404,6 +405,11 @@ impl<'a> Parser<'a> { for word in words { match word.letter { + // IJK is already incremental; this declaration does not change G90/G91. + 'G' if word.value == 91.1 => {} + 'G' if word.value == 90.1 => { + return Err(fail(line_no, "G90.1 absolute arc centres are not supported; use G91.1 for incremental IJK")); + } 'G' if self.is_probe_word(word) => probe_words += 1, 'G' => { let code = g_or_m(line_no, word, 'G')?; diff --git a/crates/post/src/audit_5ax.rs b/crates/post/src/audit_5ax.rs index fb8f4e3..80154d1 100644 --- a/crates/post/src/audit_5ax.rs +++ b/crates/post/src/audit_5ax.rs @@ -4,6 +4,7 @@ //! work frame, TCP, tool-length offset, inverse time, rotary words, and `G53` machine retracts. //! `G53` is not a motion and does not change the modal position or the motion mode. Rotary words //! and inverse-time codes on that line still update, matching [`crate::read_back`]. +//! `G91.1` confirms incremental IJK without changing positioning; `G90.1` is rejected. //! //! An index block is rotary words with no `X` `Y` `Z`. It is not a motion. LinuxCNC leaves the //! work-frame and TCP hooks empty, so those flags stay off; the machine-mode transform and the @@ -898,6 +899,11 @@ fn parse_words(line_no: usize, words: &[Word]) -> Result { let mut saw_t = false; for word in words { match word.letter { + // IJK is already incremental; keep arc-centre mode separate from positioning. + 'G' if word.value == 91.1 => {} + 'G' if word.value == 90.1 => { + return Err(fail(line_no, "G90.1 absolute arc centres are not supported; use G91.1 for incremental IJK")); + } 'G' => parsed.g.push(g_or_m(line_no, word, 'G')?), 'M' => parsed.m.push(g_or_m(line_no, word, 'M')?), 'F' => { diff --git a/crates/post/src/read.rs b/crates/post/src/read.rs index 7f86883..3922c0f 100644 --- a/crates/post/src/read.rs +++ b/crates/post/src/read.rs @@ -1,6 +1,7 @@ //! Read the G-code dialect a post just wrote. //! //! The parser is modal and absolute. `I` `J` `K` are centre offsets from the start of the block. +//! `G91.1` explicitly confirms those offsets; `G90.1` absolute arc centres are not supported. //! A signed `R` is the radius. Comments, line numbers, `%` tape edges and `O` program numbers are //! skipped. `G4` is a dwell. A post with a rotary block also accepts its A/B word and the //! inverse-time codes (`G93` / `G94` on LinuxCNC). Those stay modal the way `X` `Y` `Z` do. @@ -229,6 +230,10 @@ impl<'a> Parser<'a> { let mut probe_count = 0u32; for (letter, value) in &words { match *letter { + // Arc-centre mode is independent of G90/G91 positioning. Match only the + // supported decimal code, without rounding other decimal G codes into it. + 'G' if *value == 91.1 => {} + 'G' if *value == 90.1 => return Err(invalid("G90.1 absolute arc centres are not supported; use G91.1 for incremental IJK")), 'G' if self.is_probe_code(*value) => probe_count += 1, 'G' => g_codes.push(g_number(*value)?), 'M' => saw_m = true, diff --git a/crates/post/tests/arc_centre_mode.rs b/crates/post/tests/arc_centre_mode.rs new file mode 100644 index 0000000..1ddaee7 --- /dev/null +++ b/crates/post/tests/arc_centre_mode.rs @@ -0,0 +1,116 @@ +//! Regression for #144: declaring incremental IJK must not change interpreted motion. + +#![allow(clippy::expect_used)] + +use campanion_ir::{ArcSense, ClBlock, ClMotion, ClPath, CompensationMode, Feeds, MoveRole, Plane, Pose, Program, Vec3}; +use campanion_post::{PostDef, PostOptions, ReadMove, audit, audit_five, compare, load_post, post_program, read_back}; + +#[allow(clippy::panic)] +fn test_post(id: &str) -> PostDef { + // Keep hand-written blocks readable; sequence numbering is unrelated to arc-centre mode. + let json = match id { + "mach3" => include_str!("../posts/mach3.json"), + "linuxcnc_5ax" => include_str!("../posts/linuxcnc_5ax.json"), + "linuxcnc_router" => include_str!("../posts/linuxcnc_router.json"), + _ => panic!("unknown test post"), + }; + load_post(&json.replace("\"enabled\": true", "\"enabled\": false")).expect("post") +} + +// Nonzero start and centre distinguish incremental IJK from absolute centres. The second +// arc omits G3 to exercise modal motion after an arc-centre declaration on the first arc. +const PROGRAM: &str = "G21 G90 G17\nG0 X12 Y20 Z0\nG3 X10 Y22 Z0 I-2 J0 F100\nX8 Y20 Z1 I0 J-2\n"; + +#[test] +fn incremental_centres_preserve_arcs_and_helix_in_all_readers() { + for id in ["mach3", "linuxcnc_5ax"] { + let post = test_post(id); + let baseline = audit(PROGRAM, &post).expect("baseline audit"); + for text in [PROGRAM.replace("G90", "G90 G91.1"), PROGRAM.replace("G3 X", "G91.1 G3 X"), PROGRAM.replace("G90", "G90 G091.100")] { + assert_eq!(audit(&text, &post).expect("incremental audit"), baseline); + if id == "linuxcnc_5ax" { + assert_eq!(audit_five(&text, &post).expect("five-axis audit"), audit_five(PROGRAM, &post).expect("baseline five")); + } + let moves = read_back(&text, &post).expect("read-back"); + assert_eq!(moves.len(), 3); + let ReadMove::Arc { center, end, .. } = moves.get(1).expect("arc") else { panic!("expected arc") }; + assert_eq!(*center, [10.0, 20.0, 0.0]); + assert_eq!(*end, [10.0, 22.0, 0.0]); + let ReadMove::Arc { center, end, .. } = moves.get(2).expect("helix") else { panic!("expected helix") }; + assert_eq!(*center, [10.0, 20.0, 0.0]); + assert_eq!(*end, [8.0, 20.0, 1.0]); + } + // A declaration alone is not a motion, even after G3 has become modal. + let text = PROGRAM.replace("X8 Y20", "G91.1\nX8 Y20"); + assert_eq!(read_back(&text, &post).expect("standalone read-back").len(), 3); + assert_eq!(audit(&text, &post).expect("standalone audit").motions.len(), 3); + } +} + +#[test] +fn absolute_centres_and_other_unsupported_codes_are_rejected() { + for id in ["mach3", "linuxcnc_5ax"] { + let post = test_post(id); + for (code, message) in [ + ("G90.1", "G90.1 absolute arc centres are not supported; use G91.1 for incremental IJK"), + ("G91", "incremental motion is not supported"), + ("G91.2", "G code is not an integer"), + ("G91.1001", "G code is not an integer"), + ("G90.1001", "G code is not an integer"), + ("M91.1", "M code is not an integer"), + ] { + let text = PROGRAM.replace("G90", &format!("G90 {code}")); + // read_back deliberately skips M codes, while the auditors validate them. + if !code.starts_with('M') { + let error = read_back(&text, &post).expect_err("unsupported read-back").to_string(); + assert!(error.contains(message), "{id}: {error}"); + } + let error = audit(&text, &post).expect_err("unsupported audit").to_string(); + assert!(error.contains(message), "{id}: {error}"); + if id == "linuxcnc_5ax" { + let error = audit_five(&text, &post).expect_err("unsupported five-axis audit").to_string(); + assert!(error.contains(message), "{error}"); + } + } + for codes in ["G90.1 G91.1", "G91.1 G90.1"] { + let text = PROGRAM.replace("G90", codes); + assert!(read_back(&text, &post).is_err()); + assert!(audit(&text, &post).is_err()); + } + } +} + +#[test] +fn custom_mach3_header_posts_and_round_trips_a_full_circle() { + let json = include_str!("../posts/mach3.json").replace("\"G90\"", "\"G90\", \"G91.1\""); + let post = load_post(&json).expect("custom post"); + let pose = Pose::new(Vec3::new(12.0, 20.0, 0.0).expect("point"), Vec3::Z).expect("pose"); + let blocks = vec![ + ClBlock::Motion(ClMotion::Rapid { to: pose, feed: None }), + ClBlock::Motion(ClMotion::Arc { + to: pose, + plane: Plane::Xy, + center: Vec3::new(10.0, 20.0, 0.0).expect("centre"), + sense: ArcSense::Ccw, + feed: 100.0, + role: MoveRole::Cut, + }), + ]; + let feeds = Feeds::try_new(100.0, None, None, None, None).expect("feeds"); + let path = ClPath::try_new(None, None, CompensationMode::Off, feeds, None, Vec::new(), blocks).expect("path"); + let program = Program::try_new(None, Vec::new(), None, vec![path], Vec::new(), Vec::new(), None).expect("program"); + let text = post_program(&program, &post, &PostOptions { arc_tolerance: 0.01 }).expect("post"); + assert!(text.lines().any(|line| line == "G91.1"), "{text}"); + let moves = read_back(&text, &post).expect("read-back custom header"); + compare(&program, &moves, 0.01, &post).expect("full circle agrees with IR"); + assert_eq!(audit(&text, &post).expect("audit custom header").motions.len(), 2); +} + +#[test] +fn router_probe_support_survives_arc_centre_declaration() { + let post = test_post("linuxcnc_router"); + let baseline = PROGRAM.replace("G0 X12", "G38.2 Z-5 F50\nG0 X12"); + let text = baseline.replace("G90", "G90 G91.1"); + assert_eq!(audit(&text, &post).expect("probe audit"), audit(&baseline, &post).expect("baseline probe")); + assert_eq!(read_back(&text, &post).expect("probe read-back").len(), 3); +}