GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-20 16:52:07 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,739 Commits 108 Branches 105 Tags
fix/encoder-stack-depth-json-flake
Commit Graph

7 Commits

Author SHA1 Message Date
romanett 9b034f8291 Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) 
# Description

Nullable reference types are now enabled once, in `common.props`, and
every project in the repository builds warning-clean with them.

**Configuration**
- `<Nullable>enable</Nullable>` lives in `common.props`. All per-project
`<Nullable>` settings are removed: 178 project files (enable,
annotations, and the `disable` in the three Aot test projects).
- 419 per-file `#nullable enable`/`restore` directives and the 4
`#nullable disable` directives are removed.
- Kept on purpose:
- The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type
Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`,
`PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the
project setting in generated code, so the directive is required there.
- `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against
the 1.5.378 packages and does not import the repository props.
- `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now
describe the central setting.

**Migration** (the 43 unset and 13 annotations-only projects, roughly
10k diagnostics)
- **Production code: annotated at the declarations.** Covers
`tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the
LDS sample.
- Members, parameters and returns are nullable where null actually
flows.
- Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`,
`IsMethodTypeDesign`).
- `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take
`params object?[]`. `Template.AddReplacement` takes `string?` (rendering
already skips null replacements).
- `IRegisteredServerStore.RegisterAsync` returns
`Task<RegistrationEntry?>`: it returns null when a registration is
removed.
- Dereferences backed by an invariant (for example, the validated model)
use `!`, so runtime behaviour is unchanged.
- **Behavioural changes in production code** (small, deliberate):
- `NodeSetToModelDesign` throws `InvalidOperationException` when the
file is not a NodeSet. Before, it threw a NullReferenceException on the
next line.
- `ModelDesignValidator` starts its lookup tables empty instead of null.
A redundant `if (m_nodes == null) return;` guard is gone.
- **Test code: minimal annotations.**
  - `?` on values that can be null, `!` where the test asserts non-null.
  - `= null!` for fixture members assigned in SetUp.
- `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the
`IEncodeable` constraint.
- A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`)
folded into the adjacent `IsNull`/`Count` checks.

**Notes for reviewers**
- The diff is large (~1,500 files) but almost entirely mechanical: `?`,
`!` and directive removal. The non-annotation changes are listed above.
- Known follow-up, not done here:
`IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay
non-nullable even though the validator returns null before it loads a
dictionary. Making them nullable would ripple through every generator
call site.

## Related Issues

- Fixes #4439

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

