Commit Graph

2 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
Klaus Landsdorf b25c1f6ad4 Asset Management Basics: the OPC 10000-110 companion SDK (#4662)
# Description

Stacked on #4650. Until that is merged, this PR also shows its commits;
the six commits of this PR start after `fix(server): keep conformance
unit aggregation idempotent and ordered with registrations`.

This adds OPC 10000-110 Asset Management Basics 1.01.1
(`http://opcfoundation.org/UA/AMB/`) as a companion SDK, in the pattern
of the DI, Machinery, Pumps and Scales packages. The tracking issue
#4649 describes the scope, the design and the open questions. In short:

| Package | Content |
|---|---|
| `Opc.Ua.AMB` | the model generated by the source generator from the
unmodified NodeSet, plus the contracts server and client share
(`AssetFaultSeverity`, `AssetLocationKind`, `MaintenanceStateKind`,
`DocumentationLinkProperties`) |
| `Opc.Ua.AMB.Server` | a sidecar node manager next to the owner of the
Device Integration address space (`AddOpcUaDi`, `AddMachinery`,
`AddPumps` or `AddScales`), the asset registry `IAssetManagement` with a
fluent builder, `AddAssetManagement()` hosting, memory and file system
stores, the AMB conformance units and server facet |
| `Opc.Ua.AMB.Client` | `AmbClient`: discovery, identification, health
alarm and maintenance streams, documentation links, locations, structure
and snapshots, plus `AddAssetManagementClient()` |

**Sidecar.** An asset stays in the node manager that owns it. Everything
AMB adds to it (`DeviceHealth`, health alarms, maintenance conditions,
the DocumentationLinks AddIn, AssetId and the other properties) is
created there. The AMB node manager serves the AMB model, the alias
categories `Assets`, `AssetsByProductInstanceUri` and `AssetsByAssetId`
below `0:Aliases`, the location hierarchies below `0:Locations` and the
server-specific alarm and condition types. Owners register assets with
the shared registry from their setup, in either startup order.

**Implemented conformance units** (Table 54): Asset Identification,
Configurable Asset Identification, Discovery by ProductInstanceUri and
by AssetId, Health Status Base, Alarms, Root Causes and Alarm
Categories, Version Information, Operation Counters (inspected only; AMB
defines no counters), DocumentationLinks Base, Edit Base and Edit
Advanced, Requirements, Capabilities, Classification, Current and Future
Maintenance Activities, Local Time, the three location properties and
both location object hierarchies, Sub-assets and Asset relations. The
two history units (Health Tracking, Past Maintenance Activities) are not
part of this PR. `docs/AMB.md` has the matrix with the rule the server
applies for each unit and whether a test checks it over a session.

**Dependency on #4650.** The health alarms and maintenance conditions
are created through `CreateAlarm<TState>`, and the conformance units
follow the registered assets through the recomputation in
`ConformanceUnitsManager`.

## Changes

Six commits, each building on its own:

1. **`feat(amb)`: model package** (`src/Opc.Ua.AMB/`, about 2.4k lines,
of which the NodeSet and the two identifier tables are 1.6k).
- The NodeSet is the publication from `UA-Nodeset/latest/AMB`, byte for
byte (git blob `21f9475b646e9c5508171277ffe5b74edbeb0a58`).
`.gitattributes` checks it out with LF endings, because a test pins its
SHA-256.
- The published identifier table is vendored unchanged; a derived table
pins the symbol-to-NodeId mapping, and the generator fails the build
when they disagree. `tools/nodesets/README.md` documents the one new
kind of renamed symbol (top-level instances below `0:Aliases` and
`0:Locations`).
2. **`feat(amb)`: server package** (`src/Opc.Ua.AMB.Server/`, about
10.7k lines). Node manager, registry and builders, discovery, health,
maintenance (`MaintenanceEventStateMachine` with all transitions),
documentation links, locations, version information, classification
(IRDI dictionary entries below `Server/Dictionaries`) and structure,
stores, conformance evaluation, hosting. Log messages use the EventId
block 8300 to 8379, after Pumps (8100) and Scales (8200).
3. **`feat(amb)`: client package** (`src/Opc.Ua.AMB.Client/`, about 3.0k
lines). Calls AddLink and RemoveLink through the generated
`DocumentationLinksTypeClient`.
4. **`test(amb)`: test suite** (`tests/Opc.Ua.AMB.Tests/`, 163 tests,
about 8.8k lines; commit 6 adds one more).
5. **`docs(amb)`: guide and samples**: `docs/AMB.md`, links from
`README.md`, `docs/README.md`, `docs/Samples.md` and
`docs/AliasNames.md`; `samples/AMB/AmbServer` (AMB next to
`AddMachinery`: two DI devices and a Machinery press as assets, with a
simulation) and `samples/AMB/AmbClient`. The sample server listens on
62553, which no other sample uses.

6. **`feat(amb)`: tell the links users added from the links of the
manufacturer.** OPC 10000-110 gives a client no way to see which links
`RemoveLink` accepts (§10.5.4: only those `AddLink` created). As §10.5.1
lets vendors add metadata Properties to the links, the server gives
every link `AddLink` created, and every such link restored after a
restart, the `Boolean` Property `UserLink` (`true`), qualified with its
namespace of server-specific types, with a NodeId derived from the
persisted link identifier; declared links carry none, and `RemoveLink`
deletes it with the link. `DocumentationLinkRecord` gets the additive
`IsUserLink`, which `ReadDocumentationLinksAsync` fills with one Browse
over all links and one Read of the markers found (comparing the name
only, as the namespace is configurable; `false` for a server that does
not mark links). The name is shared as
`DocumentationLinkProperties.UserLink`.

`UA.slnx` and `.azurepipelines/expected-packages.txt` get the new
projects and packages in the commit that adds them.

All projects target `net48;net8.0;net9.0;net10.0` through the repository
properties (`LibTargetFrameworks`, `TestsTargetFrameworks`,
`AppTargetFrameworks`); `IsAotCompatible` and `PublishAot` are set from
net10.0 on, as in the other companion projects.

## Tests

- **`Opc.Ua.AMB.Tests`: 164 tests**, most of them against a hosted
server over real client sessions:
- the model: the SHA-256 of the NodeSet and the generator behaviours AMB
is the first companion model to rely on;
- each building block in-process and over a session: discovery through
every category with NodeVersion and model change events, AssetId writes,
health alarms and maintenance activities through all transitions with
acknowledgement, AddLink and RemoveLink, locations and structure, the
advertised conformance units and the facet;
  - a restart with the file system stores (links and AssetId survive);
- the `UserLink` marker: present on an added link only, the same NodeId
after a restart, removed with `RemoveLink`, and `IsUserLink` on the
client in the snapshot, after `AddLink`, after a restart and after
`RemoveLink`;
- composition with `AddOpcUaDi` (both startup orders), `AddPumps` and
`AddMachinery`;
- `AmbClient` end to end against a hosted server, including a Machinery
machine as an asset.
- **Results (macOS, local):** 164 passed / 0 failed on net10.0 and on
net8.0. .NET Framework is covered by the fork CI's Windows legs.
- **Coverage** of the three AMB assemblies (coverlet with
`tests/coverlet.runsettings.xml`, net10.0, generated `*.g.cs`,
`*TypeClient` proxies and `*Log` classes excluded): lines 93.9 % (4890
of 5206), branches 84.0 % (1294 of 1539); model 100 %, client 94.5 %,
server 93.4 %.
- **Build:** the three packages, the tests and the two samples build
without warnings for net48, net8.0, net9.0 and net10.0; `UA.slnx` builds
in Release (its warnings are all in files this PR does not touch, as on
master). `dotnet format` (whitespace, style, analyzers with `--severity
info`) reports nothing on the 95 changed C# files.
- **Samples:** `AmbServer` and `AmbClient` ran against each other on
net10.0: the client discovered the three assets, printed their snapshots
and the location hierarchies, added and removed a documentation link and
streamed alarms and maintenance.
- Fork CI (full matrix incl. macOS) and CodeQL: green on the same five
package commits before the rebase onto the current #4650 head — [CI run
37144486107](https://github.com/biancode/UA-.NETStandard/actions/runs/37144486107),
[CodeQL run
37144487873](https://github.com/biancode/UA-.NETStandard/actions/runs/37144487873).
The run on the current head (rebased, plus the user-link commit; AMB
tests 164/164 locally on net10.0) is in progress: [CI run
37203517457](https://github.com/biancode/UA-.NETStandard/actions/runs/37203517457).

## Notes for review

- **Size.** About 26.8k lines, larger than each of the companion PRs
#4577 to #4580. Without changes to the code it splits into the model,
server and server tests, and the client, end-to-end tests, samples and
docs on top; the issue asks which you prefer.
- **Interfaces on types.** §9.4.1 and §12.1 ask for alarm and condition
types that implement `IRootCauseIndicationType` and
`IMaintenanceEventType`. The server creates subtypes of the DI alarm
types in its own type namespace (`urn:opcua-netstandard:amb:types`);
`UseServerDefinedAlarmTypes = false` uses `HasInterface` on the instance
instead, and then does not report the units that need the type-level
interfaces.
- **Conformance unit names** follow Tables 54 to 56; the profile
database has no AMB entries yet. The QualifiedNames are in namespace 0,
as DI and Machinery publish theirs (question 5 in the issue).
- **Generator gap.** Children a NodeSet gives to a top-level instance
(here the `NodeVersion` properties of the alias categories) get no
identifier constants. The fix changes the identifiers of other generated
models, the reference server's among them, so it is not part of this PR;
`AmbModelTests.NodeVersionHasNoIdentifierConstant` records it and fails
once the generator emits the constants.
- **net48.** The code that the SDK so far only built for .NET 8 and
later uses the `NET5_0_OR_GREATER` pattern of `JsonEncoder` for
`Enum.IsDefined` and API forms that .NET Framework has (`string.Format`,
`Guid.ToString(format, provider)`); the tests use a small async file
helper instead of `File.WriteAllTextAsync`.
- **User links.** The `UserLink` Property is metadata of this server,
not an AMB name: commit 6 explains why and how a client uses it. Changed
lines of that commit covered: 69/70 (the one line left is the loop
continuation for a link that has other Properties but no marker).
- **Known limitations** are listed in the guide: the `DeviceHealth`
SourceTimestamp across a restart, removing optional maintenance members
once set, `PatchIdentifiers`, locations only at the deepest level, and
the history units.

## Related Issues

- Part of #4649
- Stacked on #4650

## Checklist

- [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. (The AMB
suite on net10.0 and net8.0 locally on macOS; .NET Framework is covered
by the fork CI's Windows net48 legs.)
- [ ] 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: Marc Schier <marcschier@users.noreply.github.com>
2026-10-05 07:45:46 +02:00