diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs index c286ca0f..fa7d22c9 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs @@ -369,6 +369,15 @@ public sealed class S7Driver $"S7 tag '{t.Name}' is a Counter address ('{t.Address}') but is typed {t.DataType}. " + "Counter tags must be DataType=Int32 (decoded to count, read-only)."); } + + // (d) Timer/Counter declared Writable — writes are read-only this phase; without this + // a node is discovered as Operate-writable but every write returns BadNotSupported. + if (parsed.Area is S7Area.Timer or S7Area.Counter && t.Writable) + { + throw new NotSupportedException( + $"S7 tag '{t.Name}' is a Timer/Counter ('{t.Address}') declared Writable — " + + "Timer/Counter are read-only this phase; set Writable=false."); + } } } @@ -647,7 +656,14 @@ public sealed class S7Driver if (addr.Area is S7Area.Timer) return (object)global::S7.Net.Types.Timer.FromByteArray(block); if (addr.Area is S7Area.Counter) + { + // NOTE: S7.Net Counter.FromByteArray returns the RAW big-endian word ((bytes[0]<<8)|bytes[1]), + // NOT a BCD decode. On classic S7-300/400 the C-area word is BCD (0-999), so on that hardware + // this raw value can differ from the displayed count; S7-1200/1500 use IEC/DB counters (plain + // ints), where it is correct. Surfacing S7.Net's value verbatim is the faithful choice; BCD + // reinterpretation for legacy C-area counters is a live-hardware-gated follow-up. return (object)(int)global::S7.Net.Types.Counter.FromByteArray(block); + } // Each numeric arm is boxed to object explicitly: a bare switch expression would unify // long/ulong/double to their common type (double) and box THAT, mis-typing Int64/UInt64. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs index 3f96608c..005c9d5c 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7DriverScaffoldTests.cs @@ -176,4 +176,53 @@ public sealed class S7DriverScaffoldTests drv.GetHealth().State.ShouldBe(DriverState.Faulted); } + + // ── Phase 4d bundle-review fix — Writable Timer/Counter guard ─────────────────────── + // + // A Timer/Counter tag declared Writable=true must be rejected at init: without this guard + // the node is discovered as Operate-writable but every write returns BadNotSupported + // (EncodeScalarBlock throws). The guard fires before plc.OpenAsync, so the reserved-for- + // documentation host never receives traffic. + + /// Verifies that a Timer tag declared Writable=true is rejected at init with a Writable message. + [Fact] + public async Task Initialize_rejects_Timer_tag_declared_Writable() + { + var opts = new S7DriverOptions + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromMilliseconds(250), + Tags = [new S7TagDefinition("Timer5", "T5", S7DataType.Float64, Writable: true)], + }; + using var drv = new S7Driver(opts, "s7-timer-writable"); + + var ex = await Should.ThrowAsync(async () => + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); + ex.Message.ShouldContain("Timer5"); + ex.Message.ShouldContain("Writable", Case.Insensitive); + ex.Message.ShouldContain("read-only", Case.Insensitive); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } + + /// Verifies that a Counter tag declared Writable=true is rejected at init with a Writable message. + [Fact] + public async Task Initialize_rejects_Counter_tag_declared_Writable() + { + var opts = new S7DriverOptions + { + Host = "192.0.2.1", + Timeout = TimeSpan.FromMilliseconds(250), + Tags = [new S7TagDefinition("Counter3", "C3", S7DataType.Int32, Writable: true)], + }; + using var drv = new S7Driver(opts, "s7-counter-writable"); + + var ex = await Should.ThrowAsync(async () => + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken)); + ex.Message.ShouldContain("Counter3"); + ex.Message.ShouldContain("Writable", Case.Insensitive); + ex.Message.ShouldContain("read-only", Case.Insensitive); + + drv.GetHealth().State.ShouldBe(DriverState.Faulted); + } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs index db40a0b8..f2672bf6 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ScalarBlockTests.cs @@ -403,10 +403,11 @@ public sealed class S7ScalarBlockTests /// /// Verifies a 2-byte S5TIME block decodes to a duration in SECONDS as a double, /// routed by the Timer AREA (NOT the tag's Float64 DataType — that would mis-read the - /// 2-byte block as an 8-byte LReal). The fixture is a hand-built S5TIME word: the high - /// nibble of byte 0 carries the 2-bit time base (0=10 ms, 1=100 ms, 2=1 s, 3=10 s), the - /// low nibble of byte 0 + the two nibbles of byte 1 carry a 3-digit BCD value. Here - /// base bits = 2 (×1 s) and BCD = 100 → 100.0 seconds (matches S7.Net's + /// 2-byte block as an 8-byte LReal). The fixture is a hand-built S5TIME word: bits [15:14] + /// of the 16-bit word carry the time base (0=×10 ms, 1=×100 ms, 2=×1 s, 3=×10 s), bits + /// [13:12] are unused (always 0), and bits [11:0] carry the 3-digit BCD value (hundreds in + /// byte0 low nibble, tens in byte1 high nibble, ones in byte1 low nibble). Here base=2 + /// (×1 s) and BCD=100 → 100.0 s (verified against S7.Net's /// S5TIME decode). /// [Fact] @@ -437,6 +438,30 @@ public sealed class S7ScalarBlockTests result.ShouldBeOfType().ShouldBe(0.0); } + /// + /// Covers the two remaining S5TIME time bases to complete all four (0–3). S5TIME word + /// layout: bits[15:14]=base, bits[13:12]=0, bits[11:8]=BCD-hundreds, bits[7:4]=BCD-tens, + /// bits[3:0]=BCD-ones. Bases 1 and 2 are covered by the Fact tests above; base 0 + /// (×0.01 s) and base 3 (×10 s) are added here. + /// + /// Byte derivation: + /// + /// Base 0 (×0.01 s) / BCD=015: byte0=[base=0<<6|0<<4|hundreds=0]=0x00, + /// byte1=[tens=1<<4|ones=5]=0x15 → 15 × 0.01 s = 0.15 s. + /// Base 3 (×10 s) / BCD=005: byte0=[base=3<<6|0<<4|hundreds=0]=0x30, + /// byte1=[tens=0<<4|ones=5]=0x05 → 5 × 10 s = 50.0 s. + /// + /// + /// + [Theory] + [InlineData(new byte[] { 0x00, 0x15 }, 0.15)] // base 0 (×0.01 s), BCD=015 → 15×0.01=0.15 s + [InlineData(new byte[] { 0x30, 0x05 }, 50.0)] // base 3 (×10 s), BCD=005 → 5×10=50.0 s + public void DecodeScalarBlock_Timer_all_time_bases(byte[] block, double expectedSeconds) + { + var result = S7Driver.DecodeScalarBlock(TimerTag(), TimerAddr(), block); + result.ShouldBeOfType().ShouldBe(expectedSeconds, tolerance: 1e-9); + } + // ── DecodeScalarBlock — Counter (BCD/raw word → count, read-only) ────────────────────── ///