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

fix(driver-twincat): resolve Medium code-review finding (Driver.TwinCAT-003)

Browse Source 
Reject Structure-typed pre-declared tags in BuildTag at config-parse time
with a clear InvalidOperationException; replaces the previous silent
garbage read (MapToClrType fell through to typeof(int)) and late
NotSupportedException on writes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
2026-05-22 10:39:00 -04:00
parent a48b5396dc
commit de43690e0f
2 changed files with 21 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
code-reviews/Driver.TwinCAT/findings.md
View File

@@ -103,7 +103,7 @@ Discovery `ToDriverDataType` maps `Structure` to `String`, compounding the incon
does not support UDT tags, and `BrowseSymbolsAsync` already correctly yields
`DataType = null` for them.
**Resolution:** _(open)_
**Resolution:** Resolved 2026-05-22 — `BuildTag` now parses the `DataType` field first and rejects `TwinCATDataType.Structure` with an `InvalidOperationException` that names the tag and explains the limitation; configuration-time failure replaces the previous silent garbage read or late `NotSupportedException`.
### Driver.TwinCAT-004

23
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverFactoryExtensions.cs
View File

@@ -63,17 +63,34 @@ public static class TwinCATDriverFactoryExtensions
};
}
private static TwinCATTagDefinition BuildTag(TwinCATTagDto t, string driverInstanceId) =>
new(
private static TwinCATTagDefinition BuildTag(TwinCATTagDto t, string driverInstanceId)
{
var dataType = ParseEnum<TwinCATDataType>(t.DataType, t.Name, driverInstanceId, "DataType");
// Driver.TwinCAT-003: Structure-typed pre-declared tags are not supported. The driver's
// atomic surface cannot read/write UDT blobs — MapToClrType falls through to typeof(int)
// and ConvertForWrite throws NotSupportedException, producing garbage reads or late
// runtime failures. BrowseSymbolsAsync already correctly yields DataType = null for
// Structure symbols so they never appear in the discovered address space. Reject here
// with a clear error so operators get a configuration-time failure, not a silent wrong value.
if (dataType == TwinCATDataType.Structure)
throw new InvalidOperationException(
$"TwinCAT tag '{t.Name ?? "<unnamed>"}' in '{driverInstanceId}' specifies " +
"DataType 'Structure'. The driver does not support UDT/FB-instance pre-declared tags. " +
"Use EnableControllerBrowse to discover UDT members individually, or declare " +
"individual atomic-typed tags for the fields you need.");
return new TwinCATTagDefinition(
Name: t.Name ?? throw new InvalidOperationException(
$"TwinCAT config for '{driverInstanceId}' has a tag missing Name"),
DeviceHostAddress: t.DeviceHostAddress ?? throw new InvalidOperationException(
$"TwinCAT tag '{t.Name}' in '{driverInstanceId}' missing DeviceHostAddress"),
SymbolPath: t.SymbolPath ?? throw new InvalidOperationException(
$"TwinCAT tag '{t.Name}' in '{driverInstanceId}' missing SymbolPath"),
DataType: ParseEnum<TwinCATDataType>(t.DataType, t.Name, driverInstanceId, "DataType"),
DataType: dataType,
Writable: t.Writable ?? true,
WriteIdempotent: t.WriteIdempotent ?? false);
}
private static T ParseEnum<T>(string? raw, string? tagName, string driverInstanceId, string field)
where T : struct, Enum