Skip to content

Initial commit. - #2

Merged
nathanpocock merged 1 commit into
OPCFoundation:masterfrom
barnstee:master
Mar 2, 2016
Merged

Initial commit.#2
nathanpocock merged 1 commit into
OPCFoundation:masterfrom
barnstee:master

Conversation

@barnstee

Copy link
Copy Markdown
Member

No description provided.

nathanpocock added a commit that referenced this pull request Mar 2, 2016
@nathanpocock
nathanpocock merged commit 4029526 into OPCFoundation:master Mar 2, 2016
@hamduc7 hamduc7 mentioned this pull request May 15, 2023
5 tasks
marcschier added a commit that referenced this pull request Jun 1, 2026
…eviceSet

Bug: Pump #1 was referenced in PumpNodeManager.Configure.cs via
  builder.Node("Pumps/Pump #1/...") but never created anywhere.
  Every wiring call threw BadNodeIdUnknown which was silently swallowed by
  try/catch blocks, so the entire fluent simulation pipeline was dead code.
  Only Pump #2 (declared via ConfigureDevicesFor in Program.cs) actually
  existed, with no measurements / supervision / simulation.

Fix: in PumpNodeManager.CreateAddressSpaceAsync, after the base call but
  before the fluent Configure step, materialise a typed PumpState
  (OPC 40223 PumpType) named "Pump #1" as a child of the DI DeviceSet.
  Uses the source-generated CreateInstanceOfPumpType factory + the
  manager's NodeIdFactory + AddPredefinedNodeAsync.

  Then strip the "Pumps/" prefix from every browse path in Configure.cs
  (Pumps/Pump #1/... -> Pump #1/...) so the fluent root resolver finds
  the freshly-created device by browse name in PredefinedNodes.

  Pump #2 stays as-is in Program.cs, demonstrating the parallel DI
  declarative path (CreateDeviceAsync + WithIdentification).

README: update browse instruction to Objects > DeviceSet > Pump #1.
  Rewrite the "Add a second pump" section to document both patterns
  (hand-rolled CreateInstanceOfPumpType vs DI ConfigureDevicesFor).

Verified locally: server starts on port 62599 and logs
  Materialised 'Pump #1' (PumpType) under DeviceSet, NodeId=ns=4;s=5001_Pump #1.
  followed by PumpNodeManager: address space ready (10140 predefined nodes).
  No skipping/path-not-found messages remain.
marcschier added a commit that referenced this pull request Jun 1, 2026
…SU sample

Resolves PR feedback threads 3333417092 + 3333374009:
* Renames Applications/MinimalPumpServer/ -> Applications/PumpDeviceIntegrationServer/
  with matching csproj AssemblyName + PackageId. RootNamespace stays 'Pumps'.
* Lifts the WithSoftwareUpdate(...) wiring + SoftwarePackageSeeder from
  the now-deleted MinimalSoftwareUpdateServer into the pump server's
  Program.cs, attached to the declarative Pump #2.
* Deletes Applications/MinimalSoftwareUpdateServer/ outright (csproj +
  Program.cs + Properties + README all gone; the seeder moves to the
  pump server with the namespace switched from SoftwareUpdate to Pumps).
* Updates UA.slnx to drop the old project entries and add the new one.
* Updates Tests/Opc.Ua.Di.Tests.csproj ProjectReference path + comment.
* Updates Docs/README.md, DeviceIntegration.md, SoftwareUpdate.md (incl.
  the Walkthrough section), and SourceGeneratedNodeManagers.md to point
  at the new sample location.
* Updates two doc-comment references in
  Libraries/Opc.Ua.Server/Fluent/ReferenceBuilderExtensions.cs and one
  in Tests/Opc.Ua.Di.Tests/PumpsNodeSetLoadingTests.cs.

Validation:
* dotnet build PumpDeviceIntegrationServer.csproj -c Debug -f net10.0: 0 errors
* dotnet build Opc.Ua.Di.Tests.csproj -c Debug -f net10.0: 0 errors
* dotnet vstest with TestCategory=Pumps|TestCategory=Hosting|FullyQualifiedName~SoftwareUpdate:
  88/88 tests pass.
* Server boots on the new endpoint
  'opc.tcp://localhost:62611/PumpDeviceIntegrationServer'.
marcschier added a commit that referenced this pull request Jun 1, 2026
Resolves PR feedback threads 3333380238 / 3333396295 (DeviceHealth
simulation): the pump sample exercised supervision flags
(cavitation / motor-overheat) but never reflected them on the DI
DeviceHealth variable that NAMUR NE 107 clients expect to observe.

Important model nuance: PumpType inherits from TopologyElementType
(not DeviceType), so PumpState does NOT carry DeviceHealth.
Pump #1 stays unchanged. Instead the simulation drives the
DeviceHealth variable on Pump #2 (a declarative DeviceState device
created via ConfigureDevicesFor), reusing the source-generator
AddDeviceHealth() helper.

