diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs
index a2378a55..3b48f4d1 100644
--- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs
+++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/ReadCommand.cs
@@ -57,6 +57,14 @@ public sealed class ReadCommand : ModbusCommandBase
ValidateEndpoint();
var ct = console.RegisterCancellationHandler();
+ // The driver's tag factory refuses an incompatible region/type pair (the tag would read
+ // BadNodeIdUnknown) — say why up front instead.
+ if (!ModbusTagDefinitionFactory.IsRegionTypeCompatible(Region, DataType))
+ throw new CliFx.Exceptions.CommandException(
+ Region is ModbusRegion.Coils or ModbusRegion.DiscreteInputs
+ ? $"Region '{Region}' only carries Bool values (--type Bool); got '{DataType}'."
+ : $"Region '{Region}' carries register types; use --type BitInRegister for a bit of a register, not '{DataType}'.");
+
var tagName = SynthesiseTagName(Region, Address, DataType);
var tag = new ModbusTagDefinition(
Name: tagName,
diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs
index 07bb3bf7..e1876a49 100644
--- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs
+++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs
@@ -74,6 +74,9 @@ public sealed class WriteCommand : ModbusCommandBase
throw new CliFx.Exceptions.CommandException(
$"Region 'Coils' only supports boolean values (--type Bool). " +
$"Type '{DataType}' cannot represent a single-bit coil write.");
+ if (!ModbusTagDefinitionFactory.IsRegionTypeCompatible(Region, DataType))
+ throw new CliFx.Exceptions.CommandException(
+ $"Region '{Region}' carries register types; use --type BitInRegister for a bit of a register, not '{DataType}'.");
var tagName = ReadCommand.SynthesiseTagName(Region, Address, DataType);
var tag = new ModbusTagDefinition(
diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs
index 02e2ff34..86b0a4e9 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Contracts/ModbusTagDefinitionFactory.cs
@@ -50,7 +50,13 @@ public static class ModbusTagDefinitionFactory
// Strict enum reads: a present-but-invalid (typo'd) enum rejects the tag
// (→ BadNodeIdUnknown) instead of silently defaulting to a wrong-width Good.
if (!TagConfigJson.TryReadEnumStrict(root, "region", ModbusRegion.HoldingRegisters, out var region)) return false;
- if (!TagConfigJson.TryReadEnumStrict(root, "dataType", ModbusDataType.Int16, out var dataType)) return false;
+ // An absent dataType defaults per region: a bit region (Coils / DiscreteInputs) only carries
+ // Bool, so defaulting it to Int16 would reject every bit tag authored without the key.
+ if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DefaultDataType(region), out var dataType)) return false;
+ // Region/type compatibility: a bit region carries only Bool, a register region never Bool.
+ // A mismatch used to throw out of the coalescing planner (killing every tag's poll) or
+ // silently return a bool into an Int32 node.
+ if (!IsRegionTypeCompatible(region, dataType)) return false;
if (!TagConfigJson.TryReadEnumStrict(root, "byteOrder", ModbusByteOrder.BigEndian, out var byteOrder)) return false;
if (!TagConfigJson.TryReadEnumStrict(root, "stringByteOrder", ModbusStringByteOrder.HighByteFirst, out var stringByteOrder)) return false;
var bitIndex = (byte)ReadInt(root, "bitIndex");
@@ -94,6 +100,25 @@ public static class ModbusTagDefinitionFactory
catch (InvalidOperationException) { return false; }
}
+ /// The data type an authored tag gets when its dataType key is absent.
+ /// The tag's region.
+ /// Bool for Coils / DiscreteInputs, Int16 for the register regions.
+ public static ModbusDataType DefaultDataType(ModbusRegion region)
+ => region is ModbusRegion.Coils or ModbusRegion.DiscreteInputs ? ModbusDataType.Bool : ModbusDataType.Int16;
+
+ ///
+ /// Region/type compatibility, the one rule every authoring path and the driver share: Coils and
+ /// DiscreteInputs carry only Bool; HoldingRegisters and InputRegisters carry every register
+ /// type (including BitInRegister) but never Bool.
+ ///
+ /// The tag's region.
+ /// The tag's data type.
+ /// when the driver can read the pair.
+ public static bool IsRegionTypeCompatible(ModbusRegion region, ModbusDataType dataType)
+ => region is ModbusRegion.Coils or ModbusRegion.DiscreteInputs
+ ? dataType == ModbusDataType.Bool
+ : dataType != ModbusDataType.Bool;
+
///
/// Inverse of : serialises a typed definition back to the camelCase
/// TagConfig JSON the mapper reads. Used by authoring paths (the Client CLI) that construct a
@@ -247,7 +272,17 @@ public static class ModbusTagDefinitionFactory
// Type-dependent guards. The byte/ushort NARROWING is part of the rule, not an accident of the
// mapper: it reads `(byte)bitIndex` / `(ushort)stringLength`, so 256 is bit 0 and is ACCEPTED.
// Describing the un-narrowed value here would call a blob invalid that the driver serves.
- if (!TagConfigJson.TryReadEnumStrict(root, "dataType", ModbusDataType.Int16, out var dataType)) return;
+ var region = TagConfigJson.TryReadEnumStrict(root, "region", ModbusRegion.HoldingRegisters, out var r)
+ ? r : ModbusRegion.HoldingRegisters;
+ if (!TagConfigJson.TryReadEnumStrict(root, "dataType", DefaultDataType(region), out var dataType)) return;
+
+ if (root.TryGetProperty("region", out _) && !IsRegionTypeCompatible(region, dataType))
+ {
+ warnings.Add(region is ModbusRegion.Coils or ModbusRegion.DiscreteInputs
+ ? $"Modbus region {region} only carries Bool tags; got {dataType} — the tag is rejected (BadNodeIdUnknown)."
+ : $"Modbus Bool tags only live in Coils / DiscreteInputs; a {region} tag must use a register type "
+ + "(use BitInRegister for a bit of a register) — the tag is rejected (BadNodeIdUnknown).");
+ }
if (dataType == ModbusDataType.String && (ushort)ReadInt(root, "stringLength") < 1)
{
diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
index 648e4c99..eedfb70b 100644
--- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
+++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs
@@ -1186,6 +1186,9 @@ public sealed class ModbusDriver
{
if (!table.TryGetValue(fullReferences[i], out var tag)) continue;
if (tag.CoalesceProhibited) continue;
+ // Defensive: the factory refuses incompatible pairs, but the planner's register math must never
+ // throw for a tag that slipped through — one bad tag would otherwise fail the whole batch.
+ if (!ModbusTagDefinitionFactory.IsRegionTypeCompatible(tag.Region, tag.DataType)) continue;
if (tag.ArrayCount.HasValue) continue;
if (tag.Region is not (ModbusRegion.HoldingRegisters or ModbusRegion.InputRegisters)) continue;
if (tag.DataType is ModbusDataType.String or ModbusDataType.BitInRegister) continue;
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor
index 116f9bd5..a3b38dd4 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/ModbusTagConfigEditor.razor
@@ -5,7 +5,7 @@
-
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs
index 323830ab..29050fc4 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/ModbusTagConfigModel.cs
@@ -30,6 +30,19 @@ public sealed class ModbusTagConfigModel
/// key stays absent.
public bool? Writable { get; set; }
+ ///
+ /// Changes the region and keeps the data type compatible with it (the driver refuses a bit region
+ /// with a non-Bool type, and a register region with Bool): switching to Coils / DiscreteInputs selects
+ /// Bool, switching from them back to a register region selects Int16.
+ ///
+ /// The newly selected region.
+ public void SetRegion(ModbusRegion region)
+ {
+ Region = region;
+ if (!ModbusTagDefinitionFactory.IsRegionTypeCompatible(region, DataType))
+ DataType = ModbusTagDefinitionFactory.DefaultDataType(region);
+ }
+
private JsonObject _bag = new();
// The blob this model was LOADED FROM, or null when it was built by object initialiser rather than
@@ -49,7 +62,9 @@ public sealed class ModbusTagConfigModel
_source = json ?? "",
Region = TagConfigJson.GetEnum(o, "region", ModbusRegion.HoldingRegisters),
Address = TagConfigJson.GetInt(o, "address"),
- DataType = TagConfigJson.GetEnum(o, "dataType", ModbusDataType.Int16),
+ // Absent dataType defaults per region, exactly as the driver's factory does (Bool for a bit region).
+ DataType = TagConfigJson.GetEnum(o, "dataType",
+ ModbusTagDefinitionFactory.DefaultDataType(TagConfigJson.GetEnum(o, "region", ModbusRegion.HoldingRegisters))),
ByteOrder = TagConfigJson.GetEnum(o, "byteOrder", ModbusByteOrder.BigEndian),
BitIndex = TagConfigJson.GetInt(o, "bitIndex"),
StringLength = TagConfigJson.GetInt(o, "stringLength"),
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs
index a47a721a..6d2363b1 100644
--- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusArrayWriteSupportParityTests.cs
@@ -25,10 +25,43 @@ public sealed class ModbusArrayWriteSupportParityTests
return data;
}
+ ///
+ /// Shapes where the driver has moved ahead of the server-side table (review 07 P6 / U3, Modbus lane) and
+ /// — owned outside the Modbus lane — has not followed yet
+ /// (follow-up F1): the tag factory now REFUSES a Coils tag with a non-Bool type (so the driver serves no
+ /// such tag), and the driver now encodes String[] and BitInRegister[] holding-register writes.
+ /// fails the moment the table follows, so this list
+ /// is removed with it rather than left to rot.
+ ///
+ private static bool PendingF1(ModbusRegion region, ModbusDataType dataType) =>
+ (region == ModbusRegion.Coils && dataType != ModbusDataType.Bool)
+ || (region == ModbusRegion.HoldingRegisters && dataType is ModbusDataType.String or ModbusDataType.BitInRegister && PendingU3);
+
+ // Flipped to true by the U3 commit (String[] / BitInRegister[] encoders).
+ private const bool PendingU3 = false;
+
+ public static TheoryData PendingShapes()
+ {
+ var data = new TheoryData();
+ foreach (var (region, dt) in Shapes().Where(s => PendingF1(s.Region, s.DataType))) data.Add(region, dt);
+ return data;
+ }
+
+ [Theory]
+ [MemberData(nameof(PendingShapes))]
+ public void Pending_F1_divergences_still_exist(ModbusRegion region, ModbusDataType dataType)
+ {
+ var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":2}""";
+ var driverAccepts = region == ModbusRegion.HoldingRegisters; // String[] / BitInRegister[] encode; mistyped Coils are refused
+ DriverArrayWriteSupport.Supports(DriverTypeNames.Modbus, "Int16", json).ShouldNotBe(driverAccepts,
+ "F1 landed — DriverArrayWriteSupport now agrees with the driver for this shape; delete it from PendingF1");
+ }
+
[Theory]
[MemberData(nameof(AllShapes))]
public async Task Table_agrees_with_the_driver_write_path(ModbusRegion region, ModbusDataType dataType)
{
+ if (PendingF1(region, dataType)) return; // covered by Pending_F1_divergences_still_exist
var json = $$"""{"region":"{{region}}","address":10,"dataType":"{{dataType}}","stringLength":4,"isArray":true,"arrayLength":2}""";
var fake = new ModbusDriverTests.FakeTransport();
var drv = new ModbusDriver(
diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRegionTypeValidationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRegionTypeValidationTests.cs
new file mode 100644
index 00000000..b4acdaee
--- /dev/null
+++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusRegionTypeValidationTests.cs
@@ -0,0 +1,70 @@
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
+
+namespace ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests;
+
+///
+/// Review 07 P6: region/type compatibility is enforced at the tag factory (and so at the deploy gate
+/// and the AdminUI parse consult). A HoldingRegisters + Bool tag used to throw out of the
+/// coalescing planner and kill the whole subscription's poll cycle; Coils + Int16 silently
+/// returned a bool into an Int32 node.
+///
+[Trait("Category", "Unit")]
+public sealed class ModbusRegionTypeValidationTests
+{
+ [Theory]
+ [InlineData("""{"region":"Coils","address":1,"dataType":"Int16"}""")]
+ [InlineData("""{"region":"DiscreteInputs","address":1,"dataType":"Float32"}""")]
+ [InlineData("""{"region":"HoldingRegisters","address":1,"dataType":"Bool"}""")]
+ [InlineData("""{"region":"InputRegisters","address":1,"dataType":"Bool"}""")]
+ public void Incompatible_region_and_type_is_rejected(string json)
+ {
+ ModbusTagDefinitionFactory.FromTagConfig(json, "T", out _).ShouldBeFalse();
+ ModbusTagDefinitionFactory.Inspect(json).ShouldContain(w => w.Contains("only") || w.Contains("register"));
+ }
+
+ [Theory]
+ [InlineData("""{"region":"Coils","address":1,"dataType":"Bool"}""", ModbusDataType.Bool)]
+ [InlineData("""{"region":"Coils","address":1}""", ModbusDataType.Bool)]
+ [InlineData("""{"region":"DiscreteInputs","address":1}""", ModbusDataType.Bool)]
+ [InlineData("""{"region":"InputRegisters","address":1,"dataType":"BitInRegister","bitIndex":3}""", ModbusDataType.BitInRegister)]
+ [InlineData("""{"region":"HoldingRegisters","address":1}""", ModbusDataType.Int16)]
+ public void Compatible_region_and_type_is_accepted(string json, ModbusDataType expected)
+ {
+ // Positive control for the rejection theory, including the absent-dataType default per region.
+ ModbusTagDefinitionFactory.FromTagConfig(json, "T", out var def).ShouldBeTrue();
+ def.DataType.ShouldBe(expected);
+ ModbusTagDefinitionFactory.Inspect(json).ShouldBeEmpty();
+ }
+
+ [Fact]
+ public async Task A_mis_typed_tag_no_longer_kills_the_coalesced_poll_of_its_neighbours()
+ {
+ var fake = new ModbusDriverTests.FakeTransport();
+ fake.HoldingRegisters[100] = 7;
+ fake.HoldingRegisters[102] = 9;
+ var good1 = new ModbusTagDefinition("G1", ModbusRegion.HoldingRegisters, 100, ModbusDataType.Int16);
+ var good2 = new ModbusTagDefinition("G2", ModbusRegion.HoldingRegisters, 102, ModbusDataType.Int16);
+ var opts = new ModbusDriverOptions
+ {
+ Host = "f",
+ MaxReadGap = 5,
+ RawTags =
+ [
+ ModbusRawTags.Entry(good1),
+ new RawTagEntry("Bad", """{"region":"HoldingRegisters","address":101,"dataType":"Bool"}""", false),
+ ModbusRawTags.Entry(good2),
+ ],
+ Probe = new ModbusProbeOptions { Enabled = false },
+ };
+ await using var drv = new ModbusDriver(opts, "m1", _ => fake);
+ await drv.InitializeAsync("{}", TestContext.Current.CancellationToken);
+
+ var r = await drv.ReadAsync(["G1", "Bad", "G2"], TestContext.Current.CancellationToken);
+ r[0].Value.ShouldBe((short)7);
+ r[1].StatusCode.ShouldBe(0x80340000u); // BadNodeIdUnknown — skipped at init, reported
+ r[2].Value.ShouldBe((short)9);
+ drv.GetSkippedTags()!.Sample.ShouldContain(s => s.RawPath == "Bad");
+ }
+}
diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs
index 2bb03383..e1afd0c1 100644
--- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs
+++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/ModbusTagConfigModelTests.cs
@@ -121,4 +121,24 @@ public sealed class ModbusTagConfigModelTests
[InlineData("""{"region":3,"address":1,"dataType":true}""")]
public void Validate_accepts_every_enum_value_the_driver_accepts(string json)
=> ModbusTagConfigModel.FromJson(json).Validate().ShouldBeNull();
+
+ /// Review 07 P6: switching region keeps the data type one the driver accepts for it.
+ [Theory]
+ [InlineData(ModbusDataType.Int16, ModbusRegion.Coils, ModbusDataType.Bool)]
+ [InlineData(ModbusDataType.Float32, ModbusRegion.DiscreteInputs, ModbusDataType.Bool)]
+ [InlineData(ModbusDataType.Bool, ModbusRegion.HoldingRegisters, ModbusDataType.Int16)]
+ [InlineData(ModbusDataType.Float32, ModbusRegion.InputRegisters, ModbusDataType.Float32)]
+ public void SetRegion_keeps_the_type_compatible(ModbusDataType from, ModbusRegion region, ModbusDataType expected)
+ {
+ var m = new ModbusTagConfigModel { DataType = from };
+ m.SetRegion(region);
+ m.Region.ShouldBe(region);
+ m.DataType.ShouldBe(expected);
+ ModbusTagConfigModel.FromJson(m.ToJson()).Validate().ShouldBeNull();
+ }
+
+ /// A Coils + Int16 blob (CSV / API) is refused at authoring, matching the driver.
+ [Fact]
+ public void Validate_refuses_an_incompatible_region_and_type()
+ => ModbusTagConfigModel.FromJson("""{"region":"Coils","address":1,"dataType":"Int16"}""").Validate().ShouldNotBeNull();
}