The "no new warnings" box covers net10.0 and net48: full non-incremental
builds of `UA.slnx`, `tools/SourceGeneration.slnx`,
`tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0
warnings and 0 errors under the repository's warnings-as-errors
settings. net9.0 and net8.0 also had no nullable diagnostics, but they
were not rebuilt after the final fixes. Tests have not been run locally
yet.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-08 08:48:13 +02:00
romanett cce2004fdd CTT: keep GeneratesEvent off Object instances and Views off non-hierarchical references; classify Deadband Err-006 as CTT defect (#4667) 
# Description

Three findings from the 2026-10-03 CTT run with the patched 1.05.513
scripts. Two are server/model defects and are fixed here. The third is a
CTT script defect, recorded as C56 in `plans/ctt-issues.md`.

## 1. Monitored Items Deadband Filter `Err-006.js`: CTT defect (C56, no
server change)

The script modifies a MonitoredItem on the first static numeric scalar
(a plain BaseDataVariable with no EURange) to use a PercentDeadband. It
accepts only `Good` or `Bad_MonitoredItemFilterUnsupported`. Since #4629
the server returns `Bad_DeadbandFilterInvalid`, and that is the code the
spec defines for this case:

- OPC 10000-8 §7.2: PercentDeadband "applies only to AnalogItems with an
EURange Property".
- OPC 10000-8 §7.3.2 Table 61: `Bad_DeadbandFilterInvalid` is returned
when "a PercentDeadband is not supported, since an EURange is not
configured".

CreateMonitoredItems and ModifyMonitoredItems both go through
`ValidateMonitoringFilterAsync` and return the same code. Monitor Basic
`Err-022.js` already accepts `Bad_FilterNotAllowed` for non-analog
items, so the scripts disagree with each other here.

Draft Mantis note (not filed yet):

> **Monitored Items Deadband Filter Err-006.js rejects
Bad_DeadbandFilterInvalid for a PercentDeadband on a node without
EURange**
> Err-006.js picks the first node of `/Server Test/NodeIds/Static/All
Profiles/Scalar/NumericSettings`, which is normally a BaseDataVariable
without an EURange. It then calls ModifyMonitoredItems with a
PercentDeadband (10 %) and accepts only Good or
Bad_MonitoredItemFilterUnsupported. OPC 10000-8 §7.2 limits
PercentDeadband to AnalogItems with an EURange. §7.3.2 Table 61 defines
Bad_DeadbandFilterInvalid for "a PercentDeadband is not supported, since
an EURange is not configured", so a compliant server fails this test.
Monitor Basic Err-022.js already accepts Bad_FilterNotAllowed for the
non-analog fallback.
> Proposed fix: add StatusCode.BadDeadbandFilterInvalid (and, consistent
with Err-022.js, StatusCode.BadFilterNotAllowed) to the
ExpectedAndAcceptedResults of the ModifyMonitoredItems call.

## 2. GeneratesEvent on Objects (Base Info Core Structure 2 `001.js`):
source generator defect

The CTT reported `ns=9;i=234` (the `<ServiceName>` placeholder of
AuthorizationServicesFolderType) and
`ns=9;s=AuthorizationServices/Default` as Objects that are the source of
GeneratesEvent references. OPC 10000-3 §7.15 says: "The SourceNode of
References of this type shall be an ObjectType, a VariableType or a
Method".

`OpcUaGdsModel.xml` declares both references
(AccessTokenRequested/Issued audit events) correctly, on
`AuthorizationServiceType`. The source generator copied the type's own
non-hierarchical references onto every Object or Variable built from the
type: onto instance declarations (the placeholder) and into
`CreateInstanceOf<Type>`, which `AuthorizationServiceState` initialises
from, and that is how the `Default` service gets them. UA-ModelCompiler
does the same thing, so the official `Opc.Ua.Gds.NodeSet2.xml` in
UA-Nodeset has the same two references on `ns=1;i=234`.

Fix:
- `TranslateReferences` now marks references declared on a type
(`HierarchyReference.DefinedOnType`, which already existed but was never
set).
- `NodeStateGenerator.GetReferences` drops forward GeneratesEvent
references (including subtypes such as AlwaysGeneratesEvent) for Object
and Variable nodes when the reference came from the type.
- ObjectTypes, VariableTypes and Methods keep these references. For
example, the RegisterApplication methods and the Acknowledge/AddComment
methods of the alarm types are unchanged.
- References that a model declares explicitly on an instance are emitted
as before.

## 3. View targeted by HasCause/HasEffect (Base Info Core Structure 2
`001.js`): sample model defect

#4485 added `Views_Operations HasCause/HasEffect Views_Engineering` so
that View Minimum Continuation Point 01 `012.js` and View Basic 2
`018.js` have at least two references to View nodes on the configured
View (`/Server Test/NodeIds/NodeClasses/View` = `Views_Operations`). OPC
10000-3 §5.4 says Views "can be the SourceNode of any hierarchical
Reference. They shall not be the SourceNode of any NonHierarchical
Reference."

`Views_Operations` now Organizes `Views_Engineering` and a new View,
`Views_Maintenance`, which organizes `DataAccess`. This keeps two
View-class references and adds no loops (ContainsNoLoops stays true).

## Tests
-
`NodeStateGeneratorTests.TypeGeneratesEventReferencesAreNotCopiedToObjectInstances`:
covers the type, the placeholder, the instance, `CreateInstanceOf`,
Methods and an explicit instance reference. It fails without the fix.
- `ReferenceServerTest.SampleViewsHaveNoNonHierarchicalReferencesAsync`.
The existing `NodeClassSampleNodesReferenceTheirOwnNodeClassAsync` still
passes.
- Results with net10.0:
  - SourceGeneration.Core.Tests: 4267 passed, 0 failed
  - InformationModel.Tests: 1078 passed, 0 failed
  - Di.Tests, including RobotOpenUsdE2e: 438 passed, 0 failed
  - OpenUsd.Tests: 921 passed, 0 failed
  - Server.Tests `ReferenceServerTest`: 56 passed, 0 failed
- `UA.slnx` builds with net10.0. The changed test projects build with
net48.

## CTT (patched 1.05.513 scripts, compared with the 2026-10-03 patched
baseline)

| Part | Change |
| --- | --- |
| p02-base-info | Base Info Core Structure 2 `001.js`: Error → Skipped.
The GeneratesEvent and View errors are gone; the remaining skips are the
CTT's own "type not defined within the test-script" notes. No other
change. |
| p03-base-view-method | No change. View Basic 2 `018.js` and View
Minimum Continuation Point 01 `012.js` still pass. |
| p13-monitored-items | Deadband `Err-006.js` still Error (C56). With
the script fix it passes. Monitor Basic `007.js` gave a
response-timestamp delay warning (>200 ms) caused by machine load; it
also appears in the 2026-09-29 base run. |

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-05 05:50:30 +00:00
romanett 442889cba5 Fix source generator audit findings; conform structure-field matrices and type metadata to OPC UA 1.05 (#4569) 
# Description

This PR fixes a bug audit of the source generation projects
(`tools/Opc.Ua.SourceGeneration*`: model loading, NodeSet import, all
emitters, the Roslyn hosts and packaging). It also fixes a max-effort
code review of those fixes, and a check of every spec-relevant change
against the OPC UA 1.05 text.

- **Scope:** about 85 audit findings, 48 review findings and 32
spec-grounding items were addressed. Each fix has a regression test that
fails without it.
- **Wire change:** it includes one deliberate, stack-wide wire change:
**structure-field matrices now follow OPC 10000-6 5.2.5 Table 28**.

Most changes are internal correctness fixes. The sections below list
every change a user can observe: wire formats, generated code and API,
generated address-space content, diagnostics and runtime behaviour.

## Behaviour / user-facing changes

### Wire format (encoders)
- **Binary inline matrices** (OPC 10000-6 5.2.5 Table 28).
- **Layout:** a multi-dimensional *structure field* is encoded as
`Int32[] Dimensions`, then the values **without a length prefix**, with
at least 2 dimensions.
- **Empty and null:** any dimension <= 0 means no values follow, and a
null matrix is `-1`. An empty matrix is written as `[0,0]`.
- **Compatibility:** this is **not compatible with earlier builds of
this SDK** for matrix fields. It applies to generated types,
`[DataType]` types, `Structure`, client ComplexTypes and
`WriteEncodeableMatrix`.
  - **Unchanged:** Variant matrices (5.2.2.16) keep their old encoding.
- **Declared rank decides matrix vs array.** A 1-D `MatrixOf<T>` in an
array field or a Variable value is written as an array again.
Matrix-declared fields go through the new public
`EncoderExtensions.WriteInlineMatrixValue`, which generated code also
uses. A non-empty 1-D value in a matrix field throws `BadEncodingError`.
- **Decoder hardening:**
- Before allocating, the decoder bounds all matrix dimensions against
overflow, `MaxArrayLength` and the remaining message length.
  - The decoder rejects:
    - a matrix with fewer than 2 dimensions;
- a populated matrix whose rank differs from the field's declared rank;
    - a union SwitchField greater than the number of fields;
    - an EncodingMask with unassigned bits set.
- **Encoder `MaxArrayLength`:** the encoder enforces `MaxArrayLength`
for inline matrices, as it already did for arrays.
- **XML structure fields** (5.3.1, 5.3.4, 5.3.5).
- **Fields:** `Structure` and client ComplexTypes write fields like
typed fields (`<A>1</A>`, array elements directly) instead of a Variant
body.
- **Matrix fields:** `Dimensions` and `Elements` go directly inside the
field element, with no `<Matrix>` wrapper.
- **Empty matrices:** written as nil, because XML dimensions must be
greater than 0.
  - **Compatibility:** decoders still accept the old shapes.
- **Enum fixes:** enum array elements with value 0 are no longer
dropped, and enum array element names are fixed.
- **Other encoding fixes:**
- A null `DiagnosticInfo` structure field writes `0x00` instead of
nothing.
- A null scalar field in a runtime `Structure` is written as its default
instead of zero bytes, which used to shift every following field.
- A Variant holding an empty matrix encodes as an empty array instead of
throwing.
  - `Variant.Copy` keeps the matrix identity of null and empty matrices.
- **NodeSet export and the DataContract surrogate** write Variable
values with the Variant rules again. A null or empty `MatrixOf` variable
no longer produces invalid XML or unreadable NodeSets.
- **`JsonEncoder.CanOmitFields`** is `true` only for Compact encoding.
Verbose and RawData JSON include default-valued fields.

### `[DataType]` source-generated types
- **Default-value handling** (OPC 10000-6 5.4.1, Table 1).
- **Encode:** a field is omitted only when its value *and* its declared
default are the type's default (0, false, null). Fields with a
non-default initializer or constructor default, and empty strings, are
always written, so configuration files get slightly larger.
- **JSON decode:** a field missing from JSON decodes to the type
default.
- **XML decode:** a field missing from XML keeps its declared default,
which older configuration files rely on.
- **Why:** this fixes silent round-trip loss, for example
`SamplingInterval = 0` read back as `-1`.
- **Nested `[DataType]` types:** they are now generated inside their
containing types. Without an explicit id, their default TypeId and
XmlName are qualified (e.g. `s=Outer.Foo`); **wire identity change** for
nested types. Colliding activator names get a `_2` suffix.
- **Activators:** they are `internal` when the type, or any containing
type, is not public (was CS0060).
- **Matrix properties:**
  - `MatrixOf<T>` properties use the inline matrix encoding.
- Matrices of non-sealed structures, or with
`StructureHandling.ExtensionObject`, are written as ExtensionObjects.
- Their published ValueRank is 2, because the rank can't be known from
`MatrixOf<T>`.
- **Published StructureDefinitions:**
  - They include base-class fields.
  - Nullable properties are no longer marked optional.
  - Inline structure fields name their concrete type.
- **`[Flags]` enums:** they are published as plain enumerations with
their member values, which matches how they are encoded.
- **Other generated code:**
- `Clone` deep-copies arrays and matrices of structures, Variants,
ExtensionObjects and DataValues.
- `IsEqual` and `GetHashCode` are consistent: EncodingMask, active union
member and NaN handling agree.
- Pooled `Reuse()` restores the freshly constructed state through the
new `protected virtual ResetForReuse()`, and returns an instance to its
runtime type's pool.
- **Enum capture:** enum member values are captured culture-invariantly.
Negative values were lost on cultures that use U+2212 as the minus sign.

### Model-driven generator: generated code, API and file names
- **Removed API:** generated `*_Placeholder` proxy accessors (e.g.
`PowerTrainTypeClient.GetMotorIdentifier_PlaceholderAsync`) and fluent
`X_Placeholder()` accessors are removed; they could never resolve
anything.
- Proxy accessors now look children up by BrowseName with the right
namespace.
- `RoboticsClient` finds motors and gears by type and drives via
`IsDrivenBy`. Its topology snapshots used to be empty.
- **Renamed files:** generated files for a bound `[NodeManager]` are now
named `{Namespace}.{Class}.NodeManager.g.cs` (also `.Factory` and
`.FluentBuilders`).
- **Generated NodeState code:**
- Attributes are set through `baseState`/`nodeState` locals, so children
named like base attributes (e.g. `DataType`, `EventNotifier`) compile.
- Children that hide base members are declared `public new`, using names
reflected from the runtime.
  - `global::` qualification is used throughout.
- **Proxies and fluent builders:** a proxy whose supertype is excluded
(e.g. Draft) derives from the nearest emitted ancestor. Fluent wrappers
and NodeState factories fall back the same way; they used to fail with
CS0246.
- **Derived structures:**
  - The optional-field `EncodingMask` continues across inheritance.
- A derived structure over a base type from another model no longer
relies on `base.ResetForReuse`.
- **Default values:**
- NodeId, QualifiedName and ExpandedNodeId defaults resolve by namespace
URI (`context.NamespaceUris.GetIndexOrAppend`) and server URI.
- StatusCode, negative enum, `double`/`float` (shortest round-trip
literal) and escaped string defaults now compile.
- **Event records:**
- Vendor event records have `Decoder.GetStandardFields(NamespaceTable)`,
and their browse paths use the model's namespace.
- A field whose namespace is unknown to the session no longer matches a
namespace-0 field.
  - Companion `StandardFields` is `[Obsolete]`.
- Vendor simple subtypes such as `Percent : Double` map to their
built-in type.
- **Other fixes:**
- `MethodState.CallAsync` passes the real `objectId` to synchronous
`OnCallMethod2` handlers; it used to pass `NodeId.Null`.
- Enumeration fields with an explicit `Identifier` keep it, instead of
taking a trailing `_<digits>`.
  - Explicit NumericIds are no longer reused by automatic numbering.
  - Dependencies are loaded in topological order.

### Generated address-space content
- **Arguments:**
- Methods without arguments no longer get empty
`InputArguments`/`OutputArguments` properties (OPC 10000-3 5.7).
- About 2,660 automatically assigned namespace-0 NodeIds of 15000 and
above shift. Every NodeId in `StandardTypes.csv` is unchanged.
- **Permissions:** type nodes and method declarations on types now carry
the model's `RolePermissions`/`AccessRestrictions`.
- **Restored nodes:** NodeSet import keeps nodes that were silently
dropped before:
- `NamespaceMetadata` objects and the NodeSet's own TypeDictionary
variables;
  - ISA95 placeholders linked only non-hierarchically;
  - instances whose parent is in another namespace.
- **Import details:**
  - ValueRank > 1 without ArrayDimensions keeps its rank.
  - `svr=` indexes map correctly.
  - Invalid MinimumSamplingInterval values import as -1, not 0.
  - Localized texts prefer the invariant or English translation.
- **StructureDefinitions:**
- They include inherited fields, including for subtypes without their
own fields (e.g. `AnonymousIdentityToken` now publishes PolicyId).
  - The StructureType kind covers inherited AllowSubTypes fields.
  - Unions ignore IsOptional.
- StructureFields never publish ValueRank 0; a OneOrMoreDimensions field
without dimensions becomes rank 2.
- Fields with ValueRank -2/-3 are published as scalar BaseDataType,
which is what is on the wire.
- **OptionSets:**
  - Implicit fields get the next free bit.
- `OptionSetValues` uses null for undefined bits instead of "Reserved",
and structure OptionSets publish all their bits (no 32/96-bit cap).
- Numeric OptionSets are limited to their base type's width, and UInt64
bit 63 works.
- **Schemas:**
- Structures with inline-matrix fields are left out of the `.bsd`
(5.2.5).
- In the BSD, Variant-encoded and non-structure subtyped fields are
described as `ua:Variant`.
- In the `.xsd`, matrix fields are typed `ua:Matrix` with
`Dimensions`/`Elements`, and `EncodingMask` is required.

### NodeSet / model input handling
- **Model selection:**
- The newest NodeSet of a model is chosen by `ModelVersion` (SemVer 2.0
precedence), then PublicationDate. The `Version` attribute, which Annex
F says is not for comparison, is only a tie-break.
- The order is total and does not depend on input order; referenced
assemblies use the same rule.
- **Default names:**
- Invalid default Name/Prefix values are repaired (`201301ISA95` becomes
`_201301ISA95`, `class` becomes `class_`).
  - Valid names are unchanged.
  - Sanitising no longer depends on the OS.
- **`ModelUri` metadata** that doesn't match the NodeSet raises a
warning instead of silently losing the model.
- **Input detection:**
- Only XML files whose root is `ModelDesign` or `UANodeSet` are treated
as models.
- A malformed NodeSet stays a NodeSet input, so it no longer breaks the
ModelDesign pass.
- **`IdentifierFile` paths:** they are normalized (`./`, `../`), and the
suffix match requires a folder boundary.
- **Dependency payloads** (`ModelDependencyV1`) carry field IsOptional,
AllowSubTypes, ArrayDimensions and the producer's namespace tables in
skippable trailers, so old and new readers stay compatible.

### Roslyn host, diagnostics, packaging
- **New diagnostics:**
- MODELGEN036: `[NodeManager]` target not supported (nested, generic,
file-local or global namespace).
- MODELGEN037: `[DataType]` target not supported (struct, generic,
abstract, file-local or global namespace).
- MODELGEN038: the definition of an encodeable base type can't be found.
- **New validator errors:**
- a structure with both optional and subtyped fields (OPC 10000-6 F.13);
  - multi-bit or duplicate OptionSet masks.
- **Diagnostic locations:** `[DataType]`/`[NodeManager]` diagnostics
point at real source locations again, so `#pragma` and `.editorconfig`
apply. MODELGEN010 is the exception.
- **Incremental caching:** pipeline values compare by value, so
unrelated edits no longer regenerate the whole model or all
`[DataType]`s.
- **`ModelSourceGeneratorIgnore`** on a ModelDesign input is honoured;
the design still resolves as a dependency.
- **Cancellation:** a cancelled generation run propagates
`OperationCanceledException` instead of reporting a MODELGEN003 error.
- **Stack package:** the `Stack` NuGet package now ships
`build/OPCFoundation.NetStandard.Opc.Ua.SourceGeneration.Stack.props`,
so `StackSourceGeneratorMode` and `StackSourceGeneratorExclude` reach
the compiler.