Manager-side (PumpDeviceIntegrationServer/PumpNodeManager.cs):
* New 'public void RegisterSupervisedDeviceHealth(
    BaseDataVariableState<DeviceHealthEnumeration>?)' to attach an
  externally-materialised health variable to the simulation loop.
* New supervised-health field on PumpNodeManager.Configure.cs
  (alongside the existing simulation state).
* New 'public static DeviceHealthEnumeration
    MapSupervisionToDeviceHealth(bool cavitation, bool motorOverheat)'
  pure helper applying NAMUR NE 107 severity ordering:
  FAILURE > MAINTENANCE_REQUIRED > NORMAL. Motor overheat wins
  when both flags are active.
* AdvanceSimulation now calls UpdateDeviceHealth() each tick which
  ClearChangeMasks-es the registered variable on transition so
  subscriptions receive the NORMAL ↔ MAINTENANCE_REQUIRED ↔ FAILURE
  flow.

Program.cs:
* After WithIdentification on Pump #2, calls pump.Device.AddDeviceHealth
  (source-gen helper), pump.WithDeviceHealth(NORMAL) to seed the
  value, and casts ctx.Manager to PumpNodeManager to register the
  variable with the simulation loop.

Tests/Opc.Ua.Di.Tests/PumpDeviceHealthMappingTests.cs (new):
* 4 unit tests on the pure mapping helper covering NORMAL,
  MAINTENANCE_REQUIRED (cavitation-only), FAILURE (motor-overheat-
  only), and FAILURE-wins (both flags) severity ordering.

Validation:
* dotnet build PumpDeviceIntegrationServer + Opc.Ua.Di.Tests
  (net10.0): 0 errors.
* dotnet vstest TestCategory=Pumps|Hosting: 48/48 pass (was 44; the
  4 new mapping tests join the suite).
* Smoke test: server boots clean on
  opc.tcp://localhost:62612/PumpDeviceIntegrationServer with the
  RegisterSupervisedDeviceHealth wiring active and no warnings about
  missing DeviceHealth child.
marcschier added a commit that referenced this pull request Jun 2, 2026
Address 7 of the 16 unresolved review comments from round 3:

- #6 Tools/Opc.Ua.SourceGeneration/ModelCompilation.cs:300 - add
  `using System.Collections.Generic` and
  `using Opc.Ua.SourceGeneration.Dependency` so the file no longer
  spells fully-qualified type names inline. Same sweep for the
  `System.Collections.Generic.`/`Opc.Ua.SourceGeneration.Dependency.`
  prefixes at other call sites in the file.
- #7 Tools/Opc.Ua.SourceGeneration.Core/Generators.cs:634 (and 3 other
  sites in FluentBuilderGenerator + GeneratorOptions + ModelCompilationOptions)
  - drop `FB-3 phase 3` / phase / track references from comments and
  xmldoc; describe the feature, not the rollout.
- #9 Libraries/Opc.Ua.Di.Client/Hosting/OpcUaClientDiBuilderExtensions.cs:69
  - re-wrap the long `<item><description>...</description></item>` doc
  list to fit inside the 140-char editorconfig limit.
- #12 Strip `global::` qualification from all hand-written PR code
  (Libraries/Opc.Ua.Di.*, Applications/PumpDeviceIntegrationServer).
  Only kept where required to disambiguate (test-side `Pumps.PumpNodeManager`
  / `Pumps.PumpNodeManagerFactory` collide with the source-generated
  `Opc.Ua.Pumps` namespace; restored `global::` on those refs).
  Hand-written code now matches the .editorconfig convention of letting
  `using` directives carry the namespace burden; source-generated code
  still uses `global::` (unchanged).
