diff --git a/docs/drivers/S7-TIA-Import.md b/docs/drivers/S7-TIA-Import.md index 1dfb789..d1ada41 100644 --- a/docs/drivers/S7-TIA-Import.md +++ b/docs/drivers/S7-TIA-Import.md @@ -90,6 +90,98 @@ See [`docs/v2/s7.md` "UDT / STRUCT support"](../v2/s7.md#udt--struct-support) for the full fan-out semantics, the 4-level nesting cap, and the Optimized-block-access prerequisite. +## Instance DBs / FB parameters + +PR-S7-D3 / [#301](https://github.com/dohertj2/lmxopcua/issues/301) — multi-instance +Function-Block (FB) instances are addressed symbolically inside the PLC program +(`MyFB_Instance.MyParam`) but the runtime wire access still needs the absolute +`DBn.DBW_offset`. TIA Portal's "Show all tags" CSV export distinguishes these +rows from regular global DBs via the **`DB type`** column. + +### `DB type` column convention + +| `DB type` value | Meaning | Path | +|---|---|---| +| (empty) | Legacy export — no column at all (TIA pre-v15 / partial export). Treated as Global. | D1 (existing) | +| `Global DB` / `Global` / `Global Data Block` | Standalone DB declared in the project tree. | D1 (existing) | +| `Globaler Datenbaustein` | Same as above, DE locale. | D1 (existing) | +| `Instance DB` / `Instance` / `Instance Data Block` | Multi-instance FB instance. Member tags are the FB's `IN` / `OUT` / `IN_OUT` / `STAT` parameters. | **D3 (new)** | +| `Instance-DB` / `Instanz-DB` / `Instanz-Datenbaustein` | Same as above (locale + dashing variants). | **D3 (new)** | + +The `DB type` column is matched case-insensitively; quoting and surrounding +whitespace are tolerated. + +### `MyFB_Instance.MyParam` → `DBn.DBW_offset` + +The TIA Portal export ships the **resolved absolute address** in the +`Logical address` column for every instance-DB member — TIA itself walks the FB +interface declaration at export time and writes out the byte-offset-anchored +address verbatim. The importer accepts these rows the same way as a Global-DB +row, with two differences: + +1. The row counts under `S7ImportResult.InstanceDbCount` (a sub-counter of + `ParsedCount`) so the operator can see how much of the import depends on the + FB-interface layout. +2. The row is rejected from the UDT placeholder path even if the data type + column happens to match a UDT name pattern — instance-DB members always + import as fully-functional scalar tags. + +Example fixture row: + +```csv +Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type +MotorFB_1.Speed,FB instances,Int,%DB7.DBW0,Speed setpoint,True,Instance DB +``` + +The imported `S7TagDefinition` ends up with: + +```csharp +new S7TagDefinition( + Name: "MotorFB_1.Speed", + Address: "DB7.DBW0", + DataType: S7DataType.Int16, + Writable: true); +``` + +### Empty-`Logical address` fallback + +When TIA exports an instance-DB row with an empty `Logical address` column +(rare in practice — happens when the export was generated against a +not-yet-compiled project), `InstanceDbResolver` can compute the absolute +address from explicit parent-DB / parent-base-offset / member-offset inputs. +This fallback is exposed at the resolver-class level for advanced bootstrap +scenarios; the CSV path itself does not currently parse interface declarations +out of the file (TIA's CSV doesn't carry them). + +For now the operator workflow is: re-export from TIA after compiling the +project so every instance-DB row carries a resolved `Logical address`. + +### Re-import on FB-interface edit — caveat + +When the FB interface changes — a member is added, removed, or reordered in +TIA — the instance-DB layout shifts on the PLC side. Member byte offsets that +worked yesterday point at the wrong word today; absolute-offset addressing has +no in-band schema check. + +**The driver does not auto-detect this.** Operators must: + +1. Recompile the FB in TIA Portal. +2. Download the updated program to the PLC. +3. **Re-export "Show all tags" CSV** from the updated project. +4. Re-import the CSV via `AddTiaCsvImport` or the `import-symbols` CLI. +5. Restart the driver instance (Admin UI → Drivers → Reload). + +A stale import will silently read / write the wrong byte offsets — the values +will look like valid PLC data but reference whichever member used to live at +that offset before the interface edit. There is no runtime guard; this is the +same caveat that applies to all absolute-offset DB addressing on S7-1200 / +1500 (see [`docs/v2/s7.md` "UDT / STRUCT support"](../v2/s7.md#udt--struct-support) +for the parallel UDT-edit story). + +A future enhancement may add a project-fingerprint compare at driver init — +hashing the interface offsets at import time and re-checking against a known +PLC system function. Tracked as a follow-up; not in PR-S7-D3. + ## DE locale handling TIA Portal honours the Windows display locale when writing CSV. A DE-locale @@ -188,12 +280,14 @@ straight into the driver-instance config under `--emit summary` writes a single line: ``` -Imported 142 tag(s), skipped 3, errors 0, udt-placeholders 5. +Imported 142 tag(s), skipped 3, errors 0, udt-placeholders 5, instance-db 9. ``` `Skipped` covers HMI-accessible-false rows + missing-required-field rows; `errors` covers rows whose `Address` failed to parse as an S7 address; -`udt-placeholders` covers UDT-typed rows that imported as placeholders. +`udt-placeholders` covers UDT-typed rows that imported as placeholders; +`instance-db` (PR-S7-D3) covers rows whose `DB type` column tagged them as +multi-instance FB-instance members. ## API surface — `IS7SymbolImporter` + `AddTiaCsvImport` / `AddAwlImport` diff --git a/docs/v2/s7.md b/docs/v2/s7.md index 5084404..ef89f5e 100644 --- a/docs/v2/s7.md +++ b/docs/v2/s7.md @@ -1044,6 +1044,30 @@ The fan-out rejects, with clear errors: - Tag with `UdtName` AND `ElementCount > 1` (array-of-UDT belongs in the UDT layout, not at the parent-tag level) +### Re-import on UDT / FB-interface edit — caveat + +The static-offset model assumes the declared layout matches the runtime +layout exactly. When the underlying UDT or FB interface changes in TIA Portal +— a member added, removed, or reordered — the byte offsets shift on the PLC +side and the cached `S7UdtDefinition` / instance-DB addresses point at the +wrong member. + +**The driver does not auto-detect interface drift.** After any UDT edit or +multi-instance-FB interface edit on the PLC side, the operator must: + +1. Recompile + download the updated program in TIA Portal. +2. Re-export "Show all tags" CSV from the updated project. +3. Re-import via `AddTiaCsvImport` (or `import-symbols` CLI) and update the + matching `S7UdtDefinition` declarations to mirror the new offsets. +4. Restart the driver instance (Admin UI → Drivers → Reload). + +A stale UDT layout will silently read / write the wrong byte offsets — the +values will look like valid PLC data but reference whichever member used to +live at that offset before the edit. The same caveat applies to multi-instance +FB-instance DBs imported via PR-S7-D3 / [#301](https://github.com/dohertj2/lmxopcua/issues/301); +see [`docs/drivers/S7-TIA-Import.md` "Re-import on FB-interface edit"](../drivers/S7-TIA-Import.md#re-import-on-fb-interface-edit--caveat) +for the FB-instance-specific workflow. + ## References 1. Siemens Industry Online Support, *Modbus/TCP Communication between SIMATIC S7-1500 / S7-1200 and Modbus/TCP Controllers with Instructions `MB_CLIENT` and `MB_SERVER`*, Entry ID 102020340, V6 (Feb 2021). https://cache.industry.siemens.com/dl/files/340/102020340/att_118119/v6/net_modbus_tcp_s7-1500_s7-1200_en.pdf diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/InstanceDbResolver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/InstanceDbResolver.cs new file mode 100644 index 0000000..fe15dae --- /dev/null +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/InstanceDbResolver.cs @@ -0,0 +1,226 @@ +using System.Globalization; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.SymbolImport; + +/// +/// PR-S7-D3 / #301 — recognise and resolve TIA Portal CSV rows that belong to a +/// multi-instance Function-Block instance DB. Multi-instance FBs are addressed +/// symbolically (MyFB_Instance.MyParam) inside the PLC program, but the runtime +/// wire access still needs the absolute DBn.DBW_offset. TIA's "Show all tags" +/// CSV export tags these rows with a different DB type column value +/// (locale-variant: Instance DB / Instance / Instance data block) +/// and — most of the time — already supplies the resolved absolute address in the +/// Logical address column. +/// +/// +/// +/// Two-phase model: +/// +/// +/// DB type column == Instance DB (or locale variant) AND +/// Logical address non-empty → accept verbatim, normalise the address +/// the same way as a Global-DB row, count it under +/// . +/// +/// +/// DB type column == Instance DB AND Logical address +/// empty BUT a parent FB-interface declaration is reachable → compute +/// DBn.DBW(parentOffset + memberOffset) from the interface table. +/// The pure-compute fallback is rare in practice (TIA almost always emits the +/// resolved address) but is required by the PR plan. +/// +/// +/// +/// +/// Re-import on FB-interface edit: when the FB interface changes (member +/// added, removed, or reordered in TIA), the instance-DB layout shifts. Operators +/// must re-import the CSV after any such edit; absolute offsets that cached against +/// the previous layout will silently point at the wrong member otherwise. See +/// docs/drivers/S7-TIA-Import.md "Instance DBs / FB parameters" section. +/// +/// +public sealed class InstanceDbResolver +{ + private readonly ILogger _logger; + + public InstanceDbResolver() : this(NullLogger.Instance) { } + + public InstanceDbResolver(ILogger logger) + { + _logger = logger ?? NullLogger.Instance; + } + + /// + /// Recognise an arbitrary DB type column value as identifying an + /// instance-DB row. Accepts the canonical TIA en-US value Instance DB, + /// the bare Instance, the German Instanz-Datenbaustein / + /// Instanz-DB, and a few other common locale variants. The match is + /// case-insensitive and tolerates surrounding whitespace + quotes. + /// + public static bool IsInstanceDbType(string? dbTypeColumn) + { + if (string.IsNullOrWhiteSpace(dbTypeColumn)) return false; + var v = dbTypeColumn.Trim().Trim('"').Trim().ToLowerInvariant(); + return v switch + { + "instance" => true, + "instance db" => true, + "instance-db" => true, + "instance data block" => true, + // German + dashed variants TIA emits when the project locale is DE. + "instanz" => true, + "instanz-db" => true, + "instanz db" => true, + "instanz-datenbaustein" => true, + "instanz datenbaustein" => true, + _ => false, + }; + } + + /// + /// Recognise the DB type column value as identifying a Global / standard + /// data block (the default that PR-S7-D1 already handles). Useful for explicit + /// downstream branching — anything that is neither Global nor Instance is logged + /// and treated as Global so the existing pipeline keeps working. + /// + public static bool IsGlobalDbType(string? dbTypeColumn) + { + if (string.IsNullOrWhiteSpace(dbTypeColumn)) return false; + var v = dbTypeColumn.Trim().Trim('"').Trim().ToLowerInvariant(); + return v switch + { + "global" or "global db" or "global-db" or "global data block" => true, + "globaler datenbaustein" or "globaler-datenbaustein" or "global-datenbaustein" => true, + _ => false, + }; + } + + /// + /// Resolve an instance-DB row into an . Accepts both + /// the canonical "address-already-resolved" form (the common case) and the + /// "compute from interface offsets" form for exports that ship offsets only. + /// + /// Raw row state — Name, resolved address (if any), declared data type, parent FB info. + /// On success, the materialised tag definition; address is normalised (no leading %) the same way does it. + /// true if the row was recognised AND resolved; false when neither a resolved address nor sufficient interface info was available. + public bool TryResolve(InstanceDbRow row, out S7TagDefinition? resolved) + { + resolved = null; + ArgumentNullException.ThrowIfNull(row); + + // Path 1 — address already resolved by TIA (the common case; the export ships + // %DB7.DBW0 verbatim for a multi-instance FB member). + if (!string.IsNullOrWhiteSpace(row.LogicalAddress)) + { + var normalised = TiaCsvImporter.NormaliseAddress(row.LogicalAddress, row.DeLocale); + if (!S7AddressParser.TryParse(normalised, out _)) + { + _logger.LogWarning( + "InstanceDbResolver: row '{Name}' has unparseable address '{Address}' (DB type='{DbType}').", + row.Name, normalised, row.DbType); + return false; + } + + if (!TiaCsvImporter.TryResolveDataType(row.DataType ?? string.Empty, normalised, out var s7Type)) + { + _logger.LogWarning( + "InstanceDbResolver: row '{Name}' has unrecognised Data type '{DataType}' for address '{Address}'.", + row.Name, row.DataType, normalised); + return false; + } + + resolved = new S7TagDefinition( + Name: row.Name, + Address: normalised, + DataType: s7Type, + Writable: true, + StringLength: row.StringLength ?? 254); + return true; + } + + // Path 2 — compute the absolute address from the FB interface declaration. + // Required only when the TIA export ships interface offsets without resolved + // addresses; rare in practice but in the PR plan. + if (row.ParentDbNumber is int dbNumber && row.MemberOffset is int memberOffset) + { + if (!TiaCsvImporter.TryResolveDataType(row.DataType ?? string.Empty, string.Empty, out var s7Type)) + { + _logger.LogWarning( + "InstanceDbResolver: row '{Name}' has unrecognised Data type '{DataType}' (interface-resolved path).", + row.Name, row.DataType); + return false; + } + + var sizeLetter = SizeLetterFor(s7Type); + var absOffset = (row.ParentBaseOffset ?? 0) + memberOffset; + var address = $"DB{dbNumber.ToString(CultureInfo.InvariantCulture)}.DB{sizeLetter}{absOffset.ToString(CultureInfo.InvariantCulture)}"; + + if (s7Type == S7DataType.Bool && row.MemberBitOffset is int bitOffset) + { + address = $"DB{dbNumber.ToString(CultureInfo.InvariantCulture)}.DBX{absOffset.ToString(CultureInfo.InvariantCulture)}.{bitOffset.ToString(CultureInfo.InvariantCulture)}"; + } + + if (!S7AddressParser.TryParse(address, out _)) + { + _logger.LogWarning( + "InstanceDbResolver: row '{Name}' computed unparseable address '{Address}'.", + row.Name, address); + return false; + } + + resolved = new S7TagDefinition( + Name: row.Name, + Address: address, + DataType: s7Type, + Writable: true, + StringLength: row.StringLength ?? 254); + return true; + } + + _logger.LogWarning( + "InstanceDbResolver: row '{Name}' has no resolved address and insufficient interface info to compute one (DB type='{DbType}').", + row.Name, row.DbType); + return false; + } + + private static char SizeLetterFor(S7DataType t) => t switch + { + S7DataType.Bool => 'X', + S7DataType.Byte or S7DataType.Char => 'B', + S7DataType.Int16 or S7DataType.UInt16 or S7DataType.WChar or S7DataType.Date => 'W', + S7DataType.Int32 or S7DataType.UInt32 or S7DataType.Float32 + or S7DataType.Time or S7DataType.TimeOfDay => 'D', + S7DataType.Int64 or S7DataType.UInt64 or S7DataType.Float64 => 'D', // S7-1500 LWord lives in DBD-stride pairs + // STRING / WSTRING / DTL / DT / S5Time map to byte access — the codec slices client-side. + _ => 'B', + }; +} + +/// +/// PR-S7-D3 — input row to . Carries +/// everything the resolver needs to either accept a TIA-resolved address verbatim +/// or compute one from FB-interface offsets. +/// +/// Tag name; same semantics as . +/// Raw DB type column value as it appeared in the CSV — used in diagnostic logs only. +/// TIA Logical address column. May be empty when the export ships only interface offsets. +/// TIA Data type column (primitive type name). +/// DE-locale flag forwarded from the importer so address normalisation rewrites comma-decimals. +/// Optional Length column for STRING tags; null = default 254. +/// For the interface-offset path: the DB number the FB instance lives in. +/// For the interface-offset path: the FB instance's base byte offset within the DB. Defaults to 0 when null. +/// For the interface-offset path: the member's byte offset within the FB interface. +/// For the interface-offset path: the bit offset for BOOL members; null for non-BOOL members. +public sealed record InstanceDbRow( + string Name, + string? DbType, + string? LogicalAddress, + string? DataType, + bool DeLocale = false, + int? StringLength = null, + int? ParentDbNumber = null, + int? ParentBaseOffset = null, + int? MemberOffset = null, + int? MemberBitOffset = null); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/S7ImportResult.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/S7ImportResult.cs index 7c120e7..2eb0be5 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/S7ImportResult.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/S7ImportResult.cs @@ -3,18 +3,25 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.S7.SymbolImport; /// /// Outcome of a single run. carries /// the imported tag definitions ready to drop into S7DriverOptions.Tags; -/// , , , and -/// give the operator a single line of telemetry -/// ("imported 142 / skipped 3 / errored 0 / udt 5") suitable for either a CLI summary -/// or a startup-time log line. +/// , , , +/// , and give the operator +/// a single line of telemetry ("imported 142 / skipped 3 / errored 0 / udt 5 / instance-db 9") +/// suitable for either a CLI summary or a startup-time log line. /// /// /// -/// includes UDT placeholders — placeholders count as -/// imported tags (they materialise as rows the driver -/// can list in the Admin UI), they're just non-functional until PR-S7-D2 lands. -/// is a sub-count operators can compare against -/// to spot how much of the import is still placeholder. +/// includes UDT placeholders and instance-DB rows — +/// both count as imported tags (they materialise as +/// rows the driver can list in the Admin UI). +/// and are sub-counts operators can compare against +/// to spot how the import breaks down. +/// +/// +/// (PR-S7-D3 / #301) tracks rows whose DB type +/// column identified them as belonging to a multi-instance FB-instance DB. They +/// import as fully-functional tags — the count is informational so an operator +/// knows how many of the imported rows came from FB-instance DBs (and therefore +/// need a re-import after any FB-interface edit on the PLC side). /// /// public sealed record S7ImportResult( @@ -22,4 +29,5 @@ public sealed record S7ImportResult( int ParsedCount, int SkippedCount, int ErrorCount, - int UdtPlaceholderCount); + int UdtPlaceholderCount, + int InstanceDbCount = 0); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/TiaCsvImporter.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/TiaCsvImporter.cs index ed25768..a5b2a5f 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/TiaCsvImporter.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.S7/SymbolImport/TiaCsvImporter.cs @@ -65,6 +65,9 @@ public sealed class TiaCsvImporter : IS7SymbolImporter var skipped = 0; var errors = 0; var udtPlaceholders = 0; + var instanceDbCount = 0; + + var resolver = new InstanceDbResolver(); using var reader = new StreamReader(stream, Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 4096, leaveOpen: true); @@ -75,6 +78,7 @@ public sealed class TiaCsvImporter : IS7SymbolImporter int? commentIdx = null; int? hmiAccessibleIdx = null; int? lengthIdx = null; + int? dbTypeIdx = null; var headerSeen = false; var deLocale = false; var lineNumber = 0; @@ -125,6 +129,14 @@ public sealed class TiaCsvImporter : IS7SymbolImporter case "hmi accessible": case "hmi-accessible": hmiAccessibleIdx = i; break; case "length": lengthIdx = i; break; + // PR-S7-D3 — TIA Portal exports include a "DB type" column + // that distinguishes Global DBs (already handled by D1) from + // multi-instance FB instance DBs (the new bucket). DE locale + // emits "Datenbaustein-Typ"; both header forms are accepted. + case "db type": + case "db-type": + case "datenbaustein-typ": + case "datenbausteintyp": dbTypeIdx = i; break; } } @@ -170,6 +182,7 @@ public sealed class TiaCsvImporter : IS7SymbolImporter ? ParseBoolColumn(SafeField(fields, hmiAccessibleIdx.Value), defaultValue: true) : true; var lengthStr = lengthIdx.HasValue ? SafeField(fields, lengthIdx.Value).Trim().Trim('"') : null; + var dbTypeStr = dbTypeIdx.HasValue ? SafeField(fields, dbTypeIdx.Value).Trim().Trim('"') : null; if (string.IsNullOrWhiteSpace(name) || string.IsNullOrWhiteSpace(address)) { @@ -190,6 +203,49 @@ public sealed class TiaCsvImporter : IS7SymbolImporter // to '.' so the rest of the pipeline sees a single canonical address shape. var normalised = NormaliseAddress(address, deLocale); + // PR-S7-D3 — recognise instance-DB rows (multi-instance FB members) BEFORE + // the UDT detection runs. TIA's CSV typically already ships the resolved + // absolute address (%DB7.DBW0) so the data type column is a primitive — but + // the row still wants to land in the InstanceDbCount sub-counter so the + // operator knows how much of the import depends on the FB-interface layout. + // (Re-import is required after any FB-interface edit; see docs.) + if (InstanceDbResolver.IsInstanceDbType(dbTypeStr)) + { + var idbRow = new InstanceDbRow( + Name: name, + DbType: dbTypeStr, + LogicalAddress: address, // raw address — resolver normalises internally + DataType: dataTypeStr, + DeLocale: deLocale, + StringLength: int.TryParse(lengthStr, NumberStyles.Integer, CultureInfo.InvariantCulture, out var idbLen) && idbLen > 0 + ? idbLen + : null); + + if (resolver.TryResolve(idbRow, out var idbTag) && idbTag is not null) + { + tags.Add(idbTag); + instanceDbCount++; + parsed++; + _logger.LogInformation( + "TIA CSV row at line {LineNumber} accepted as instance-DB member (Name='{Name}', Address='{Address}').", + lineNumber, idbTag.Name, idbTag.Address); + continue; + } + + // Resolver rejected the row — fall through to the strict-error path + // so the outer try/catch consistently increments errors / skips. + if (!opts.IgnoreInvalid) + { + throw new InvalidDataException( + $"TIA CSV row at line {lineNumber} flagged as instance-DB but failed to resolve (Name='{name}', address='{address}')."); + } + errors++; + _logger.LogWarning( + "TIA CSV row at line {LineNumber} skipped — instance-DB row failed to resolve (Name='{Name}', address='{Address}').", + lineNumber, name, address); + continue; + } + // Detect UDT placeholder before the strict address parser runs. UDTs in TIA // typically address as DBn (whole DB) and don't carry a width suffix; the // address parser would reject them. @@ -271,10 +327,10 @@ public sealed class TiaCsvImporter : IS7SymbolImporter if (!headerSeen) { - return new S7ImportResult([], 0, skipped, errors, udtPlaceholders); + return new S7ImportResult([], 0, skipped, errors, udtPlaceholders, instanceDbCount); } - return new S7ImportResult(tags, parsed, skipped, errors, udtPlaceholders); + return new S7ImportResult(tags, parsed, skipped, errors, udtPlaceholders, instanceDbCount); } /// diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Fixtures/sample_tia_export_with_fb_instance.csv b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Fixtures/sample_tia_export_with_fb_instance.csv new file mode 100644 index 0000000..19bcff9 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Fixtures/sample_tia_export_with_fb_instance.csv @@ -0,0 +1,10 @@ +Name,Path,Data type,Logical address,Comment,Hmi accessible,Hmi visible,Hmi writeable,Length,DB type +ProbeWord,Default tag table,UInt,%MW0,Probe word for liveness,True,True,True,, +GlobalSetpoint,Default tag table,Int,%DB1.DBW10,Standalone global-DB tag,True,True,True,,Global DB +GlobalCounter,Default tag table,DInt,%DB1.DBD20,Standalone global-DB tag,True,True,True,,Global DB +MotorFB_Inst.Speed,FB instances,Int,%DB1.DBW10,FB-instance member resolved into DB1 layout,True,True,True,,Instance DB +MotorFB_Inst.Setpoint,FB instances,Real,%DB1.DBD30,FB-instance member resolved into DB1 layout,True,True,True,,Instance DB +MotorFB_Inst.Enabled,FB instances,Bool,%DB1.DBX50.3,FB-instance member resolved into DB1 layout,True,True,True,,Instance DB +PumpFB_Inst.Counter,FB instances,DInt,%DB1.DBD20,Second FB-instance member,True,True,True,,Instance DB +PumpFB_Inst.WriteScratch,FB instances,UInt,%DB1.DBW100,Second FB-instance member — writable scratch,True,True,True,,Instance DB +HiddenInternal,Default tag table,Int,%MW100,Internal symbol — should be filtered,False,False,False,, diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/SymbolImport/InstanceDbImportIntegrationTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/SymbolImport/InstanceDbImportIntegrationTests.cs new file mode 100644 index 0000000..b55d6b5 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/SymbolImport/InstanceDbImportIntegrationTests.cs @@ -0,0 +1,96 @@ +using System.IO; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.S7_1500; +using ZB.MOM.WW.OtOpcUa.Driver.S7.SymbolImport; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.SymbolImport; + +/// +/// PR-S7-D3 / #301 — golden-fixture integration test for instance-DB / FB parameter +/// resolution. Loads Fixtures/sample_tia_export_with_fb_instance.csv, materialises +/// a driver-options object via , +/// and exercises the runtime read path against the python-snap7 simulator using the +/// resolved instance-DB addresses. +/// +/// +/// +/// The fixture mixes three categories so a single import covers the full +/// DB type column matrix: +/// +/// Two global-DB tags (DB type = Global DB) — pre-existing D1 path. +/// Five instance-DB tags (DB type = Instance DB) — new D3 path. +/// One HMI-hidden row — must be filtered. +/// One M-area probe (no DB type) — global by default. +/// +/// +/// +/// The instance-DB tag addresses deliberately overlap with the snap7 seed offsets +/// baked into so a successful round-trip proves the +/// resolver normalises addresses identically to the global-DB path. The simulator +/// doesn't know (or care) that a tag came from an FB-instance row — what we're +/// testing is that the importer recognised it and the driver +/// accepted the resolved address verbatim. +/// +/// +/// Auto-skips when no simulator is reachable (build-only on hosts without snap7). +/// +/// +[Collection(Snap7ServerCollection.Name)] +[Trait("Category", "Integration")] +[Trait("Device", "S7_1500")] +public sealed class InstanceDbImportIntegrationTests(Snap7ServerFixture sim) +{ + private static string FixturePath(string name) => + Path.Combine(AppContext.BaseDirectory, "Fixtures", name); + + [Fact] + public async Task Driver_resolves_fb_instance_then_reads_seeded_member() + { + if (sim.SkipReason is not null) Assert.Skip(sim.SkipReason); + + var baseOptions = new S7DriverOptions + { + Host = sim.Host, + Port = sim.Port, + CpuType = global::S7.Net.CpuType.S71500, + Timeout = TimeSpan.FromSeconds(5), + Probe = new S7ProbeOptions { Enabled = false }, + Tags = [], + }; + + var options = baseOptions.AddTiaCsvImport( + FixturePath("sample_tia_export_with_fb_instance.csv"), + out var importResult); + + // Fixture: 9 rows total = 1 probe + 2 global-DB + 5 instance-DB + 1 hidden. + // Hidden filtered → SkippedCount = 1; everything else lands. + importResult.ParsedCount.ShouldBe(8); + importResult.SkippedCount.ShouldBe(1); + importResult.InstanceDbCount.ShouldBe(5); + importResult.UdtPlaceholderCount.ShouldBe(0); + importResult.ErrorCount.ShouldBe(0); + options.Tags.Count.ShouldBe(8); + + await using var drv = new S7Driver(options, driverInstanceId: "s7-tia-import-fb"); + await drv.InitializeAsync("{}", TestContext.Current.CancellationToken); + + // Read three resolved instance-DB members. Each address overlaps a snap7 seed in + // S7_1500Profile, so a successful round-trip proves the resolver pointed the + // driver at the right absolute byte offset. + var snapshots = await drv.ReadAsync( + ["MotorFB_Inst.Speed", "MotorFB_Inst.Setpoint", "MotorFB_Inst.Enabled"], + TestContext.Current.CancellationToken); + + snapshots.Count.ShouldBe(3); + foreach (var s in snapshots) + s.StatusCode.ShouldBe(0u, "instance-DB resolved address must read end-to-end"); + + // Speed sits at DB1.DBW10 → SmokeI16Seed; Setpoint at DB1.DBD30 → SmokeF32Seed; + // Enabled at DB1.DBX50.3 → SmokeBool. + Convert.ToInt32(snapshots[0].Value).ShouldBe((int)S7_1500Profile.SmokeI16SeedValue); + Convert.ToSingle(snapshots[1].Value).ShouldBe(S7_1500Profile.SmokeF32SeedValue, tolerance: 0.0001f); + Convert.ToBoolean(snapshots[2].Value).ShouldBeTrue(); + } +} diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.csproj b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.csproj index 6c5df2e..07432ec 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.csproj +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests.csproj @@ -30,6 +30,8 @@ + + diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/SymbolImport/InstanceDbResolverTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/SymbolImport/InstanceDbResolverTests.cs new file mode 100644 index 0000000..23555be --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/SymbolImport/InstanceDbResolverTests.cs @@ -0,0 +1,267 @@ +using System.IO; +using System.Text; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.S7.SymbolImport; + +namespace ZB.MOM.WW.OtOpcUa.Driver.S7.Tests.SymbolImport; + +/// +/// Unit coverage for and 's +/// instance-DB recognition path (PR-S7-D3 / #301). The resolver-level cases drive the +/// class directly with synthesised inputs; the importer-level +/// cases drive the parser through in-memory CSV streams that mix Global + Instance rows +/// to confirm both buckets land correctly and that +/// tracks just the instance-DB rows. +/// +[Trait("Category", "Unit")] +public sealed class InstanceDbResolverTests +{ + private static S7ImportResult ParseString(string csv, S7ImportOptions? opts = null) + { + var importer = new TiaCsvImporter(NullLogger.Instance); + using var stream = new MemoryStream(Encoding.UTF8.GetBytes(csv)); + return importer.Parse(stream, opts); + } + + [Fact] + public void IsInstanceDbType_recognises_canonical_and_locale_variants() + { + // en-US canonical shapes — TIA's "Show all tags" CSV emits any of these depending on + // export options and TIA Portal version. + InstanceDbResolver.IsInstanceDbType("Instance DB").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("instance db").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("Instance").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("Instance Data Block").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("instance-db").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("\"Instance DB\"").ShouldBeTrue(); // quotes survive + // German export variants. + InstanceDbResolver.IsInstanceDbType("Instanz-DB").ShouldBeTrue(); + InstanceDbResolver.IsInstanceDbType("Instanz-Datenbaustein").ShouldBeTrue(); + // Negative cases — Global rows must NOT be confused for instance DBs. + InstanceDbResolver.IsInstanceDbType("Global DB").ShouldBeFalse(); + InstanceDbResolver.IsInstanceDbType("Global").ShouldBeFalse(); + InstanceDbResolver.IsInstanceDbType("").ShouldBeFalse(); + InstanceDbResolver.IsInstanceDbType(null).ShouldBeFalse(); + } + + [Fact] + public void IsGlobalDbType_recognises_canonical_and_locale_variants() + { + InstanceDbResolver.IsGlobalDbType("Global DB").ShouldBeTrue(); + InstanceDbResolver.IsGlobalDbType("global").ShouldBeTrue(); + InstanceDbResolver.IsGlobalDbType("Global Data Block").ShouldBeTrue(); + InstanceDbResolver.IsGlobalDbType("Globaler Datenbaustein").ShouldBeTrue(); + InstanceDbResolver.IsGlobalDbType("Instance DB").ShouldBeFalse(); + InstanceDbResolver.IsGlobalDbType("").ShouldBeFalse(); + } + + [Fact] + public void TryResolve_with_resolved_address_returns_tag_at_that_address() + { + var resolver = new InstanceDbResolver(NullLogger.Instance); + var row = new InstanceDbRow( + Name: "MotorFB_1.Speed", + DbType: "Instance DB", + LogicalAddress: "%DB7.DBW0", + DataType: "Int"); + + resolver.TryResolve(row, out var tag).ShouldBeTrue(); + tag.ShouldNotBeNull(); + tag!.Name.ShouldBe("MotorFB_1.Speed"); + tag.Address.ShouldBe("DB7.DBW0"); + tag.DataType.ShouldBe(S7DataType.Int16); + } + + [Fact] + public void TryResolve_with_empty_address_but_interface_offsets_computes_address() + { + // FB interface declares Speed at member offset 4 within an FB instance whose base + // sits at offset 8 inside DB12. Resolver computes DB12.DBW(8 + 4) = DB12.DBW12. + var resolver = new InstanceDbResolver(NullLogger.Instance); + var row = new InstanceDbRow( + Name: "PumpFB_2.Speed", + DbType: "Instance DB", + LogicalAddress: null, + DataType: "Int", + ParentDbNumber: 12, + ParentBaseOffset: 8, + MemberOffset: 4); + + resolver.TryResolve(row, out var tag).ShouldBeTrue(); + tag.ShouldNotBeNull(); + tag!.Address.ShouldBe("DB12.DBW12"); + tag.DataType.ShouldBe(S7DataType.Int16); + } + + [Fact] + public void TryResolve_with_empty_address_for_bool_uses_bit_offset() + { + // BOOL members carry a bit offset; resolver builds DBn.DBX{byte}.{bit}. + var resolver = new InstanceDbResolver(NullLogger.Instance); + var row = new InstanceDbRow( + Name: "PumpFB_2.Enabled", + DbType: "Instance DB", + LogicalAddress: null, + DataType: "Bool", + ParentDbNumber: 12, + ParentBaseOffset: 0, + MemberOffset: 6, + MemberBitOffset: 3); + + resolver.TryResolve(row, out var tag).ShouldBeTrue(); + tag.ShouldNotBeNull(); + tag!.Address.ShouldBe("DB12.DBX6.3"); + tag.DataType.ShouldBe(S7DataType.Bool); + } + + [Fact] + public void TryResolve_with_no_address_and_no_interface_info_fails() + { + var resolver = new InstanceDbResolver(NullLogger.Instance); + var row = new InstanceDbRow( + Name: "Orphan.Member", + DbType: "Instance DB", + LogicalAddress: null, + DataType: "Int"); + + resolver.TryResolve(row, out var tag).ShouldBeFalse(); + tag.ShouldBeNull(); + } + + [Fact] + public void TryResolve_with_unparseable_address_fails() + { + var resolver = new InstanceDbResolver(NullLogger.Instance); + var row = new InstanceDbRow( + Name: "Garbage", + DbType: "Instance DB", + LogicalAddress: "%XYZ-not-an-address", + DataType: "Int"); + + resolver.TryResolve(row, out var tag).ShouldBeFalse(); + tag.ShouldBeNull(); + } + + [Fact] + public void Parse_five_instance_db_rows_with_resolved_addresses_yield_five_tags() + { + // Five instance-DB members under the same MotorFB_Instance, fully resolved by TIA. + const string csv = """ + Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type + MotorFB_1.Speed,FB,Int,%DB7.DBW0,Speed setpoint,True,Instance DB + MotorFB_1.Accel,FB,Int,%DB7.DBW2,Acceleration ramp,True,Instance DB + MotorFB_1.Setpoint,FB,Real,%DB7.DBD4,Position setpoint,True,Instance DB + MotorFB_1.Enabled,FB,Bool,%DB7.DBX8.0,Drive enabled bit,True,Instance DB + MotorFB_1.Fault,FB,Bool,%DB7.DBX8.1,Fault bit,True,Instance DB + """; + + var result = ParseString(csv); + result.ParsedCount.ShouldBe(5); + result.InstanceDbCount.ShouldBe(5); + result.UdtPlaceholderCount.ShouldBe(0); + result.SkippedCount.ShouldBe(0); + result.ErrorCount.ShouldBe(0); + + result.Tags[0].Address.ShouldBe("DB7.DBW0"); + result.Tags[0].DataType.ShouldBe(S7DataType.Int16); + result.Tags[1].Address.ShouldBe("DB7.DBW2"); + result.Tags[2].Address.ShouldBe("DB7.DBD4"); + result.Tags[2].DataType.ShouldBe(S7DataType.Float32); + result.Tags[3].Address.ShouldBe("DB7.DBX8.0"); + result.Tags[3].DataType.ShouldBe(S7DataType.Bool); + result.Tags[4].Address.ShouldBe("DB7.DBX8.1"); + } + + [Fact] + public void Parse_mixed_global_and_instance_rows_handles_both() + { + // Two global-DB rows (no DB type column → treated as Global by D1) + three + // instance-DB rows (DB type=Instance DB → routed through the resolver). + const string csv = """ + Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type + ProbeWord,Tags,UInt,%MW0,Probe,True, + Setpoint,Tags,Real,%DB1.DBD4,Setpoint,True,Global DB + FB1.Speed,FB,Int,%DB7.DBW0,FB speed,True,Instance DB + FB1.Accel,FB,Int,%DB7.DBW2,FB accel,True,Instance DB + FB1.Enabled,FB,Bool,%DB7.DBX8.0,FB enabled,True,Instance DB + """; + + var result = ParseString(csv); + result.ParsedCount.ShouldBe(5); + result.InstanceDbCount.ShouldBe(3); + result.SkippedCount.ShouldBe(0); + result.ErrorCount.ShouldBe(0); + result.UdtPlaceholderCount.ShouldBe(0); + + // First two rows are global; last three are instance. + result.Tags[0].Name.ShouldBe("ProbeWord"); + result.Tags[1].Name.ShouldBe("Setpoint"); + result.Tags[2].Name.ShouldBe("FB1.Speed"); + result.Tags[3].Name.ShouldBe("FB1.Accel"); + result.Tags[4].Name.ShouldBe("FB1.Enabled"); + } + + [Fact] + public void Parse_instance_db_with_locale_variants_are_all_recognised() + { + // Verify every locale variant of the DB type column gets routed through the + // resolver. One row per variant; all five must land in InstanceDbCount. + const string csv = """ + Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type + T1,FB,Int,%DB10.DBW0,t1,True,Instance + T2,FB,Int,%DB10.DBW2,t2,True,Instance DB + T3,FB,Int,%DB10.DBW4,t3,True,Instance-DB + T4,FB,Int,%DB10.DBW6,t4,True,Instance Data Block + T5,FB,Int,%DB10.DBW8,t5,True,Instanz-DB + """; + + var result = ParseString(csv); + result.ParsedCount.ShouldBe(5); + result.InstanceDbCount.ShouldBe(5); + } + + [Fact] + public void Parse_data_type_resolution_unchanged_from_d1_for_instance_rows() + { + // Sanity: instance-DB rows go through the same TryResolveDataType path as global + // rows, so the existing primitive-type table is honoured. Spot-check the wider + // set: Bool, Int, DInt, Real, String. + const string csv = """ + Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type,Length + FB.Bit,FB,Bool,%DB1.DBX0.0,b,True,Instance DB, + FB.I16,FB,Int,%DB1.DBW2,i16,True,Instance DB, + FB.I32,FB,DInt,%DB1.DBD4,i32,True,Instance DB, + FB.F32,FB,Real,%DB1.DBD8,f32,True,Instance DB, + FB.Str,FB,String,%DB1.DBB12,s,True,Instance DB,32 + """; + + var result = ParseString(csv); + result.ParsedCount.ShouldBe(5); + result.InstanceDbCount.ShouldBe(5); + + result.Tags[0].DataType.ShouldBe(S7DataType.Bool); + result.Tags[1].DataType.ShouldBe(S7DataType.Int16); + result.Tags[2].DataType.ShouldBe(S7DataType.Int32); + result.Tags[3].DataType.ShouldBe(S7DataType.Float32); + result.Tags[4].DataType.ShouldBe(S7DataType.String); + result.Tags[4].StringLength.ShouldBe(32); + } + + [Fact] + public void Parse_global_rows_still_count_zero_instance_db() + { + // Negative regression — a CSV without any Instance-DB rows must report + // InstanceDbCount = 0 even when the DB type column is present. + const string csv = """ + Name,Path,Data type,Logical address,Comment,Hmi accessible,DB type + A,T,Int,%MW0,a,True,Global DB + B,T,Real,%DB1.DBD4,b,True,Global DB + """; + + var result = ParseString(csv); + result.ParsedCount.ShouldBe(2); + result.InstanceDbCount.ShouldBe(0); + } +}