From 9460da513c41fca1dbef7b950b53f30dc48411b5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 13:58:40 -0400 Subject: [PATCH] fix(focas): decode all four bytes of an axis/spindle name record, up to the first NUL ReadNameRecord read only the first 2 bytes of each 4-byte 0x0089/0x008A record, so the real 31i-B's paired axes 'AA6'/'AA7' (live capture ref-axisname-0x0089.bin) both decoded as 'AA' -- the 'Makino with two AA axes' that #548 then had to disambiguate as AA_1/AA_2. The name is now the record's ASCII up to its FIRST NUL (not trailing-NUL trimming), so buffer residue such as Z-34184's spindle record 53 31 00 4e ('S1' + an 'N' left from an earlier 'CNC') is discarded. Client-visible: on such a machine Axes/AA_1 and Axes/AA_2 become Axes/AA6 and Axes/AA7 (the old names were a decode artefact). #548's disambiguation stays as the guard for genuinely duplicate names. Ported from the sister ignitionoee FOCAS driver (Focas.Wire-001, Focas.Wire-022 / v0.18.1). --- .../FixedTreeNames.cs | 17 ++-- .../Wire/FocasWireClient.cs | 12 ++- .../Wire/FocasWireProtocol.cs | 31 ++++++-- .../FocasNameRecordTests.cs | 78 +++++++++++++++++++ .../FocasWireProtocolTests.cs | 5 +- 5 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasNameRecordTests.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FixedTreeNames.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FixedTreeNames.cs index 2666310d..e614146e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FixedTreeNames.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FixedTreeNames.cs @@ -41,11 +41,18 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; /// config redeploy). /// /// -/// Suffix collisions with genuine names are structurally impossible for axes (FOCAS axis -/// displays are at most two characters — name + suffix — so they cannot contain _), -/// but the generator still checks: a generated segment that would collide with anything -/// else in the set escalates its ordinal until unique, so the output is always -/// collision-free whatever the input. +/// Suffix collisions with genuine names are not expected (a FOCAS name record is at most four +/// ASCII characters and controls do not use _), but the generator still checks: a +/// generated segment that would collide with anything else in the set escalates its ordinal +/// until unique, so the output is always collision-free whatever the input. +/// +/// +/// Note on the Makino that motivated this. Its two "AA" axes were a decode artefact: +/// the name records actually read "AA6" and "AA7", and the wire layer read only +/// the first two bytes of each 4-byte record. With the full record decoded +/// () that machine reports unique names and +/// its axes publish bare (Axes/AA6, Axes/AA7). This disambiguation remains as the +/// guard for a control that genuinely reports duplicates. /// /// internal static class FixedTreeNames diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs index e160386d..9b823200 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireClient.cs @@ -1114,7 +1114,17 @@ public sealed class FocasWireClient : IAsyncDisposable, IDisposable return alarms; } - private static IReadOnlyList ReadNameRecords(byte[] payload, short maxCount, Func factory) + /// + /// Decode an axis-name (0x0089) or spindle-name (0x008A) payload: an array of 4-byte records, + /// each named by . Blank records are skipped but + /// keep their slot's 1-based index. + /// + /// The record type the factory builds. + /// The response-block payload. + /// Maximum number of records to inspect. + /// Builds one record from its 1-based index and name. + /// The non-blank records, in wire order. + internal static IReadOnlyList ReadNameRecords(byte[] payload, short maxCount, Func factory) { var names = new List(); for (var offset = 0; offset + 4 <= payload.Length && offset / 4 < maxCount; offset += 4) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireProtocol.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireProtocol.cs index 35df33d3..2c6b394e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireProtocol.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireProtocol.cs @@ -239,19 +239,34 @@ internal static class FocasWireProtocol } /// - /// Read an axis/spindle name record — the first 2 bytes of a 2-byte (axis) or 4-byte - /// (spindle) slot. Trailing spaces and NULs are stripped so "X " becomes - /// "X". + /// Read one 4-byte axis/spindle name record: the ASCII up to (not including) the record's + /// FIRST NUL, at most 4 bytes, with trailing spaces trimmed — "X \0\0" → "X", + /// "AA6\0" → "AA6", "S1\0N" → "S1". /// - /// The bytes to decode. - /// The decoded name record. + /// + /// All four bytes are name bytes. This used to read only the first two, which + /// collapsed the real 31i-B's paired axes "AA6"/"AA7" (live capture + /// ref-axisname-0x0089.bin) to two axes both called "AA" — the duplicate Gitea + /// #548 then had to disambiguate as AA_1/AA_2. + /// The first NUL terminates, not trailing-NUL trimming. A control writes only the + /// bytes its name needs and leaves the rest of the buffer as it found it: Makino Z-34184's + /// cnc_rdspdlname answered 53 31 00 4e — "S1" plus the 'N' of an + /// earlier "CNC" response. Both rules are ported from the sister ignitionoee FOCAS + /// driver (Focas.Wire-001 / Focas.Wire-022). + /// + /// The record bytes (4 on the wire; a shorter span is read as far as it goes). + /// The decoded name; empty for a blank record. public static string ReadNameRecord(ReadOnlySpan bytes) { - if (bytes.Length < 2) return string.Empty; - var buffer = bytes.Slice(0, Math.Min(2, bytes.Length)).ToArray(); - return Encoding.ASCII.GetString(buffer).TrimEnd(' ', '\0'); + var record = bytes.Slice(0, Math.Min(NameRecordLength, bytes.Length)); + var nul = record.IndexOf((byte)0); + if (nul >= 0) record = record.Slice(0, nul); + return Encoding.ASCII.GetString(record).TrimEnd(' '); } + /// Byte length of one axis/spindle name record. + public const int NameRecordLength = 4; + private static byte[] BuildRequestBlock(RequestBlock request) { var extra = request.ExtraPayload ?? Array.Empty(); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasNameRecordTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasNameRecordTests.cs new file mode 100644 index 00000000..9e0a6be1 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasNameRecordTests.cs @@ -0,0 +1,78 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Wire; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Axis-name (0x0089) / spindle-name (0x008A) record decode. Each record is 4 bytes; the name is +/// the ASCII up to the record's FIRST NUL. Ported from the sister ignitionoee FOCAS driver +/// (Focas.Wire-001 and Focas.Wire-022, v0.18.1). +/// +/// +/// Reading only 2 bytes collides real names. The live 31i-B capture +/// (ref-axisname-0x0089.bin) carries "AA6" and "AA7"; the old 2-byte read +/// turned both into "AA", which is exactly the "one observed Makino returns two axis +/// folders both called AA" that Gitea #548 then had to disambiguate as AA_1/AA_2. +/// Trailing-only trimming keeps buffer residue. A control writes only the bytes the +/// name needs; on Makino Z-34184 (2026-08-19) cnc_rdspdlname came back +/// 53 31 00 4e — "S1" then the 'N' of an earlier "CNC". The name +/// ends at the first NUL. +/// +[Trait("Category", "Unit")] +public sealed class FocasNameRecordTests +{ + [Fact] + public void The_live_axis_name_capture_keeps_three_character_names_distinct() + { + var payload = WireFixtures.Payload("ref-axisname-0x0089.bin", 0x0089); + + var names = FocasWireClient.ReadNameRecords(payload, 32, (index, name) => (index, name)); + + names.Select(n => n.name).ShouldBe(["X", "Y", "Z", "B", "C", "AA6", "AA7"]); + names.Select(n => n.index).ShouldBe([(short)1, 2, 3, 4, 5, 6, 7]); + } + + [Fact] + public void The_live_spindle_name_capture_decodes() + { + var payload = WireFixtures.Payload("ref-spdlname-0x008a.bin", 0x008a); + + FocasWireClient.ReadNameRecords(payload, 8, (_, name) => name).ShouldBe(["S1"]); + } + + [Fact] + public void A_name_ends_at_its_first_NUL_so_residue_after_it_is_discarded() + { + // The Z-34184 spindle-name record: "S1", NUL, then an 'N' left over from a previous "CNC". + FocasWireProtocol.ReadNameRecord([0x53, 0x31, 0x00, 0x4e]).ShouldBe("S1"); + // A single-character name with residue in BOTH trailing bytes. + FocasWireProtocol.ReadNameRecord([0x58, 0x00, 0x43, 0x4e]).ShouldBe("X"); + } + + [Fact] + public void All_four_record_bytes_are_name_bytes() + { + FocasWireProtocol.ReadNameRecord("AA6\0"u8.ToArray()).ShouldBe("AA6"); + FocasWireProtocol.ReadNameRecord("ABCD"u8.ToArray()).ShouldBe("ABCD"); + FocasWireProtocol.ReadNameRecord("X \0\0"u8.ToArray()).ShouldBe("X"); + } + + [Fact] + public void A_record_that_starts_with_NUL_is_blank() + { + FocasWireProtocol.ReadNameRecord([0x00, 0x41, 0x41, 0x00]).ShouldBe(string.Empty); + } + + [Fact] + public void Servo_meter_names_come_out_whole_too() + { + var blocks = WireFixtures.Blocks("svmeter-pair-0x0056-0x0089.bin"); + var sv = blocks.First(b => b.Command == 0x0056).Payload; + var names = blocks.First(b => b.Command == 0x0089).Payload; + + var meters = FocasWireClient.ParseServoMeters(sv, names, maxCount: 32); + + meters.Take(7).Select(m => m.Name).ShouldBe(["X", "Y", "Z", "B", "C", "AA6", "AA7"]); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWireProtocolTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWireProtocolTests.cs index ad6bb6fd..c6fb23b8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWireProtocolTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasWireProtocolTests.cs @@ -239,11 +239,12 @@ public sealed class FocasWireProtocolTests } [Fact] - public void ReadNameRecord_reads_two_bytes_and_strips_padding() + public void ReadNameRecord_strips_padding() { + // Full record semantics (first NUL, all four bytes) are pinned in FocasNameRecordTests. FocasWireProtocol.ReadNameRecord("X \0\0"u8.ToArray()).ShouldBe("X"); FocasWireProtocol.ReadNameRecord("XY"u8.ToArray()).ShouldBe("XY"); - FocasWireProtocol.ReadNameRecord(new byte[] { 0x58 }).ShouldBe(string.Empty); // < 2 bytes + FocasWireProtocol.ReadNameRecord(ReadOnlySpan.Empty).ShouldBe(string.Empty); } // ---- helpers ----