- #15 Remove `// -----` / `// =====` ASCII-banner `#region`-style
  comments from SoftwareUpdateClient.StateMachine.cs,
  SoftwareUpdateClient.Upload.cs, SoftwareUpdateFacetWiring.cs and
  SoftwareUpdateFileTransferManager.cs. Inline section comments
  remain unchanged.
- #16 Expand 344 one-line `/// <summary>text</summary>` declarations
  across 72 files in Applications/, Libraries/, Stack/, Tests/ and
  Tools/ into the three-line
  `/// <summary>\n/// text\n/// </summary>` form.
- Carry-over from FB-3 phase 3: while sweeping `global::` also
  un-qualify the hand-written Pumps server callsites
  (Pump #1 wiring in PumpNodeManager.cs / PumpNodeManager.Configure.cs)
  so the new code matches the reviewer's convention.

Validation:
- `dotnet build UA.slnx` — clean (0 errors, 15 unrelated warnings
  reported transiently by the build engine).
- `Opc.Ua.SourceGeneration.Core.Tests` — 3535 passed / 8 skipped.
- `Opc.Ua.Di.Tests` (Pump + DI client + SoftwareUpdate filter) —
  85/85 passed.

The remaining 9 round-3 items (architecture relocation #8,
`=>`→`{}` body sweep #10, ObjectType-proxy refactor #11/#14,
brace-formatting sweep #13, pump server Configure refactor #1/#2,
DeviceHealth + functional-group exposure #3/#5 plus DI doc
disambiguation #4) are tracked separately and will land in
subsequent commits.
marcschier pushed a commit that referenced this pull request Jun 2, 2026
Mirror the sentinel-keep-alive pattern from the stable sister test

ProcessReceivedMessagesAsyncShouldProcessMessagesInOrderAsync so the

worker is parked inside the keep-alive dispatch before any data

messages are queued. Without the sentinel, a fast consumer could read

the first arrival (e.g. #3) before the rest queued, leaving the

priority channel nothing to re-order; later #1/#2 arrivals were then

dropped as stale and the test would observe [2,3,4,5] instead of

[1,2,3,4,5].

Also updated the XML remark to accurately describe the

priority-channel guarantee (re-orders items present at the same read,

not 'any burst regardless of consumer speed') and asserted on the

sentinel + reordered tail separately.
marcschier added a commit that referenced this pull request Jun 2, 2026
…uilder + ad-hoc functional group

Addresses 3 PR review threads from round 3:

- FB-R3-1 (PumpNodeManager.cs:133) — split OnAddressSpaceReadyAsync
  into two clearly-named phases: an async ConfigureInstancesAsync(ct)
  that materialises predefined instances (currently Pump #1) and a
  synchronous fluent Configure(builder) pass that wires callbacks.
  CreatePumpInstanceAsync renamed to MaterialisePumpInstanceAsync to
  match the new phase boundary. The reviewer's suggestion to use the
  DiNodeManager.CreateDeviceAsync<TDevice> helper does not apply here
  because PumpType in OPC 40223 derives from MachineType (Machinery),
  not from the DI ComponentType hierarchy that CreateDeviceAsync's
  generic constraint requires — documented in the xmldoc on the new
  ConfigureInstancesAsync partial.
- FB-R3-2 (PumpNodeManager.cs:150) — collapse the six-arg
  `new NodeManagerBuilder(SystemContext, this, nsIndex, lookup, lookup, lookup)` +
  `AttachToBuilder(builder)` + `Configure(builder)` + `builder.Seal()`
  quadruple to a single fluent chain. New
  `FluentNodeManagerBase.CreateFluentBuilder(ushort nsIndex)` helper
  constructs the builder, wires the three default PredefinedNodes
  resolvers, and attaches the event-source + simulation registries.
  New `NodeManagerBuilder.Configure(Action<INodeManagerBuilder>)`
  extension preserves chaining. Pump callsite now reads
  `this.CreateFluentBuilder(nsIndex).Configure(Configure).Seal();`.
- FB-R3-5 (DeviceIntegration.md:230) — demonstrate the non-typed
  `WithFunctionalGroup(QualifiedName)` builder on Pump #2 with an
  ad-hoc `Diagnostics` functional group that surfaces LastError /
  ErrorCount / LastSelfTest as plain properties. Doc snippet appended
  to Docs/DeviceIntegration.md §'Functional groups'.

New public surface in Opc.Ua.Server.Fluent:
- FluentNodeManagerBase.CreateFluentBuilder(ushort nsIndex)
- FluentNodeManagerBuilderExtensions.Configure(NodeManagerBuilder, Action<INodeManagerBuilder>)

Validation:
- Full build: 0 errors, 0 warnings (server + pump server + tests).
- Tests/Opc.Ua.Di.Tests pump suite — 14/14 passing.
- Tests/Opc.Ua.Server.Tests fluent suite — 247/247 passing.
marcschier added a commit that referenced this pull request Jun 5, 2026
- Move all channel-related types into `Stack/Opc.Ua.Core/Stack/Client/Channels/`
  subfolder (#13).
- Un-obsolete `SessionReconnectHandler` and the SessionExtensions
  `ReconnectAsync(connection, ct)` / `(channel, ct)` extensions; remove
  all SRH-related `#pragma warning disable CS0618` suppressions from
  applications and tests. AttachChannel/DetachChannel remain obsolete (#1, #4).
- Remove the shared-budget section from MigrationGuide.md (#3).
- Mark `ManagedSession.AttachChannel`/`DetachChannel` `[Obsolete]` so the
  warning surfaces all the way up the supported API surface (#9).
- Break the `<see cref=…>` line in `Session.ChannelManager.cs` to stay under
  140 chars (#10).
- Move `ChannelManagerSessionFactory` from `Opc.Ua.Gds.Client.Common` into
  `Opc.Ua.Client` as a public, documented session-factory option (#11).
- Audit other client classes for `IClientChannelManager`-aware overloads;
  add `ISessionFactory`-accepting overloads on `GlobalDiscoveryServerClient`,
  `LocalDiscoveryServerClient`, `ServerPushConfigurationClient` so any
  session factory (including `ChannelManagerSessionFactory`) works (#12).
- Refactor MCP server (`OpcUaSessionManager`, `Program`) to use
  `ManagedSession` + DI-resolved `IClientChannelManager`; remove
  `SessionReconnectHandler` and manual keep-alive reconnect (#2).
- ConnectionStateMachine code-style fixes: use named delegate types,
  collapse multi-line callback properties to single lines, swap
  `timeProvider`/`maxTotalReconnectTime` constructor param order,
  fix multi-line declaration (#6, #7, #8).
- HTTPS resilience: confirmed `Microsoft.Extensions.Http` and
  `Microsoft.Extensions.Http.Resilience` 10.6.0 support
  `net472`/`net48`/`netstandard2.1`; removed `#if NET8_0_OR_GREATER`
  gating from `ManagedSessionBuilder` and `OpcUaClientBuilderExtensions`
  and HTTPS csproj package refs (#5).

Verification:
- `Opc.Ua.Client` + `Opc.Ua.Core` + `Opc.Ua.Gds.Client.Common` build
  clean (0 warnings, 0 errors).
- 56/56 core channel-manager / retry-budget / HTTPS factory tests pass.
- 129/129 client (ManagedSession + legacy SRH back-compat + SessionExtensions)
  tests pass.
marcschier added a commit that referenced this pull request Jun 11, 2026
Phase 1 (security-agent MCP, svc 59eec07a-...) + Phase 2 (security-
review SAST sub-agent on the PR diff) + Phase 3 (deep-dive) findings
on branch `vsubs`. All CRITICAL/HIGH/MEDIUM findings fixed in-line;
the MCP secrets-usage, code-integrity, policy-governance, and tls
scans report no PR-relevant items.

CRITICAL #1 — Unbounded partition fan-out under hostile server
---------------------------------------------------------------
A hostile or buggy server returning `Bad_TooManyMonitoredItems`
on every `CreateMonitoredItems` reply caused the wrapper to mint
a fresh server-side `Subscription` per `TryAdd`, growing client
memory and server-side handle space without bound.
`MaxMonitoredItemsPerPartition = null` (the documented default)
made the per-partition cap unlimited, so the reactive fallback was
the only growth gate and it kept minting forever.

Fix: introduce `SubscriptionOptions.MaxPartitionCount` (default
`32`, comfortably above legit fan-out), wire it through
`PartitionPlacementPolicy`, and stop minting at the cap. The
composite collection returns the new
`PlacementDecision.RejectMaxPartitionCountReached` state and
`TryAdd` returns `false` — the caller observes the standard
add-failed path.

HIGH #2 — Snapshot consistency across LogicalGroupId not validated
-------------------------------------------------------------------
`ManagedSessionExtensions.ValidateAndSortGroup` checked only
`PartitionIndex` contiguity. Two snapshots crafted to share a
`LogicalGroupId` but disagree on durability / publishing
interval / lifetime would be merged into one logical wrapper
with inconsistent partitions — letting a corrupted or attacker-
controlled snapshot file smuggle non-durable behaviour into a
durable subscription (or vice versa) at restore time.

Fix: extend `ValidateAndSortGroup` to assert all snapshots in
the group agree on every subscription-wide option
(`PublishingIntervalMs`, `KeepAliveCount`, `LifetimeCount`,
`MaxNotificationsPerPublish`, `MinLifetimeIntervalMs`,
`Priority`, `PublishingEnabled`, `Disabled`,
`SendInitialValuesOnTransfer`). Mismatches throw
`BadDecodingError` with a descriptive message.

MEDIUM #3 — Restore-path secondaries bypass forwarding handler
---------------------------------------------------------------
`SubscriptionManager.AttachSecondaryPartitionAsync` (snapshot-
restore fan-out path) passed the raw user handler directly to
`MintPreloadedPartition` instead of the wrapper's
`PartitionForwardingHandler`. This broke the documented
single-threaded dispatch contract for restored multi-partition
subscriptions (per `Docs/UnboundedSubscriptions.md`): the
serialising semaphore was bypassed and the handler's
`subscription` parameter pointed at the partition instead of
the logical wrapper. Application handlers that mutate non-thread-
safe state could silently race after restore.

Fix: expose `LogicalSubscription.ForwardingHandler` as an
internal accessor; `AttachSecondaryPartitionAsync` now uses the
wrapper's existing forwarding handler when present (falling back
to the raw user handler only when the wrapper was constructed in
single-partition fast-path mode, i.e. `DisableUnboundedItemMode
= true`).

Validation (net10.0)
--------------------
* `Opc.Ua.Client.csproj` builds clean (0 warnings, 0 errors).
* `Opc.Ua.Client.Tests` V2 subscription unit tests: **43 / 43**.
* `Opc.Ua.Subscriptions.Tests.UnboundedSubscriptionTests`
  integration tests: **3 / 3** (1 durable-only skipped).
* `Opc.Ua.Subscriptions.Durable.Tests.*V2Async` integration
  tests: **6 / 6**.

Findings dismissed
------------------
* Affinity dictionary growth (`PartitionPlacementPolicy`) —
  uses `StringComparer.Ordinal` so no CVE-2022-29117-class
  culture sensitivity; growth is application-controlled, not
  attacker-controlled.
* Idle-timer race vs `DisposeAsync` — race window exists but
  worst observable outcome is an `ObjectDisposedException`
  swallowed by the disposer's `try/catch`. No attacker trigger
  and no integrity/confidentiality loss.
* Cross-wrapper notification routing — each `Add` allocates its
  own `PartitionForwardingHandler`; `BindLogical` rejects
  re-binding; routing keys are partition server-id → partition's
  own handler; no structural path for partition of wrapper B to
  reach wrapper A's handler.
* OPC UA spec compliance — `SetTriggering` enforces same-
  partition (Part 4 §5.13.6); durable hook ordering (Part 4
  §5.13.9) is uniform across primary and secondaries.
marcschier added a commit that referenced this pull request Jun 11, 2026
…t doc (audit follow-up #2)

Expanded the Certificates fuzz corpus from 9 seeds to 21 (+ 12) across
all five categories. The security assessment doc flagged the corpus as
"adequate but limited diversity" -- this commit closes the gap.

Generator (Fuzzing/Opc.Ua.Certificates.Fuzz.Tools/Certificates.Testcases.cs)
now produces, in addition to the original RSA application cert / CRL /
CSR / PEM / extension seeds:

  X509Cert/   certificate-expired.der        RSA cert with NotAfter in past
              certificate-selfsigned.der     RSA cert with no separate issuer
              certificate-ecc.der            P-256 ECC cert with ECC issuer

  X509CRL/    crl-multi-revoked.der          CRL listing 3 revoked entries
              crl-empty.der                  CRL with no revoked entries

  X509CSR/    request-ecc.der                ECC-signed PKCS#10 CSR
              request-multi-san.der          CSR with 4 SAN entries

  PEM/        certificate-ecc.pem            ECC PEM cert
              private-key-ecc.pem            ECC PEM private key

  ASN1/       nested-sequence.der            SEQUENCE containing inner SEQUENCE + OCTET STRING
              large-integer.der              256-bit BigInteger (past AsnReader fast paths)
              subject-alt-name-many.der      SAN with 9 entries (5 DNS + 4 IP)

Exercises code paths the original corpus missed:
- ECDsa decode branches in cert + PEM + PKCS#10 parsers
- CRL revoked-list loop with multiple entries + empty-list boundary
- Validity-window logic for expired cert
- Self-signed (no chain) decode path
- Multi-entry SAN decode loop
- AsnReader.ReadInteger multi-limb branch (past 32 / 64-bit fast paths)
- Nested OCTET STRING containing further DER content

A dedicated ECC issuer ("CN=Fuzzing Test ECC Root") is constructed
because CertificateBuilder.CreateForECDsa() signs with the issuer's
ECDsa private key, so an RSA-only issuer is rejected.

Verified (net10.0):
  - Opc.Ua.Certificates.Fuzz.Tests: 460 passed (up from 220), 0 failed.
    The doubling reflects the 12 new seeds * the harness's
    Fuzz*Assets/FuzzGoodTestcases theory expansion across the 20 cert
    targets.
  - Opc.Ua.Encoders.Fuzz.Tests baseline: 4131/4131 passed.
  - Opc.Ua.Network.Fuzz.Tests baseline: 330/330 passed.
  - Combined: 4921 passed, 0 failed.

DER seed files are committed via `git add -f` because the repo-wide
`*.der` gitignore would otherwise exclude them (matches the original
F2 commit's precedent of force-adding cert test seeds).

Side effect: deleted plans/security-assessment.md. The doc was an
untracked working-copy audit summary that has now had every item
addressed:
  - 7.1 #1 (MUST FIX, parser bound): commit 7744146
  - 7.2 #2 (corpus diversity):       this commit
  - 7.2 #3 (IOORE whitelist):        commit 80832f1
  - 7.3 future work:                 explicitly out-of-scope, tracked
                                     in the PR description instead.

Audit ref: section 7.2 #2 of the security assessment doc.
marcschier added a commit that referenced this pull request Jun 11, 2026
CI on PR #3869 was green for all functional tests after re-running
the 4 flaky failures (aot-windows `BrowseAndReadComplexTypesAsync`,
test-ubuntu-Subscriptions.Durable `DurableSubscriptionSurvivesSession
CloseV2Async`, test-windows-History
`NonExclusiveLimitAlarmActiveNormalCycleAsync`, test-windows-PubSub),
all confirmed unrelated to branch changes (also fail on master CI).

The only remaining gate was `codecov/patch` reporting 68.23% of
diff hit against an 80% target (with 5pp tolerance ⇒ 75% effective
threshold). Closes the gap by adding focused unit tests on the
lowest-coverage new code paths from the security commit (#3869
`c6997689`):

* `PartitionPlacementPolicyTests` (+5 tests):
  - `ConstructorTreatsZeroMaxPartitionCountAsUnbounded`
  - `ConstructorPreservesExplicitMaxPartitionCount`
  - `DecideRejectsOnceMaxPartitionCountReached`
  - `DecideAllowsExistingPartitionEvenAtMaxPartitionCount`
  - `DecideAllowsMintUntilMaxPartitionCount`
  Cover the CRITICAL #1 DoS guard end-to-end: the
  `maxPartitionCount` constructor parameter, the new
  `MaxPartitionCount` accessor, and the
  `RejectMaxPartitionCountReached` decision path.

* `SnapshotGroupValidationTests` (NEW file, 14 tests):
  - `ValidateAndSortGroupSortsByPartitionIndex`
  - `ValidateAndSortGroupAcceptsSinglePartition`
  - `ValidateAndSortGroupRejectsDuplicatePartitionIndex`
  - `ValidateAndSortGroupRejectsPartitionIndexGap`
  - 8 individual mismatch checks (one per option:
    `PublishingIntervalMs`, `KeepAliveCount`, `LifetimeCount`,
    `MaxNotificationsPerPublish`, `MinLifetimeIntervalMs`,
    `Priority`, `PublishingEnabled`, `Disabled`,
    `SendInitialValuesOnTransfer`)
  - `ValidateAndSortGroupAcceptsAllOptionsMatching`
  Cover every branch of the HIGH #2 snapshot-trust guard.
  `ManagedSessionExtensions.ValidateAndSortGroup` promoted from
  `private static` to `internal static` so it is reachable
  from `Opc.Ua.Client.Tests` (existing `InternalsVisibleTo`).

Validation (net10.0):
* `Opc.Ua.Client` builds clean.
* `Opc.Ua.Client.Tests` V2 subscription unit tests: **62 / 62**
  (previous 43 + 5 new placement policy + 14 new snapshot
  validation).

Re-runs:
* The 4 flaky tests above all PASS on re-run with no code change
  to them, confirming pre-existing CI flakiness (also observed on
  master in commits `4d613d11` and `a81d9817`).
marcschier added a commit that referenced this pull request Jun 15, 2026
… + rename AddOpcUaBindingsPcap → AddPcapBinding

Five review comments on Docs/migrate/2.0.x/sessions-subscriptions.md
addressed:

1. (line 344) 'These were added in 2.0, no need to discuss migration' —
   dropped the 'Two additive surface additions for the new WSS
   reverse-connect path' block and the 'Channel-customization hooks'
   block. They documented additive 2.0-new surface, not migration
   guidance from 1.5.378.

2. (line 347) 'Just document the removal of IMessageSocket abstraction,
   drop the Transports: WSS and HTTPS-JSON' — renamed the section
   heading from 'Transports: WSS and HTTPS-JSON, IMessageSocket
   removed' to 'IMessageSocket abstraction removed' and dropped the
   intro paragraph about the new WSS / HTTPS-JSON transport profiles
   (also additive, covered by Docs/Profiles.md / WhatsNewIn2.0.md).
   Kept the IMessageSocket removal table + custom-IMessageSocket
   migration note as the actual migration content.

3. (line 349) 'Remove this section' — same intent, addressed by #2.

4. (line 434) 'Remove this section since Pcap package is new in 2.0' —
   dropped the 'Opc.Ua.Bindings.Pcap consumers' block from the
   Transport-binding-registry section + the 'PcapBindings.Install'
   line from the DI consumers code snippet. Pcap is new in 2.0 so it
   has no migration story from 1.5.378.

5. (line 446) 'Rename AddOpcUaBindingsPcap() to AddPcapBinding(). In
   code, in this PR, no obsoletion required since it is new' — code
   rename: AddOpcUaBindingsPcap → AddPcapBinding. For symmetry the
   sister methods follow the same convention:
   - AddOpcUaBindingsPcap            → AddPcapBinding
   - AddOpcUaBindingsPcapFormatters  → AddPcapFormatters
   - AddOpcUaBindingsPcapReplay      → AddPcapReplay
   All callers updated: PcapServiceCollectionExtensions.cs,
   PcapBindings.cs (xmldoc), PcapTransportChannelBinding.cs (xmldoc),
   ChannelCaptureRegistry.cs (xmldoc), IKeyEscrowProvider.cs (xmldoc),
   NugetREADME.md, Applications/McpServer/Program.cs,
   Tests/Opc.Ua.Bindings.Pcap.Tests/.../*Tests.cs, Docs/PacketCapture.md.
   No [Obsolete] shim added per review instruction (the package is new
   in 2.0 and has no prior consumers to break).

Verification:
- 'dotnet build Tests/Opc.Ua.Bindings.Pcap.Tests': clean.
- 20/20 Pcap DI extension tests pass.
marcschier added a commit that referenced this pull request Jun 18, 2026
…ion-guide trim, SKS/GDS clarification

R1 (review #2/#3) — migrate modern public PubSub API collections to ArrayOf<T>
(repo-preferred): EventPublishedDataSet result, IPubSubApplication.ReplaceConfiguration
Async -> ArrayOf<StatusCode>, snapshot/message Fields, JsonNetworkMessage.ReplyTo,
UadpApplicationInformation.*, UADP discovery lists, validation Issues, SksKeyResponse/
SksSecurityGroup lists, WriterGroup/ReaderGroup/connection group views, metadata/key-ring/
policy registries. Kept PubSubApplication.Connections as a live IReadOnlyList view (ArrayOf
would copy on each access; commented). [Obsolete] shims + internal transport lists untouched.

R2 (review #4) — replace public-API tuples with named record structs: PubSubErrorEntry
(PubSubDiagnostics.RecentErrors -> ArrayOf<PubSubErrorEntry>, LastError -> PubSubErrorEntry?),
AesCtrNonceComponents (AesCtrNonceLayout.Parse), UadpHeaderByteParts (UadpFlagsEncodingMask
.Split), and WriterGroup/DataSetWriter/ReaderGroup/DataSetReader key record structs for
PubSubConfigurationSnapshot composite dictionary keys (equality/hashing preserved).

R3 (review #8) — trim Docs/migrate/2.0.x/pubsub.md to breaking changes only (keep
obsoletion, AMQP removal, STJ encoder swap, enum rename, RawData padding, content-mask,
compat matrix; remove additive sections, renumber 1-7); relocate KeepAlive + DataSetReader
filter/timeout detail to PubSub.md; fix inbound cross-links in README/WhatsNewIn2.0/migrate
README.

R4 (review #13) — investigation (files/sks-gds-dedup.md): Part 14 SKS and Part 12 Key
Credential are legitimately separate; broad unification not worth the coupling/risk. Applied
the recommended PubSub.md SKS wording clarification; deferred optional narrow helpers.

Verification: all 4 PubSub libs build net10 + net48 0/0; samples + fuzz build clean;
PubSub.Tests 1250, Udp 140, Mqtt 133, Server 141 - all pass.
marcschier added a commit that referenced this pull request Jun 22, 2026
…, docs merge

#4 (csproj): McpServer now needs only the Opc.Ua.PubSub.Diagnostics project
reference; it transitively brings PubSub/Udp/Mqtt. Removed the 3 redundant
direct refs.

#3 (architecture): replace the in-transport capture taps with a DI-injected
capturing decorator so the UDP/MQTT transports carry NO capture code (mirrors
the UA-SC CapturingMessageSocket):
- New CapturingPubSubTransport (wraps IPubSubTransport, taps send/receive to the
  observer, delegates the rest) + CapturingPubSubTransportFactory.
- AddPubSubPcap() decorates every registered IPubSubTransportFactory; capture is
  injected only when DI-configured (call after AddUdpTransport/AddMqttTransport).
- Reverted the S1 taps from UdpDatagramTransport/MqttBrokerTransport, their
  factories, and core AddPubSub; moved the capture seam types
  (IPubSubCaptureObserver/Registry, PubSubCaptureContext/Direction) into the
  Diagnostics lib (namespace Opc.Ua.PubSub.Pcap). Moved the seam test + added a
  decorator test. McpServer PubSubRuntimeManager updated to the reverted factory.

#1, #2 (docs): merged Docs/PubSubDiagnostics.md into Docs/Diagnostics.md as
section 5 (with the decorator architecture); the in-doc MCP-tools section now
links to McpServer.md, which gained a PubSub tool catalogue. Deleted the
standalone file; updated README link + UA.slnx.

Builds: diagnostics lib net8/9/10 0/0, MCP net10 0/0, core PubSub net48 0/0.
Tests: diagnostics 48 (incl. decorator), PubSub 1058, no regressions.
marcschier added a commit that referenced this pull request Jul 22, 2026
…f<Type> factories

Learns from master #4066 (which fixed the Pump sample to create Pump #2 as the generated
PumpType instead of a bare DeviceState): the Robot sample created MotionDeviceSystem/
Controller/MotionDevice/Axis as bare BaseObjectState + a type-definition reference, so the
instances lacked the companion-type's mandatory structure. Now uses
CreateInstanceOfMotionDeviceSystemType/ControllerType/MotionDeviceType/AxisType and the
typed mandatory containers (MotionDevices/Controllers/Axes), keeping the demo signals and
OpenUSD bindings (referenced by NodeId). Removed the unused RoboticsServer.CreateTypedObject
library helper that embodied the same anti-pattern. All 11 RobotOpenUsdE2eTests pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants