fix(s7): UInt64 box cast + Timer/Counter transient-write returns BadNotWritable (final review)
M1: add missing (object) cast to UInt64 arm of DecodeScalarBlock switch expression, matching the Int64 arm style and the comment that each arm is boxed explicitly. M2: short-circuit Timer/Counter writes in WriteAsync to BadNotWritable before WriteOneAsync, so transient equipment-tag refs (Writable=true from parser) return the same status code as authored tags rejected at init — documented in the docs. Adds 6 pure unit tests pinning the area-detection precondition the guard relies on. EncodeScalarBlock Timer/Counter throws remain as the defensive backstop.
This commit is contained in:
@@ -670,7 +670,7 @@ public sealed class S7Driver
|
||||
return tag.DataType switch
|
||||
{
|
||||
S7DataType.Int64 => (object)System.Buffers.Binary.BinaryPrimitives.ReadInt64BigEndian(block),
|
||||
S7DataType.UInt64 => System.Buffers.Binary.BinaryPrimitives.ReadUInt64BigEndian(block),
|
||||
S7DataType.UInt64 => (object)System.Buffers.Binary.BinaryPrimitives.ReadUInt64BigEndian(block),
|
||||
S7DataType.Float64 => System.Buffers.Binary.BinaryPrimitives.ReadDoubleBigEndian(block),
|
||||
|
||||
// S7 classic STRING: [maxLen byte][curLen byte][chars…]. S7.Net's S7String.FromByteArray
|
||||
@@ -949,6 +949,22 @@ public sealed class S7Driver
|
||||
results[i] = new WriteResult(StatusBadNotWritable);
|
||||
continue;
|
||||
}
|
||||
// Timer/Counter transient-write guard: a transient equipment-tag ref is always
|
||||
// resolved with Writable=true (S7EquipmentTagParser hardcodes it, and node-level
|
||||
// auth governs writes), so Timer/Counter addresses bypass the !Writable gate above
|
||||
// and would otherwise reach EncodeScalarBlock, which throws NotSupportedException →
|
||||
// BadNotSupported. The docs say Timer/Counter writes return BadNotWritable. Detect
|
||||
// the area here before the call so BOTH authored (caught by guard-d at init) and
|
||||
// transient Timer/Counter writes consistently return BadNotWritable.
|
||||
// TryParse (not Parse) so a malformed address falls through to the existing
|
||||
// error path rather than throwing here.
|
||||
if (_parsedByName.TryGetValue(tag.Name, out var tcParsed)
|
||||
? tcParsed.Area is S7Area.Timer or S7Area.Counter
|
||||
: (S7AddressParser.TryParse(tag.Address, out tcParsed) && tcParsed.Area is S7Area.Timer or S7Area.Counter))
|
||||
{
|
||||
results[i] = new WriteResult(StatusBadNotWritable);
|
||||
continue;
|
||||
}
|
||||
try
|
||||
{
|
||||
await WriteOneAsync(plc, tag, w.Value, cancellationToken).ConfigureAwait(false);
|
||||
|
||||
@@ -498,7 +498,10 @@ public sealed class S7ScalarBlockTests
|
||||
|
||||
// ── EncodeScalarBlock — Timer/Counter are read-only this phase ─────────────────────────
|
||||
|
||||
/// <summary>Verifies a Timer write throws NotSupportedException — Timer/Counter are read-only.</summary>
|
||||
/// <summary>Verifies a Timer write throws NotSupportedException — Timer/Counter are read-only.
|
||||
/// This backstop is still exercised even though <see cref="S7Driver.WriteAsync"/> now
|
||||
/// short-circuits to BadNotWritable before reaching EncodeScalarBlock — a mis-route (e.g.
|
||||
/// a future refactor that bypasses the WriteAsync guard) must still fail loudly here.</summary>
|
||||
[Fact]
|
||||
public void EncodeScalarBlock_Timer_throws_read_only()
|
||||
=> Should.Throw<NotSupportedException>(() => S7Driver.EncodeScalarBlock(TimerTag(), 1.0));
|
||||
@@ -507,4 +510,31 @@ public sealed class S7ScalarBlockTests
|
||||
[Fact]
|
||||
public void EncodeScalarBlock_Counter_throws_read_only()
|
||||
=> Should.Throw<NotSupportedException>(() => S7Driver.EncodeScalarBlock(CounterTag(), 42));
|
||||
|
||||
// ── WriteAsync guard — Timer/Counter area precondition ────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// Pins the precondition for the <see cref="S7Driver.WriteAsync"/> Timer/Counter
|
||||
/// short-circuit: <see cref="S7AddressParser.TryParse"/> on a Timer address must
|
||||
/// yield <see cref="S7Area.Timer"/> and on a Counter address must yield
|
||||
/// <see cref="S7Area.Counter"/>. The WriteAsync guard detects the area via TryParse (for
|
||||
/// transient equipment-tag refs not in _parsedByName) and returns BadNotWritable without
|
||||
/// reaching EncodeScalarBlock. This test verifies the area detection that gate depends on
|
||||
/// — a pure, no-PLC assertion. (<see cref="WriteAsync"/> itself requires a connected Plc
|
||||
/// and is therefore integration-tested only; the EncodeScalarBlock_Timer/Counter_throws
|
||||
/// tests above cover the defensive backstop if the guard is ever bypassed.)
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData("T0", S7Area.Timer)]
|
||||
[InlineData("T5", S7Area.Timer)]
|
||||
[InlineData("T15", S7Area.Timer)]
|
||||
[InlineData("C0", S7Area.Counter)]
|
||||
[InlineData("C3", S7Area.Counter)]
|
||||
[InlineData("C10", S7Area.Counter)]
|
||||
public void AddressParser_yields_Timer_or_Counter_area_for_TC_addresses(string address, S7Area expectedArea)
|
||||
{
|
||||
S7AddressParser.TryParse(address, out var parsed).ShouldBeTrue();
|
||||
parsed.Area.ShouldBe(expectedArea,
|
||||
$"WriteAsync BadNotWritable guard relies on TryParse('{address}').Area == {expectedArea}");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user