## Related Issues

- No tracking issue. The audit, review and spec-grounding reports are
available on request. Contains the matrix wire-format change discussed
for D-5 (OPC 10000-6 5.2.5 Table 28).

## Checklist

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
(`docs/SourceGeneratedDataTypes.md`, `docs/NodeManagers.md`, generator
readme)
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. (`UA.slnx` 0 warnings / 0 errors on net48 and
net10.0)
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. The
affected suites were run on net10.0: SourceGeneration Core/Roslyn/Stack,
Types, Core.Encoders, Encoders.Fuzz, Client, Client.ComplexTypes,
PubSub, Core, Server, GDS, Configuration, Di, Robotics, ISA95,
Positioning, OpenUsd, XRegistry. net48 was built; its test legs, and the
post-rebase verification, run in CI.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-29 00:47:40 +02:00
Copilot 02fb72e2d3 Support guid/opaque nodeids in NodeSet import (#4507) 
# Description

`NodeSetToModelDesign.ImportNode` only stored Numeric and String NodeId
identifiers. Guid and Opaque identifiers (both valid per OPC 10000-3
5.2.2) fell through to an `Information` log line, so the node was
registered in `NodesByQName`/`NodesById` with no identifier at all and
code generation continued against a node whose identity had been
discarded.

The `ModelDesign` schema has no representation for Guid/Opaque
identifiers (`NodeDesign` exposes only `NumericId`/`StringId`), so the
import now fails loudly instead of emitting output that looks correct.

- **Importer**: the fall-through branches in `ImportNode` are replaced
with an `InvalidDataException` naming the NodeId, browse name and
identifier type. This matches the existing unsupported-input handling in
the file and surfaces through `ModelCompilation` as a generator
diagnostic.
- **Logger removal**: `m_logger` had no remaining use in
`NodeSetToModelDesign` and was removed along with its
`Microsoft.Extensions.Logging` import.
- **Tests**: `ImportUnsupportedNodeIdTypeThrowsInvalidDataException`
covers Guid, empty Guid and Opaque identifiers via an in-memory NodeSet.
- **Docs**: the source generator readme states the NodeSet2 identifier
restriction.

```csharp
else
{
    // The model design can only carry numeric and string identifiers.
    // Guid and Opaque identifiers (OPC 10000-3 5.2.2) would be dropped
    // and code would be generated for a node without any identity, so
    // the import fails instead of silently producing wrong output.
    throw new InvalidDataException(
        $"NodeId ({input.NodeId}) of node '{input.BrowseName}' uses the " +
        $"{nodeId.IdType} identifier type which is not supported by the model " +
        "design. Only Numeric and String identifiers can be imported.");
}
```

Note on behaviour: a null/unspecified NodeId parses as a Numeric
identifier (`i=0`) and is still handled by the numeric branch, so the
removed `IsNull` branch was only ever reachable for an empty Guid or
Opaque identifier, which is now reported as unsupported.

## Related Issues

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #4504

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-09-21 14:52:55 +02:00
romanett 6e0ed89d3f Make UANodeSetSerializers.g.cs generation deterministic (#4252) 
## Problem

Regenerating `src/Opc.Ua.Types/Schema/UANodeSetSerializers.g.cs` via
`UANodeSetSerializers.ps1` produced ~144 lines of no-op churn on every
run. The sgen tool (Microsoft.XmlSerializer.Generator) emits the Reader
`idNN_Name` field declarations and the matching `InitIDs()` initializers
in a hash-seed-dependent order: the assigned id numbers are stable, but
their emission order shuffles between runs. This made an otherwise no-op
regeneration produce a large, noisy diff that was painful to review.

## Fix

- Added a canonicalization pass in `UANodeSetSerializers.ps1` that sorts
each of the two affected contiguous blocks (the `string idNN_Name;`
field declarations and the `idNN_Name = Reader.NameTable.Add(...)`
initializers) by numeric id ascending, so re-running on unchanged input
yields byte-identical output.
- The name regex was widened to `@"[^"]*"` so entries with an empty name
(e.g. `id3_Item`) or URI-style names do not split the block into
independently-sorted runs (which would keep it unstable).
- Regenerated the committed `.g.cs` once through the improved script;
the resulting diff is sort-only.

## Also fixed: tool download

Replaced `Expand-Archive` (which rejects the `.nupkg` extension on
Windows PowerShell) with
`System.IO.Compression.ZipFile.ExtractToDirectory`, so a clean run with
an empty tool cache works end-to-end on both Windows PowerShell 5.1 and
PowerShell 7+.

## Validation

- From an empty tool cache, the full download + extract + generate path
succeeds.
- A second run is a byte-identical no-op (`git diff --exit-code` = 0).
- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` builds with 0 warnings and 0 errors.
- All AOT post-processing is preserved: custom header, `#if
NET5_0_OR_GREATER` wrapper, module IL2026/IL3050 suppressions,
`[RequiresUnreferencedCode]`/`[RequiresDynamicCode]` on the
InitCallbacks() overrides (Writer = RequiresUnreferencedCode only,
Reader = both), pragmas, UTF-8 BOM, and CRLF line endings.

No hand-edits to `.g.cs`; the fix lives entirely in the script so future
regenerations stay stable.

## Also: Load UserAccessLevel from NodeSet2 model

Bundled here because it regenerates `UANodeSetSerializers.g.cs`, which
this PR owns.

- `UANodeSet.cs`: adds the `{Member}Specified` XmlSerializer convention
to `UAVariable.UserAccessLevel` (new `UserAccessLevelSpecified`
companion; removed the `[DefaultValue((uint)1)]`) so an
explicitly-present `UserAccessLevel` attribute is distinguishable from
the schema default of 1.
- `UANodeSetSerializers.g.cs`: regenerated - the writer now emits
`UserAccessLevel` when `UserAccessLevelSpecified`, and the reader sets
`UserAccessLevelSpecified = true`. Regenerating produced only these two
behavioral changes plus removal of pre-existing empty `if
(o.@MemberSpecified) { }` no-op blocks (see below); no `idNN` re-sort,
so determinism held.
- `UANodeSetHelpers.cs`: the runtime importer honors an
explicitly-specified `UserAccessLevel` instead of always deriving it
from `AccessLevel`.
- Source generator (`ModelDesign.cs` `RawUserAccessLevel`,
`ModelDesignExtensions.GetUserAccessLevelAsCode`,
`NodeSetToModelDesign.cs`): emit the verbatim `UserAccessLevel` when the
NodeSet sets it, otherwise derive from `AccessLevel` (matching the
runtime importer).

### Generator normalization for `{Member}Specified`

sgen emits an extra empty `if (o.@MemberSpecified) { }` no-op block near
`WriteEndElement` for every member that uses the `{Member}Specified`
convention. Adding the companion to `UserAccessLevel` would have
introduced one such block, and the committed file already contained
several for other members (`LastModified`, `AccessRestrictions`,
`PublicationDate`). `UANodeSetSerializers.ps1` now strips these empty
guard blocks during post-processing, so the generated file stays free of
dead code and a re-run of the generator remains a byte-identical no-op.
The real, non-empty guard around the `WriteAttribute` call is preserved.

The ReferenceServer sample application of this feature is intentionally
excluded and lands with #4220.

### Validation (UserAccessLevel)

- `dotnet build src\Opc.Ua.Types\Opc.Ua.Types.csproj -c Release -f
net10.0` and `dotnet build
tools\Opc.Ua.SourceGeneration.Core\Opc.Ua.SourceGeneration.Core.csproj
-c Release`: both 0 warnings / 0 errors.
- `dotnet test tests\Opc.Ua.SourceGeneration.Core.Tests -c Release -f
net8.0 --filter
"FullyQualifiedName~NodeSetToModelDesign|FullyQualifiedName~UserAccessLevel"`:
16 passed, 0 failed.
- Re-running the generator remains a byte-identical no-op.

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: romanett <7413710+romanett@users.noreply.github.com>
 2026-08-14 08:00:13 -07:00
Marc Schier 8a10139b8c Fix OPC UA address-space compliance in the device integration stack (#4117) 
# Description

A live OPC UA compliance audit of `samples/PumpDeviceIntegrationServer`
against the **OPC 40223 Pumps** and **OPC 10000-100 (DI)** companion
specifications found **13 address-space violations** plus a
serialization bug in the MCP tool. Every fix lands at its **root cause
in the shared stack** rather than as a workaround in the sample, so all
servers benefit.

The sample now exposes **N fully simulated pumps** (`--pumps N`) and
advertises the DI conformance facets it actually satisfies.

> This branch has been merged with `master` including **#4116** ("Add
simulation to PumpDeviceIntegrationServer and Event modelling"), which
independently reworked the same pump simulation and the same fluent
alarm builders. See [Relationship to #4116](#relationship-to-4116) below
for how the two were reconciled.

## Findings and fixes

| # | Finding | Root cause | Fix |
|---|---------|-----------|-----|
| 1 | `OverTempAlarm` created by the fluent API did not exist in the
address space | `AttachAlarm` attached the alarm with `AddChild` but
never indexed it into the node manager | Register the node; the same gap
existed in `AddObject`, `CreateInstance` and the state-machine creators
|
| 2 | Alarms were unsubscribable — `EventNotifier = 0` everywhere, no
`HasNotifier` from `Server` | No event-source wiring at all | Set
`HasCondition`, initialise
`SourceNode`/`SourceName`/`ConditionName`/`InputNode`, promote
`EventNotifier` up the ancestor chain, publish `HasNotifier` from the
`Server` object |
| 3 | Every instance NodeId was minted in the **DI standard namespace**
(`ns=4;s=5001_Pump #1`) | `DiNodeManager.New` inherited the parent's
namespace index | New `DiNodeManager.InstanceNamespaceIndex`; instances
move to the server's application namespace |
| 4 | `Server/Namespaces` described only 2 of 6 namespaces | The
NodeSet2 importer drops nodes parented to namespace 0, so companion-spec
metadata objects never reached the address space — and no route covered
the server's own namespace | New `NamespaceMetadataPublisher` walks the
`NamespaceArray` and fills version/publication date from
`ModelDependencyAttribute` |
| 5 | Vendor BrowseNames (`Diagnostics`, `LastError`, …) sat in the DI
namespace | Builders defaulted to the parent's namespace | Default to
the server namespace; **spec-defined BrowseNames are unchanged** |
| 6 | `AccessLevel="5"` from the Pumps NodeSet was reported as `1` | The
ModelDesign `AccessLevel` enum is not `[Flags]` and cannot represent
`CurrentRead \| HistoryRead` | Carry the verbatim NodeSet2 bitmask and
emit named `AccessLevels` constants |
| 7 | `Historizing = true` while `HistoryRead` returned
`BadHistoryOperationUnsupported` | Nothing reconciled the declared
history surface with the wired historians | Startup clears the
advertisement (including masking the attribute read callbacks) when no
provider resolves; the sample wires a real historian |
| 8 | Mandatory `ProductInstanceUri` unset | Sample omission | Populated
on every pump |
| 9 | Mandatory `TrueState`/`FalseState` empty on every supervision
boolean | Absent in the official NodeSet; a server must still populate
them | Populated on every pump |
| 10 | Machinery `Machines` folder empty (OPC 40001-1 §9.2 says
**shall**) | `TryAddToMachinesFolder` used `AddChild`, which
*re-parents* the device away from `DeviceSet` | Add the `Organizes`
reference instead |
| 11 | BrowseName contained a space and `#` | Sample naming | `Pump_1`
with the readable label in `DisplayName` |
| 12 | `Maintenance` group materialised but empty | Sample omission |
Populated |
| 13 | Uninitialised values reported `Good` | No initial-value status |
`BadWaitingForInitialData` until the first simulation tick |
| — | `ServerProfileArray` advertised only `StandardUA2017` |
`DiNodeManager` did not implement the existing `IConformanceContributor`
| Contribute DI 1.05 server facets computed at runtime from what is
actually wired, merged with `StandardUA2017` |
| — | MCP tool serialised boolean `false` and numeric `0` as JSON `null`
| `Variant.Null` is `default`, so `Variant.Equals` adopts the non-null
type and compares against the default value | Dispatch on `BuiltInType`
and use typed `TryGetValue` accessors instead of the prohibited
`AsBoxedObject` |

## Relationship to #4116

#4116 landed on `master` while this branch was in flight and targeted
overlapping ground. The merge is a **union of both intents**, not a
fast-forward:

- **`AlarmBuilderExtensions`** — kept both sides. #4116 contributes the
parent-must-be-an-Object guard, `SetEnableState`, and the
`HasEventSource` references; this branch contributes the `HasCondition`
reference, condition source initialisation, **registration of the alarm
with the node manager so it is browsable**, notifier promotion up the
whole ancestor chain, and root-notifier registration.
- **`SupervisionBuilderExtensions`** — took #4116's `SetAlarmActive`. It
is more spec-correct than this branch's version: it honours
`EnabledState`, resets Acked/Confirmed on activation, and computes
`Retain` per OPC 10000-9 so an unacknowledged Condition stays retained
after going inactive. This branch's null guard was kept so a malformed
alarm cannot dereference a missing `EnabledState`.
- **`PumpNodeManager.Configure.cs`** — took #4116's **push-based**
simulation (`RegisterPumpSimulation`, `CreatePumpSimulation`,
`IValueUpdater<T>`, `Initialize`/`Advance`/`Publish`, deterministic
per-pump phase offsets) as the structure, then folded in this branch's
compliance work: configurable pump count, Maintenance group,
`TrueState`/`FalseState`, historian wiring, per-pump mandatory
Identification values, and `BadWaitingForInitialData` until the first
tick.
- **README** — union, keeping #4116's address-space validation workflow
section.

Two test adjustments were needed, both because the merge legitimately
changed behaviour the tests pinned:

- `PumpHostedReferenceTests` now resolves pump NodeIds from the browse
result instead of hard-coding the DI namespace, and waits for the first
published value. `PumpCreatedAfterStartupJoinsTheLiveSimulationAsync`
was **added** to restore #4116's coverage that a pump created after
startup through `ConfigureDevicesFor` joins the live simulation.
- `FluentAlarmRegistrationIntegrationTests` no longer asserts that
`Retain` clears when an alarm goes inactive — that pinned this branch's
weaker semantics, and #4116's Part 9 behaviour is correct.

## Breaking change

Runtime-created DI device instance NodeIds move from the companion-spec
namespace to the server's application namespace, e.g.
`ns=<DI>;s=5001_Pump #1` becomes `ns=<application>;s=5001_Pump #1`. This
invalidates NodeIds cached by clients from earlier 2.0 previews. It is
**not** a model change — standard DI/Machinery/Pumps type NodeIds and
spec-defined BrowseNames are untouched.

Device integration hosting is new in 2.0, so there is nothing to migrate
from 1.5.378 and this is deliberately *not* in `docs/MigrationGuide.md`.
Anyone tracking 2.0 previews should rediscover devices by browsing
`DeviceSet` rather than persisting NodeIds, and resolve namespace
indexes from `NamespaceArray` per connection.

## Verification

Compliance is pinned by regression tests that assert **through the
server's service surface** (`BrowseAsync` / `ReadAsync` /
`HistoryReadAsync`), not by inspecting `NodeState` objects in memory.
That distinction mattered: three defects survived in-memory assertions
and were only caught through the read path — most subtly, `Historizing`
was cleared while `AccessLevel` still advertised `HistoryRead`, because
attribute read callbacks re-applied the NodeSet bits.

Post-merge on net10.0:

| Suite | Result |
|---|---|
| `Opc.Ua.Di.Tests` | 326 passed |
| `Opc.Ua.Server.Tests` | 3846 passed, 5 skipped |
| `Opc.Ua.Tools.Tests` | 288 passed |
| `Opc.Ua.History.Tests` | 506 passed, 27 skipped |
| `Opc.Ua.SourceGeneration.Core.Tests` | 3722 passed, 8 skipped |

The full solution was also built and tested on **net48** before the
merge.

Review feedback from `copilot-pull-request-reviewer` has been addressed
in commit `badad31dc`: the shared `ServerObjectState` lock was removed
(the forward `HasNotifier` edge is published through the owning node
manager instead), MCP array serialization now preserves element types,
and the pump option guards report the specific rejected option. The only
failure is
`ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a
pre-existing certificate-store issue that **passes when run in
isolation** and is unrelated to this change.

## Documentation

Updated `docs/DeviceIntegration.md`, `docs/MigrationGuide.md`,
`docs/HistoricalAccess.md`, `docs/DeveloperGuide.md`,
`docs/McpServer.md`, `docs/README.md`,
`samples/PumpDeviceIntegrationServer/README.md` and
`tools/Opc.Ua.Mcp/README.md`, and consolidated the four node-manager
documents into a single [`docs/NodeManagers.md`](docs/NodeManagers.md) —
overview, built-in managers (including `MasterNodeManager`), core vs
custom, registration, server address-space metadata, and source
generation — with a TOC and all inbound links repointed.

## Related Issues

No tracking issue exists for this work yet — it originated from an
ad-hoc compliance audit of the pump sample. Given the size of the change
and the runtime-breaking NodeId move, please open (or link) a tracking
issue so the design is recorded as an ADR before merging.

- Fixes #

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 0d50c3fc-58b1-4c99-85a4-1e32c88666d9
 2026-07-30 16:04:18 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00