From 665ecd55c296cefa1bb7a0081f0bcecc5913ad4d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 7 Oct 2026 13:57:11 -0400 Subject: [PATCH] fix(focas): decode cnc_rdmacro dec_val at offset 6; vacant/fractional macros go Bad The ODBM wire payload is [i32 mantissa][i16 length, a constant 10][i16 dec_val]. ReadMacroAsync read the decimal at offset 4, so every macro decoded without failing but wrongly: the captured #500 = 1.0 (05 f5 e1 00 00 0a 00 08) published 0.01. FANUC normalises macros to a 9-significant-digit mantissa, so the error was never small. - dec_val is read at [6..7]; a payload < 8 bytes is refused, not guessed at. - A vacant variable (dec_val 0xFFFF) publishes BadNoData, never a Good 0. - Integer-typed tags get the exact whole value (integer arithmetic); a fractional macro on an integer tag is BadTypeMismatch, a whole value that does not fit the type BadOutOfRange. Float tags get mantissa / 10^dec. Ported from the sister ignitionoee FOCAS driver v0.15.0 (Focas.Wire-013, Focas-017/018); live vectors from Makino Z-34184 (31i-B, G431). Adds a WireFixtures helper that loads the raw 31i-B captures in Fixtures/v3. --- .../FocasStatusMapper.cs | 8 + .../Wire/FocasWireClient.cs | 26 +++- .../Wire/FocasWireModels.cs | 24 ++- .../Wire/WireFocasClient.cs | 65 +++++++- .../FocasMacroDecodeTests.cs | 143 ++++++++++++++++++ .../WireFixtures.cs | 31 ++++ ...B.MOM.WW.OtOpcUa.Driver.FOCAS.Tests.csproj | 5 + 7 files changed, 292 insertions(+), 10 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasMacroDecodeTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/WireFixtures.cs diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs index 33f65bba..90fdcbf3 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasStatusMapper.cs @@ -19,6 +19,14 @@ public static class FocasStatusMapper public const uint BadTimeout = 0x800A0000u; public const uint BadTypeMismatch = 0x80740000u; + /// + /// The CNC answered, but its answer describes no usable value — a vacant macro variable, a + /// position / meter record whose own scale envelope is undecodable, a field the control did + /// not report. Distinct from a comms fault (the device is fine) and never a fabricated + /// Good 0. + /// + public const uint BadNoData = 0x809B0000u; + /// /// Map common FWLIB EW_* return codes. The values below match Fanuc's published /// numeric conventions (EW_OK=0, EW_FUNC=1, EW_NUMBER=3, EW_LENGTH=4, EW_ATTRIB=7, 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 02dce813..e160386d 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 @@ -577,9 +577,33 @@ public sealed class FocasWireClient : IAsyncDisposable, IDisposable ushort? pathId = null) => ReadSingleWithTimeoutAsync( 0x0015, - payload => new WireMacro(number, payload.Length >= 4 ? ReadInt32(payload, 0) : 0, payload.Length >= 6 ? ReadInt16(payload, 4) : (short)0), + payload => ParseMacro(number, payload), cancellationToken, timeout, EffectivePathId(pathId), number, number); + /// + /// Decode a cnc_rdmacro (0x0015) payload. The on-wire ODBM record is 8 + /// big-endian bytes: [0..3] i32 mantissa, [4..5] i16 length field (a + /// constant 10 on every observed response), [6..7] i16 dec_val + /// (0xFFFF = vacant). + /// + /// + /// The middle field is the trap: it is not the decimal count but looks like a plausible one, + /// so reading the decimal at offset 4 decoded every macro on every control without ever + /// failing — just wrongly (the captured #500 = 1.0 published 0.01). Pinned by + /// a live Makino 31i-B capture; found and fixed by the sister ignitionoee driver + /// (Focas.Wire-013, 2026-08-05). A payload shorter than 8 bytes carries no + /// dec_val and is refused rather than guessed at. + /// + /// The macro number the request asked for. + /// The response-block payload. + /// The decoded macro. + /// The payload is shorter than 8 bytes. + internal static WireMacro ParseMacro(short number, byte[] payload) + { + RequireLength(payload, 8, "cnc_rdmacro"); + return new WireMacro(number, ReadInt32(payload, 0), ReadInt16(payload, 6)); + } + /// /// Read a PMC range via pmc_rdpmcrng. is the numeric /// address-letter code (see ); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireModels.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireModels.cs index abdc1d74..7e246fc9 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireModels.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/FocasWireModels.cs @@ -86,13 +86,31 @@ public sealed record WireParameter( int Value); /// -/// Macro variable from cnc_rdmacro. Scaled decimal: the callable value is -/// Value / 10^Decimal. +/// Macro variable from cnc_rdmacro. Scaled decimal: the real value is +/// Value / 10^Decimal (). FANUC normalises every macro to a +/// 9-significant-digit mantissa, so alone is never a value to publish +/// (live: #502 = 270 arrives as 270000000 / 10^6). /// +/// Macro variable number (the request argument; not carried in the payload). +/// The 9-significant-digit mantissa. +/// Implied decimal places; negative (0xFFFF on the wire) = vacant. public sealed record WireMacro( short Number, int Value, - short Decimal); + short Decimal) +{ + /// + /// True when the CNC reported the variable as vacant (undefined / never set): + /// dec_val == 0xFFFF. A vacant macro's mantissa is 0, indistinguishable from a + /// genuine 0, so this must be tested before means anything. Any + /// negative decimal is treated as vacant — no other negative has ever been observed and + /// failing closed is the safe reading. + /// + public bool IsVacant => Decimal < 0; + + /// The real value, Value / 10^Decimal; null when . + public double? Scaled => IsVacant ? null : Decimal == 0 ? Value : Value / Math.Pow(10.0, Decimal); +} /// PMC range read-back from pmc_rdpmcrng: one or more values of the requested width. public sealed record WirePmcRange( diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs index bea99a40..2b96b73c 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/Wire/WireFocasClient.cs @@ -62,7 +62,7 @@ public sealed class WireFocasClient : IFocasClient { FocasAreaKind.Pmc => await ReadPmcAsync(address, type, cancellationToken).ConfigureAwait(false), FocasAreaKind.Parameter => await ReadParameterAsync(address, type, cancellationToken).ConfigureAwait(false), - FocasAreaKind.Macro => await ReadMacroAsync(address, cancellationToken).ConfigureAwait(false), + FocasAreaKind.Macro => await ReadMacroAsync(address, type, cancellationToken).ConfigureAwait(false), _ => (null, FocasStatusMapper.BadNotSupported), }; } @@ -368,17 +368,14 @@ public sealed class WireFocasClient : IFocasClient } private async Task<(object? value, uint status)> ReadMacroAsync( - FocasAddress address, CancellationToken cancellationToken) + FocasAddress address, FocasDataType type, CancellationToken cancellationToken) { try { var result = await _wire.ReadMacroAsync((short)address.Number, cancellationToken).ConfigureAwait(false); if (!result.IsOk || result.Value is null) return (null, FocasStatusMapper.MapFocasReturn(result.Rc)); - var m = result.Value; - // Macro value is scaled-decimal: the real value is Value / 10^Decimal. - var scaled = m.Value / Math.Pow(10.0, m.Decimal); - return ((object)scaled, FocasStatusMapper.Good); + return MapMacro(result.Value, type, address.BitIndex); } catch (FocasWireException ex) { @@ -386,6 +383,62 @@ public sealed class WireFocasClient : IFocasClient } } + /// + /// Turn a decoded macro into the value + quality its tag publishes. + /// + /// + /// A vacant variable has no value at all → , + /// never a Good 0 (its mantissa is 0, indistinguishable from a genuine zero). + /// Float tags get mantissa / 10^decimal. Integer tags (Byte / Int16 / + /// Int32 / Bit) get the exact whole value, tested with integer arithmetic + /// (mantissa % 10^decimal) so no epsilon is involved; a fractional macro on an + /// integer tag is — truncating or rounding + /// would publish a plausible wrong count at Good quality — and a whole value that does not + /// fit the type is . Ported from the sister + /// ignitionoee driver's macroInteger/macroReal (Focas-017/018). + /// + /// The decoded macro. + /// The tag's declared data type. + /// Optional bit index for a tag. + /// The boxed value (null when Bad) and its status. + internal static (object? value, uint status) MapMacro(WireMacro m, FocasDataType type, int? bitIndex) + { + if (m.Scaled is not { } scaled) return (null, FocasStatusMapper.BadNoData); + + switch (type) + { + case FocasDataType.Float64: + return (scaled, FocasStatusMapper.Good); + case FocasDataType.Float32: + return ((float)scaled, FocasStatusMapper.Good); + case FocasDataType.Byte or FocasDataType.Int16 or FocasDataType.Int32 or FocasDataType.Bit: + if (!TryWholeMacroValue(m, out var whole)) return (null, FocasStatusMapper.BadTypeMismatch); + return type switch + { + FocasDataType.Bit => (bitIndex is int bit ? ((whole >> bit) & 1L) != 0 : whole != 0, FocasStatusMapper.Good), + FocasDataType.Byte when whole is >= sbyte.MinValue and <= sbyte.MaxValue => ((sbyte)whole, FocasStatusMapper.Good), + FocasDataType.Int16 when whole is >= short.MinValue and <= short.MaxValue => ((short)whole, FocasStatusMapper.Good), + FocasDataType.Int32 when whole is >= int.MinValue and <= int.MaxValue => ((int)whole, FocasStatusMapper.Good), + _ => (null, FocasStatusMapper.BadOutOfRange), + }; + default: + return (scaled, FocasStatusMapper.Good); + } + } + + /// Exact mantissa / 10^decimal when it is a whole number; integer arithmetic only. + private static bool TryWholeMacroValue(WireMacro m, out long whole) + { + whole = 0; + if (m.Value == 0) return true; // 0 is whole at any decimal + if (m.Decimal > 18) return false; // an i32 mantissa over 10^19+ is always fractional + var divisor = 1L; + for (var i = 0; i < m.Decimal; i++) divisor *= 10L; + if (m.Value % divisor != 0) return false; + whole = m.Value / divisor; + return true; + } + private void RequireConnected() { if (!_wire.IsConnected) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasMacroDecodeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasMacroDecodeTests.cs new file mode 100644 index 00000000..15bcdaab --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasMacroDecodeTests.cs @@ -0,0 +1,143 @@ +using System.Buffers.Binary; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Wire; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// cnc_rdmacro (0x0015) decode, pinned to real FANUC 31i-B payloads. Ported from the +/// sister ignitionoee FOCAS driver's v0.15.0 fix (review finding Focas.Wire-013 / Focas-017). +/// +/// +/// The on-wire ODBM payload is 8 bytes: [0..3] i32 mantissa, +/// [4..5] i16 — a constant 10 on every observed response (a length field, NOT the +/// decimal), and [6..7] i16 dec_val (0xFFFF = vacant). Reading the decimal at +/// offset 4 decodes every macro on every control without ever failing — just wrongly: the +/// captured #500 = 1.0 (05 f5 e1 00 00 0a 00 08) read as 100000000 / 10^10 = +/// 0.01. FANUC normalises a macro to a 9-significant-digit mantissa, so the live vectors +/// below (Makino Z-34184, 2026-08-05) all carry large mantissas. +/// +[Trait("Category", "Unit")] +public sealed class FocasMacroDecodeTests +{ + private static byte[] Odbm(int mantissa, int decVal, int lengthField = 10) + { + var p = new byte[8]; + BinaryPrimitives.WriteInt32BigEndian(p.AsSpan(0, 4), mantissa); + BinaryPrimitives.WriteUInt16BigEndian(p.AsSpan(4, 2), (ushort)lengthField); + BinaryPrimitives.WriteUInt16BigEndian(p.AsSpan(6, 2), (ushort)(decVal & 0xFFFF)); + return p; + } + + [Fact] + public void ParseMacro_decodes_the_live_capture_with_the_decimal_at_offset_6() + { + var payload = WireFixtures.Payload("ref-macro-500-0x0015.bin", 0x0015); + + var m = FocasWireClient.ParseMacro(500, payload); + + m.Number.ShouldBe((short)500); + m.Value.ShouldBe(100_000_000); + m.Decimal.ShouldBe((short)8); // offset 6 — NOT the constant 10 at offset 4 + m.IsVacant.ShouldBeFalse(); + m.Scaled!.Value.ShouldBe(1.0, 1e-12); + } + + [Fact] + public void ParseMacro_ignores_the_length_field_at_offset_4() + { + var a = FocasWireClient.ParseMacro(508, Odbm(148_500_000, 6)); + var b = FocasWireClient.ParseMacro(508, Odbm(148_500_000, 6, lengthField: 0)); + + b.ShouldBe(a); + b.Scaled!.Value.ShouldBe(148.5, 1e-9); + } + + [Theory] + [InlineData(270_000_000, 6, 270.0)] // #502 + [InlineData(148_500_000, 6, 148.5)] // #508 + [InlineData(208_850_000, 4, 20885.0)] // #610 + [InlineData(202_608_050, 1, 20_260_805.0)] // #3011 date + [InlineData(-100_000_000, 12, -1.0e-4)] // #802 + [InlineData(969_479_000, 9, 0.969479)] // #801 + [InlineData(1234, 0, 1234.0)] + public void ParseMacro_scales_the_live_vectors(int mantissa, int dec, double expected) + { + FocasWireClient.ParseMacro(1, Odbm(mantissa, dec)).Scaled!.Value.ShouldBe(expected, 1e-9); + } + + [Fact] + public void A_vacant_macro_has_no_value_not_zero() + { + var m = FocasWireClient.ParseMacro(611, Odbm(0, 0xFFFF)); + + m.Decimal.ShouldBe((short)-1); + m.IsVacant.ShouldBeTrue(); + m.Scaled.ShouldBeNull(); + } + + [Fact] + public void A_genuine_zero_is_not_vacant() + { + var m = FocasWireClient.ParseMacro(3901, Odbm(0, 0)); + m.IsVacant.ShouldBeFalse(); + m.Scaled.ShouldBe(0.0); + } + + [Theory] + [InlineData(0)] + [InlineData(4)] + [InlineData(6)] + [InlineData(7)] + public void ParseMacro_refuses_a_payload_too_short_to_carry_dec_val(int length) + { + Should.Throw(() => FocasWireClient.ParseMacro(1, new byte[length])); + } + + // ---- published value / quality ---- + + [Fact] + public void Float64_publishes_the_scaled_value_good() + { + var (value, status) = WireFocasClient.MapMacro(FocasWireClient.ParseMacro(508, Odbm(148_500_000, 6)), FocasDataType.Float64, null); + status.ShouldBe(FocasStatusMapper.Good); + value.ShouldBe(148.5); + } + + [Fact] + public void Int32_publishes_the_exact_whole_value_not_the_mantissa() + { + var (value, status) = WireFocasClient.MapMacro(FocasWireClient.ParseMacro(502, Odbm(270_000_000, 6)), FocasDataType.Int32, null); + status.ShouldBe(FocasStatusMapper.Good); + value.ShouldBe(270); + } + + [Fact] + public void A_fractional_macro_on_an_integer_tag_is_bad_not_truncated() + { + var (value, status) = WireFocasClient.MapMacro(FocasWireClient.ParseMacro(508, Odbm(148_500_000, 6)), FocasDataType.Int32, null); + status.ShouldBe(FocasStatusMapper.BadTypeMismatch); + value.ShouldBeNull(); + } + + [Theory] + [InlineData(FocasDataType.Float64)] + [InlineData(FocasDataType.Float32)] + [InlineData(FocasDataType.Int32)] + public void A_vacant_macro_is_bad_for_every_type(FocasDataType type) + { + var (value, status) = WireFocasClient.MapMacro(FocasWireClient.ParseMacro(611, Odbm(0, 0xFFFF)), type, null); + status.ShouldBe(FocasStatusMapper.BadNoData); + value.ShouldBeNull(); + } + + [Fact] + public void A_whole_macro_out_of_range_for_its_integer_type_is_bad() + { + // 70000 is whole, but does not fit Int16. + var (value, status) = WireFocasClient.MapMacro(FocasWireClient.ParseMacro(1, Odbm(700_000_000, 4)), FocasDataType.Int16, null); + status.ShouldBe(FocasStatusMapper.BadOutOfRange); + value.ShouldBeNull(); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/WireFixtures.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/WireFixtures.cs new file mode 100644 index 00000000..0960bd37 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/WireFixtures.cs @@ -0,0 +1,31 @@ +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Wire; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Loads the raw FOCAS response PDUs under Fixtures/v3/ — byte-for-byte captures from a +/// real FANUC 31i-B (Makino, series G431) — and splits them into response blocks with the +/// production . The live-* captures +/// were taken by the sister ignitionoee FOCAS driver's read-only probes of Z-34184 (2026-08-12) +/// and are shared verbatim; the ref-* ones are this repo's own 2026-06-25 capture. +/// +internal static class WireFixtures +{ + private const int PduHeaderLength = 10; + + /// Every response block of the named capture, in wire order. + /// File name under Fixtures/v3/. + /// The parsed response blocks. + public static IReadOnlyList Blocks(string name) + { + var raw = File.ReadAllBytes(Path.Combine(AppContext.BaseDirectory, "Fixtures", "v3", name)); + return FocasWireProtocol.ParseResponseBlocks(raw.AsSpan(PduHeaderLength)); + } + + /// The payload of the first block carrying . + /// File name under Fixtures/v3/. + /// The FOCAS command id to locate. + /// That block's payload bytes. + public static byte[] Payload(string name, ushort command) => + Blocks(name).First(b => b.Command == command).Payload; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests.csproj b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests.csproj index 5d596302..4a98f116 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests.csproj +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests.csproj @@ -19,6 +19,11 @@ + + + + +