diff --git a/code-reviews/Driver.S7.Cli/findings.md b/code-reviews/Driver.S7.Cli/findings.md index 868bef2..48b14c9 100644 --- a/code-reviews/Driver.S7.Cli/findings.md +++ b/code-reviews/Driver.S7.Cli/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 7 | +| Open findings | 4 | ## Checklist coverage @@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank. | Severity | Medium | | Category | Error handling & resilience | | 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 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 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 @@ -63,7 +63,7 @@ offending value — matching the bool path. Update the test to expect `CommandEx | Severity | Medium | | 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` | -| Status | Open | +| Status | Resolved | **Description:** The `--type` option help text on `read`, `write`, and `subscribe` 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 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 @@ -92,7 +92,7 @@ options that cannot succeed. | Severity | Medium | | Category | Error handling & resilience | | 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 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 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