dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(driver-modbus-cli): resolve Medium code-review finding (Driver.Modbus.Cli-001)

Browse Source 
Add --bit-index, --string-length, and --string-byte-order options to
SubscribeCommand, mirroring ReadCommand, and pass them into ModbusTagDefinition
so that BitInRegister and String type subscriptions use the correct bit index and
string length rather than silently defaulting to bit-0 / zero-length.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 09:55:10 -04:00
parent e7d7b6cb1d
commit 63e4a6baab
2 changed files with 24 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
code-reviews/Driver.Modbus.Cli/findings.md
View File

@@ -7,7 +7,7 @@
| Review date | 2026-05-22 |
| Commit reviewed | `76d35d1` |
| Status | Reviewed |
| Open findings | 8 |
| Open findings | 6 |
## Checklist coverage
@@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank.
| Severity | Medium |
| Category | Correctness & logic bugs |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/SubscribeCommand.cs:43-51` |
| Status | Open |
| Status | Resolved |
**Description:** `SubscribeCommand` synthesises its `ModbusTagDefinition` with only
`Name`, `Region`, `Address`, `DataType`, `Writable`, and `ByteOrder` — it never
@@ -54,7 +54,7 @@ options to `SubscribeCommand` (mirroring `ReadCommand`) and pass them into the
actually supports and reject `BitInRegister` / `String` at command entry with a
clear message.
**Resolution:** _(open)_
**Resolution:** Resolved 2026-05-22 — added `--bit-index`, `--string-length`, and `--string-byte-order` options to `SubscribeCommand`, mirroring `ReadCommand`, and passed them through to `ModbusTagDefinition` so `BitInRegister` and `String` types subscribe correctly.
### Driver.Modbus.Cli-002
@@ -63,7 +63,7 @@ clear message.
| Severity | Medium |
| Category | Correctness & logic bugs |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Cli/Commands/WriteCommand.cs:54-89` |
| Status | Open |
| Status | Resolved |
**Description:** `WriteCommand` rejects read-only regions (`DiscreteInputs` /
`InputRegisters`) but does not validate that `--type` is meaningful for the
@@ -78,7 +78,7 @@ region only supports `Bool`-style boolean values.
combined with any non-boolean `--type` (anything other than `Bool`), with a
message explaining coils carry a single bit.
**Resolution:** _(open)_
**Resolution:** Resolved 2026-05-22 — added a `Region == Coils && DataType != Bool` check immediately after the read-only-region guard, throwing `CommandException` with a message explaining that coils carry a single bit and only `--type Bool` is valid.
### Driver.Modbus.Cli-003