From dd1742e31988664c17f2e7d24cbcddd61bc359a2 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 22 May 2026 09:55:16 -0400 Subject: [PATCH] fix(driver-modbus-cli): resolve Medium code-review finding (Driver.Modbus.Cli-002) Reject --region Coils combined with any non-boolean --type with a CommandException that names the constraint: coils carry a single bit, so only --type Bool is valid. Without this check a write like "--region Coils --type UInt16 --value 42" would silently coerce to a coil ON with no diagnostic. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Commands/WriteCommand.cs | 10 ++++++++++ 1 file changed, 10 insertions(+) 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 507daf5..a605bf3 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 @@ -60,6 +60,16 @@ public sealed class WriteCommand : ModbusCommandBase throw new CliFx.Exceptions.CommandException( $"Region '{Region}' is read-only in the Modbus spec; writes require Coils or HoldingRegisters."); + // Driver.Modbus.Cli-002: coils are single-bit outputs — only Bool makes sense. A + // non-boolean type (e.g. --region Coils --type UInt16) would silently coerce the value + // to a boolean via Convert.ToBoolean, landing as ON for any non-zero value, with no + // diagnostic. Reject it early so the operator sees a clear error rather than a silent + // type-mismatch coerce. + if (Region == ModbusRegion.Coils && DataType != ModbusDataType.Bool) + throw new CliFx.Exceptions.CommandException( + $"Region 'Coils' only supports boolean values (--type Bool). " + + $"Type '{DataType}' cannot represent a single-bit coil write."); + var tagName = ReadCommand.SynthesiseTagName(Region, Address, DataType); var tag = new ModbusTagDefinition( Name: tagName,