diff --git a/code-reviews/Driver.TwinCAT/findings.md b/code-reviews/Driver.TwinCAT/findings.md index 3fc88c3..c528991 100644 --- a/code-reviews/Driver.TwinCAT/findings.md +++ b/code-reviews/Driver.TwinCAT/findings.md @@ -7,7 +7,7 @@ | Review date | 2026-05-22 | | Commit reviewed | `76d35d1` | | Status | Reviewed | -| Open findings | 11 | +| Open findings | 5 | ## Checklist coverage @@ -88,7 +88,7 @@ to `UInt16`, and `USInt`/`SInt` to their natural widths. Remove the stale "Int64 | Severity | Medium | | Category | Correctness & logic bugs | | Location | `AdsTwinCATClient.cs:264-281`, `283-300` | -| Status | Open | +| Status | Resolved | **Description:** `MapToClrType` has a `_ => typeof(int)` fallthrough and `ConvertForWrite` has a `_ => throw NotSupportedException` fallthrough. `TwinCATDataType.Structure` is a declared @@ -230,7 +230,7 @@ a dedicated managed task before invoking `OnChange`/`OnDataChange`, exactly as t | Severity | Medium | | Category | Concurrency & thread safety | | Location | `TwinCATDriver.cs:80-99`, `41-72`, `366-388` | -| Status | Open | +| Status | Resolved | **Description:** `ShutdownAsync` mutates `_devices`, `_tagsByName`, and `_nativeSubs` with no synchronization while `ReadAsync`/`WriteAsync`/`SubscribeAsync` may be iterating or indexing @@ -257,7 +257,7 @@ tasks before disposing `DeviceState`s. | Severity | Medium | | Category | Error handling & resilience | | Location | `AdsTwinCATClient.cs:178-195` | -| Status | Open | +| Status | Resolved | **Description:** `BrowseSymbolsAsync` checks `cancellationToken.IsCancellationRequested` and does `yield break` (a clean completion) rather than throwing `OperationCanceledException`. @@ -281,7 +281,7 @@ discovery. | Severity | Medium | | Category | Error handling & resilience | | Location | `TwinCATStatusMapper.cs:29-42` | -| Status | Open | +| Status | Resolved | **Description:** ADS error-code mapping has gaps and an inconsistency versus `docs/v2/driver-specs.md` section 6. The spec documents symbol-not-found as 0x0701 @@ -307,7 +307,7 @@ to `BadOutOfService`/`BadInvalidState`. | Severity | Medium | | Category | Performance & resource management | | Location | `TwinCATDriver.cs:102`, `AdsTwinCATClient.cs:178-195` | -| Status | Open | +| Status | Resolved | **Description:** `GetMemoryFootprint()` returns a hard-coded 0. `docs/v2/driver-stability.md` section "In-process only (Tier A/B) — driver-instance allocation tracking" requires the