fix(s7): Counter raw-word note + reject Writable Timer/Counter + Timer time-base tests (bundle review)

2026-06-17 06:18:48 -04:00
parent 8cfb8e920e
commit 11e8e4302d
3 changed files with 94 additions and 4 deletions
@@ -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.
+
+    /// <summary>Verifies that a Timer tag declared Writable=true is rejected at init with a Writable message.</summary>
+    [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<NotSupportedException>(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);
+    }
+
+    /// <summary>Verifies that a Counter tag declared Writable=true is rejected at init with a Writable message.</summary>
+    [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<NotSupportedException>(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);
+    }
 }
@@ -403,10 +403,11 @@ public sealed class S7ScalarBlockTests
    /// <summary>
    ///     Verifies a 2-byte S5TIME block decodes to a duration in SECONDS as a <c>double</c>,
    ///     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
    ///     <see cref="S7.Net.Types.Timer.FromByteArray"/> S5TIME decode).
    /// </summary>
    [Fact]
@@ -437,6 +438,30 @@ public sealed class S7ScalarBlockTests
        result.ShouldBeOfType<double>().ShouldBe(0.0);
    }

+    /// <summary>
+    ///     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.
+    ///     <para>
+    ///     Byte derivation:
+    ///     <list type="bullet">
+    ///         <item>Base 0 (×0.01 s) / BCD=015: byte0=[base=0&lt;&lt;6|0&lt;&lt;4|hundreds=0]=0x00,
+    ///         byte1=[tens=1&lt;&lt;4|ones=5]=0x15 → 15 × 0.01 s = 0.15 s.</item>
+    ///         <item>Base 3 (×10 s) / BCD=005: byte0=[base=3&lt;&lt;6|0&lt;&lt;4|hundreds=0]=0x30,
+    ///         byte1=[tens=0&lt;&lt;4|ones=5]=0x05 → 5 × 10 s = 50.0 s.</item>
+    ///     </list>
+    ///     </para>
+    /// </summary>
+    [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<double>().ShouldBe(expectedSeconds, tolerance: 1e-9);
+    }
+
    // ── DecodeScalarBlock — Counter (BCD/raw word → count, read-only) ──────────────────────

    /// <summary>