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

docs(code-reviews): update Driver.S7 and Driver.S7.Cli findings status

Browse Source 
Mark Driver.S7-002, -004, -008, -012, -014 and Driver.S7.Cli-001, -002, -003
as Resolved; update Open findings counts (Driver.S7: 10→5, Driver.S7.Cli: 7→4).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 10:17:48 -04:00
parent 26e7b8140a
commit 607413e19f
1 changed files with 7 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
code-reviews/Driver.S7.Cli/findings.md
View File

@@ -7,7 +7,7 @@
| Review date | 2026-05-22 | | Review date | 2026-05-22 |
| Commit reviewed | `76d35d1` | | Commit reviewed | `76d35d1` |
| Status | Reviewed | | Status | Reviewed |
| Open findings | 7 | | Open findings | 4 |
## Checklist coverage ## Checklist coverage
@@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank.
| Severity | Medium | | Severity | Medium |
| Category | Error handling & resilience | | Category | Error handling & resilience |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/WriteCommand.cs:65-80` | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/WriteCommand.cs:65-80` |
| Status | Open | | Status | Resolved |
**Description:** `WriteCommand.ParseValue` parses numeric and `DateTime` values with the **Description:** `WriteCommand.ParseValue` parses numeric and `DateTime` values with the
raw BCL parsers (`short.Parse`, `float.Parse`, `DateTime.Parse`, etc.). On malformed raw BCL parsers (`short.Parse`, `float.Parse`, `DateTime.Parse`, etc.). On malformed
@@ -54,7 +54,7 @@ re-throws `FormatException` and `OverflowException` as
`CliFx.Exceptions.CommandException` with a message that names the `--type` and the `CliFx.Exceptions.CommandException` with a message that names the `--type` and the
offending value — matching the bool path. Update the test to expect `CommandException`. offending value — matching the bool path. Update the test to expect `CommandException`.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-22 — wrapped all numeric/DateTime BCL parses in `try/catch(FormatException)` and `try/catch(OverflowException)` that re-throw as `CommandException` with a message naming the `--type` and the offending value; updated `ParseValue_non_numeric_for_numeric_types_throws` to assert `CommandException`, and added an overflow-edge test.
### Driver.S7.Cli-002 ### Driver.S7.Cli-002
@@ -63,7 +63,7 @@ offending value — matching the bool path. Update the test to expect `CommandEx
| Severity | Medium | | Severity | Medium |
| Category | Design-document adherence | | Category | Design-document adherence |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ReadCommand.cs:22-29`, `Commands/WriteCommand.cs:21-33`, `Commands/SubscribeCommand.cs:18-21`; `docs/Driver.S7.Cli.md:70-73,80-81` | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ReadCommand.cs:22-29`, `Commands/WriteCommand.cs:21-33`, `Commands/SubscribeCommand.cs:18-21`; `docs/Driver.S7.Cli.md:70-73,80-81` |
| Status | Open | | Status | Resolved |
**Description:** The `--type` option help text on `read`, `write`, and `subscribe` **Description:** The `--type` option help text on `read`, `write`, and `subscribe`
advertises the full `S7DataType` set (`Int64 / UInt64 / Float64 / String / DateTime`), advertises the full `S7DataType` set (`Int64 / UInt64 / Float64 / String / DateTime`),
@@ -83,7 +83,7 @@ Float32`) until the follow-up driver PR lands, or (b) keep the surface but add a
help text and `docs/Driver.S7.Cli.md`. Option (a) is preferred so the CLI does not offer help text and `docs/Driver.S7.Cli.md`. Option (a) is preferred so the CLI does not offer
options that cannot succeed. options that cannot succeed.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-22 — updated the `--type` help text on `read`, `write`, and `subscribe` to list the implemented set (Bool/Byte/Int16/UInt16/Int32/UInt32/Float32) and appended a one-line caveat that Int64/UInt64/Float64/String/DateTime are not yet implemented and will return BadNotSupported.
### Driver.S7.Cli-003 ### Driver.S7.Cli-003
@@ -92,7 +92,7 @@ options that cannot succeed.
| Severity | Medium | | Severity | Medium |
| Category | Error handling & resilience | | Category | Error handling & resilience |
| Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ProbeCommand.cs:38-50` | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.S7.Cli/Commands/ProbeCommand.cs:38-50` |
| Status | Open | | Status | Resolved |
**Description:** `ProbeCommand` XML doc and the `Driver.S7.Cli.md` "fastest is the **Description:** `ProbeCommand` XML doc and the `Driver.S7.Cli.md` "fastest is the
device talking" framing say the probe "connects ... prints health" and "surfaces device talking" framing say the probe "connects ... prints health" and "surfaces
@@ -111,7 +111,7 @@ report derived from `driver.GetHealth()` (which `InitializeAsync` sets to
`Faulted` with the exception message before re-throwing). The probe should report an `Faulted` with the exception message before re-throwing). The probe should report an
unreachable device, not crash on it. unreachable device, not crash on it.
**Resolution:** _(open)_ **Resolution:** Resolved 2026-05-22 — wrapped the `InitializeAsync` + `ReadAsync` body in a `try/catch` that on any non-cancellation failure still prints the structured `Host:`, `CPU:`, `Health:`, and `Last error:` lines derived from `driver.GetHealth()`, so an unreachable device produces a health report rather than a stack trace.
### Driver.S7.Cli-004 ### Driver.S7.Cli-004