diff --git a/code-reviews/Driver.Cli.Common/findings.md b/code-reviews/Driver.Cli.Common/findings.md index e7f58b6..38897c2 100644 --- a/code-reviews/Driver.Cli.Common/findings.md +++ b/code-reviews/Driver.Cli.Common/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 6 | +| Open findings | 5 | ## Checklist coverage @@ -36,7 +36,7 @@ a category produced nothing rather than leaving it blank. | Severity | High | | Category | Correctness & logic bugs | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs:106-119` | -| Status | Open | +| Status | Resolved | **Description:** The `FormatStatus` shortlist maps four OPC UA status names to incorrect numeric codes. The correct OPC UA spec values (verified against the OPC Foundation @@ -67,7 +67,12 @@ depends on transitively) rather than hand-maintaining a hex shortlist, so the ta cannot drift from the spec again. If a hand-list is kept, add a test that cross-checks each entry against `Opc.Ua.StatusCodes` reflection. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — corrected the four mismapped `FormatStatus` codes +to their canonical `Opc.Ua.StatusCodes` values (`BadTimeout` 0x800A0000, `BadNoCommunication` +0x80310000, `BadWaitingForInitialData` 0x80320000, `BadNodeIdInvalid` 0x80330000); the CLI +project does not reference the `Opc.Ua` package so the hex literals were corrected in place +with a sync note, and `SnapshotFormatterTests` was updated with corrected expectations plus +a regression `[Theory]` asserting the pre-fix wrong names no longer apply. ### Driver.Cli.Common-002 diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs index c6c4b0e..2810d39 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/SnapshotFormatter.cs @@ -103,16 +103,18 @@ public static class SnapshotFormatter // Match the OPC UA shorthand for the statuses most-likely to land in a CLI run. // Anything outside this short-list surfaces as hex — operators can cross-reference // against OPC UA Part 6 § 7.34 (StatusCode tables) or Core.Abstractions status mappers. + // Numeric codes are the canonical values from the OPC Foundation Opc.Ua.StatusCodes + // table; keep them in sync with that table if this list is extended. var name = statusCode switch { 0x00000000u => "Good", 0x80000000u => "Bad", 0x80050000u => "BadCommunicationError", - 0x80060000u => "BadTimeout", - 0x80070000u => "BadNoCommunication", - 0x80080000u => "BadWaitingForInitialData", + 0x800A0000u => "BadTimeout", + 0x80310000u => "BadNoCommunication", + 0x80320000u => "BadWaitingForInitialData", 0x80340000u => "BadNodeIdUnknown", - 0x80350000u => "BadNodeIdInvalid", + 0x80330000u => "BadNodeIdInvalid", 0x80740000u => "BadTypeMismatch", 0x40000000u => "Uncertain", _ => null, diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs index 38a9a82..132d5e9 100644 --- a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common.Tests/SnapshotFormatterTests.cs @@ -25,15 +25,33 @@ public sealed class SnapshotFormatterTests } [Theory] + // Numeric codes are the canonical OPC Foundation Opc.Ua.StatusCodes values. [InlineData(0x00000000u, "Good")] [InlineData(0x80000000u, "Bad")] [InlineData(0x80050000u, "BadCommunicationError")] - [InlineData(0x80060000u, "BadTimeout")] + [InlineData(0x800A0000u, "BadTimeout")] + [InlineData(0x80310000u, "BadNoCommunication")] + [InlineData(0x80320000u, "BadWaitingForInitialData")] [InlineData(0x80340000u, "BadNodeIdUnknown")] + [InlineData(0x80330000u, "BadNodeIdInvalid")] + [InlineData(0x80740000u, "BadTypeMismatch")] [InlineData(0x40000000u, "Uncertain")] public void FormatStatus_names_well_known_status_codes(uint status, string expectedName) { - SnapshotFormatter.FormatStatus(status).ShouldContain(expectedName); + SnapshotFormatter.FormatStatus(status).ShouldBe($"0x{status:X8} ({expectedName})"); + } + + [Theory] + // Regression for Driver.Cli.Common-001: these codes were previously mapped to the + // wrong names. The hex values below are what the buggy shortlist used; they must + // now either resolve to their *correct* spec name or fall through to bare hex. + [InlineData(0x80060000u, "BadTimeout")] // was mislabelled BadTimeout + [InlineData(0x80070000u, "BadNoCommunication")] // was mislabelled BadNoCommunication + [InlineData(0x80080000u, "BadWaitingForInitialData")] // was mislabelled BadWaitingForInitialData + [InlineData(0x80350000u, "BadNodeIdInvalid")] // was mislabelled BadNodeIdInvalid + public void FormatStatus_does_not_apply_pre_fix_wrong_names(uint status, string wrongName) + { + SnapshotFormatter.FormatStatus(status).ShouldNotContain(wrongName); } [Fact]