diff --git a/docs/plans/revision-write-path.md b/docs/plans/revision-write-path.md new file mode 100644 index 0000000..3cc9abb --- /dev/null +++ b/docs/plans/revision-write-path.md @@ -0,0 +1,145 @@ +# Plan: Revision-Write Path (`AddRevisionValuesBegin/Value/End`) + +Status: **NOT STARTED.** Sub-plan extracted from `speculative-items-sweep.md` +item D2 because the work is too large for a one-push sweep. + +## Context + +The Historian's "revision write" path is the documented mechanism for +editing historized data after the fact (replaces the inferred +`ModifyData` / `DeleteData` use cases that don't exist as WCF ops). Native +managed surface (per Phase 1 findings of the write-commands plan): + +| Public method | Token | Purpose | +|---|---|---| +| `ArchestrA.HistorianAccess.AddRevisionValuesBegin` | `0x06006175` | Open a revision-edit transaction | +| `ArchestrA.HistorianAccess.AddRevisionValue` | `0x06006176` | Append a value to the open transaction | +| `ArchestrA.HistorianAccess.AddRevisionValuesEnd` | `0x06006177` | Commit the transaction | +| `ArchestrA.HistorianAccess.AddRevisionValues` | `0x0600617F` | Single-shot variant | +| `ArchestrA.HistorianAccess.AddVersionedStreamedValue` | `0x0600616F` | Push one versioned value (related path) | + +WCF surface is unknown — likely a new op group on `IHistoryServiceContract2` +or `IRetrievalServiceContract4` or a new contract. + +## Goal + +Public SDK API: + +```csharp +public Task BeginRevisionAsync(string tag, CancellationToken ct); +// On the returned transaction: +public Task AddRevisionValueAsync(HistorianSampleEdit sample, CancellationToken ct); +public Task CommitAsync(CancellationToken ct); +// IDisposable / IAsyncDisposable for cancellation rollback if such a thing exists +``` + +Or a single batch convenience: + +```csharp +public Task WriteRevisionsAsync(string tag, IReadOnlyList samples, CancellationToken ct); +``` + +The choice depends on the wire shape — if Begin/Value/End requires the +caller to maintain a server handle between calls, the disposable +transaction is necessary; if it's stateless, the batch convenience is fine. + +## Workstreams + +### A. Static analysis (1-2 hours) + +- Inspect IL for the four managed public methods to identify the + underlying `CHistoryConnectionWCF.*` calls and their server-side WCF + contract methods. +- Add the contract methods to `Wcf/Contracts/IHistoryServiceContract2.cs` + (or a new contract if appropriate) with `[OperationContract(Name = "...")]` + + `[MessageParameter]` attributes once names are known. + +### B. Native harness extension (2-3 hours) + +- Add `--scenario revision-write` to the harness. +- Refer to existing `--scenario write` plumbing for the AddTag wrapper + pattern. +- Sequence: + 1. Open connection (probably write-enabled mode `0x401`) + 2. AddTag for sandbox tag (re-uses existing harness flow) + 3. AddStreamedValue for the initial sample (currently blocked + architecturally per Phase 2 findings — but may not be required if + the revision path operates directly on the historian engine state) + 4. AddRevisionValuesBegin / AddRevisionValue × N / AddRevisionValuesEnd + 5. Read back via existing read path; verify the samples reflect the + edits + +### C. Wire capture (1 hour) + +- Same `instrument-wcf-writemessage` + `instrument-wcf-readmessage` + IL-rewrite tooling already used for EnsT2 / DelT. +- Capture both Begin/Value/End and the single-shot AddRevisionValues + variant for byte-level diff. + +### D. Decode + managed serializer (4-6 hours) + +- Walk the captured InBuff bytes against the native serializer IL. +- The Begin payload likely seeds a server-side transaction handle that + Value calls reference. Look for an `out`-returned handle in the Begin + response. +- Value payload structure is likely similar to `AddS2`'s pBuf + (uint16 version + uint32 sampleCount + N × {tagId, FILETIME, quality, + typed value bytes}) but may include a per-sample revision/version field. + +### E. Public API + tests (4-6 hours) + +- New types: `HistorianSampleEdit` (sample + reason/version metadata), + `HistorianRevisionTransaction` (disposable handle). +- Public methods on `HistorianClient` per the Goal section. +- Unit tests: golden-byte fixtures for Begin/Value/End/Commit payloads. +- Live integration tests: write a known sample, edit it via the + revision path, read back and assert the new value appears. + +## Risks + +- **Server-cache prerequisite.** If the historian's revision path + also requires the tag to be "live in the runtime cache" (the same + blocker that killed `AddS2`), the entire path may be unimplementable + for the same architectural reason. +- **State across calls.** Begin/Value/End may store transaction state + on the server keyed by the WCF session GUID. WCF's session model + needs to be configured to keep the same channel alive across all + three calls — which is a different lifecycle from the existing + one-call-per-channel pattern in the SDK orchestrators. +- **Concurrent edits.** Server may reject concurrent revision + transactions on the same tag — needs probing. +- **Time bounds.** Revision likely respects the same `RealTimeWindow` + / `FutureTimeThreshold` system parameters as `AddS2`. Out-of-window + edits silently drop or error — needs probing. + +## Success Criteria + +- Public `BeginRevisionAsync` (or batch variant) live-verified against + a sandbox tag created by `EnsureTagAsync`. +- Round-trip test: write initial value → revise it → read back → verify + the revised value persists in `History` extension table via SQL. +- Golden-byte fixtures for Begin / Value / End / Commit captured against + the sandbox tag. +- Decision documented for whether the `AddRevisionValues` single-shot + variant is exposed in addition to the Begin/Value/End sequence. + +## Dependencies + +- Existing analog write surface (`EnsureTagAsync`) — done. +- `AddS2` is **not** a prerequisite; the revision path may be an + independent code path that bypasses the runtime-cache gate. If it + doesn't, this plan is blocked the same way `AddS2` is. + +## Out of scope + +- Editing event tags. Events come from AVEVA AnE; the SDK only reads + them. +- Bulk schema changes. Forbidden over the wire per the Historian's + architecture. + +## Trigger to start + +A customer-driven request, or a real need to expose historical data +correction in the SDK's API. Without one, this remains the most +substantive remaining write-path workstream but isn't worth the 1-2 +days of focused work speculatively. diff --git a/docs/plans/speculative-items-sweep.md b/docs/plans/speculative-items-sweep.md new file mode 100644 index 0000000..29f5571 --- /dev/null +++ b/docs/plans/speculative-items-sweep.md @@ -0,0 +1,134 @@ +# Plan: Speculative Items Sweep (2026-05-04) + +The five items I previously called out as speculative / deferred. Goal: +exercise the cheap ones, investigate the medium ones to feasibility, and +write sub-plans for anything too big to ship in one push. + +## Items + +| ID | Item | Effort | Touches | +|---|---|---|---| +| C3 | Expose `IntegralDivisor` on `HistorianTagDefinition` | small | `HistorianTagDefinition.cs`, `HistorianTagWriteProtocol.cs`, orchestrator, tests | +| E | Unit tests for `AllowUntrustedServerCertificate` / `ServerDnsIdentity` | small | new test file under `tests/` | +| D3 | Root-cause Discrete/String/Int1/Int8/UInt8 EnsT2 failure | medium (investigation) | native harness, possibly serializer | +| D1 | Capture wire bytes for `AddTagExtendedProperties` | medium (capture + decode) | native harness, possibly new serializer + public API | +| D2 | Implement `AddRevisionValuesBegin/Value/End` (revision-write path) | large | new orchestrator + 3 new public APIs | + +## Parallelism + +Concurrency-safe groupings (each pair is independent at the file level): + +- **C3 ↔ E** — C3 touches `HistorianTagDefinition.cs` + `HistorianTagWriteProtocol.cs` + orchestrator + integration tests; E adds a new test file + might add a small unit-test util. No file overlap. +- **D3 ↔ D1** — Both touch the native trace harness Program.cs, so they conflict if done concurrently. Sequence them. +- **C3/E ↔ D3/D1** — No file overlap; can run concurrently with the harness work. +- **D2** stands alone (different code paths entirely). + +In a single-agent session, the order is: + +1. C3 (small, predictable) — land first +2. E (small, predictable) — land second +3. D3 (investigation; documents findings whether or not implementation is possible) +4. D1 (investigation + capture; same pattern) +5. D2 — write a focused sub-plan; do NOT implement in this sweep + +## Success Criteria + +- C3: `HistorianTagDefinition.IntegralDivisor` (default 1.0) plumbed through serializer; unit test asserts non-default value flips the wire bytes; live test asserts the value persists in `Tag.IntegralDivisor` (or wherever it lands in SQL). +- E: 2-3 unit tests asserting `HistorianWcfClientCredentialsHelper.Configure` and `HistorianWcfBindingFactory.CreateEndpointAddress` honor the new options. +- D3: documented root cause + decision (workable path / not workable / requires further capture). If a workable path emerges quickly, also implement. +- D1: documented evidence summary + decision (worth implementing / defer / requires customer ask). +- D2: `docs/plans/revision-write-path.md` (or similar) with the 5-step capture sequence + open questions. + +## Findings + +### C3 — IntegralDivisor (executed 2026-05-05) + +Plumbed through serializer + orchestrator; default 1.0. Unit test verifies +the 8-byte double immediately preceding the trailer flips with a non-default +value. Live probe: server accepts the wire bytes but `IntegralDivisor` +appears to be stored on `EngineeringUnit` (shared across all tags using that +EU) rather than per-tag, and the EU's stored value didn't change for the +test. Documented in the property's doc-comment. No live integration test +added (nothing to assert in SQL). + +### E — Cert option unit tests (executed 2026-05-05) + +Added `HistorianWcfCertOptionTests` (5 tests) covering: + +- `Configure` is a no-op when `AllowUntrustedServerCertificate=false` +- `Configure` installs the accept-any validator + `RevocationMode.NoCheck` + when the option is true +- `CreateEndpointAddress` with no DNS identity returns an address with + `Identity == null` +- `CreateEndpointAddress` with a DNS identity attaches a `DnsEndpointIdentity` +- `CreateBindingPair(RemoteTcpCertificate)` propagates `ServerDnsIdentity` + to the History endpoint (and not to the Retrieval endpoint, which uses + plain MdasNetTcp without TLS) + +### D3 — Discrete/String/Int1/Int8/UInt8 EnsT2 root cause (investigated 2026-05-05) + +Probed each unsupported type via the native trace harness with +`--write-data-type {Type}`. Result for SingleByteString and Int1 (others +truncated in the same output): + +- `HistorianAccess.AddTag` returns `Success=false`, `TagKey=0` +- Error: `ErrorCode=ValidationFailed`, `ErrorType=CustomError`, + `ErrorDescription="Transaction validation failed"` + +**Conclusion:** The failure is **server-side**, not wire-format. The +`/Hist.EnsT2` server-side validator rejects non-analog types when invoked +through the `AddTag → EnsT2` code path. To unlock these types from the SDK +we'd need to: + +1. Capture a successful native creation of a discrete/string tag via some + other mechanism (likely SMC's tag-import path or a different WCF op + like `AddTagExtendedProperties` carrying the discrete/string-specific + metadata) +2. Diff the working native flow against the failing one to see what + ancillary fields the validator expects (TagType vs CDataType, separate + StorageType, IO-server pre-registration, etc.) + +**Decision:** defer until a customer asks. The native AVEVA wrapper itself +cannot create these tags via `AddTag` from a managed client — implementing +this would require RE work on a path the wrapper doesn't exercise, which +is much higher risk than the existing analog write surface. + +### D1 — AddTagExtendedProperties feasibility (investigated 2026-05-05) + +Managed surface confirmed. Native API: + +- Public managed entry point: `ArchestrA.HistorianAccess.AddTagExtendedProperties` + (token `0x0600619B`, 140 IL instructions, 6 locals) +- WCF op: `CHistoryConnectionWCF.AddTagExtendedPropertyGroups` + (token `0x0600405C`) +- Underlying contract method: `IHistoryServiceContract2.AddTagExtendedProperties` + (already declared in our reproduced contracts) +- Managed input type: `HistorianTagExtendedPropertyGroup` wrapping the native + `CTagExtendedPropertyGroup` C++ class. Built from a `std::vector` + (visible in the IL locals). Property group structure not yet decoded. + +**Decision:** defer implementation. Cost estimate: + +1. Reflect-construct `HistorianTagExtendedPropertyGroup` via the native + harness (probably 2-4 hours — these C++/CLI types often have hidden + constructor requirements that surface only at runtime). +2. Call `AddTagExtendedProperties` with a sandbox group; capture wire bytes + via `instrument-wcf-writemessage` (1 hour). +3. Decode the `CTagExtendedPropertyGroup` payload — this is its own struct + that needs walking field-by-field against the native serializer IL + (token `0x06002038`, `CHistStorage.AddTagExtendedProperties`) (3-6 hours). +4. Implement managed `HistorianTagExtendedPropertyGroup` model + serializer + + public `AddTagExtendedPropertiesAsync` API + tests (4-6 hours). + +Total: 1-2 days of focused work. Defer until a customer asks for tag +extended properties or the analog write surface needs them as a +prerequisite. + +### D2 — AddRevisionValuesBegin/Value/End + +Sub-plan deferred to a dedicated session — see +`docs/plans/revision-write-path.md` (created in this sweep). + +## Out of scope for this sweep + +Refactoring `HistorianWcfTagClient` to respect `options.Transport` for browse / metadata (i.e., let it use cert binding from Windows). Worth doing but not part of the speculative-items list. diff --git a/src/AVEVA.Historian.Client/Models/HistorianTagDefinition.cs b/src/AVEVA.Historian.Client/Models/HistorianTagDefinition.cs index 6e49e75..be5bb54 100644 --- a/src/AVEVA.Historian.Client/Models/HistorianTagDefinition.cs +++ b/src/AVEVA.Historian.Client/Models/HistorianTagDefinition.cs @@ -69,4 +69,16 @@ public sealed record HistorianTagDefinition /// (Cyclic = 1, Delta = 2). /// public HistorianStorageType StorageType { get; init; } = HistorianStorageType.Cyclic; + + /// + /// Divisor applied when storing integral values for trend integration. Default 1.0. + /// Wire bytes flip correctly per the captured native serializer, but live testing + /// 2026-05-05 showed the server stores IntegralDivisor on + /// EngineeringUnit (shared across all tags using that EU) rather than + /// per-tag — so a non-default value sent here is accepted on the wire but does + /// not visibly persist in EngineeringUnit.IntegralDivisor for the test + /// EU. Exposed for completeness and forward-compatibility; check your server's + /// behavior before relying on it. + /// + public double IntegralDivisor { get; init; } = 1.0; } diff --git a/src/AVEVA.Historian.Client/Wcf/HistorianTagWriteProtocol.cs b/src/AVEVA.Historian.Client/Wcf/HistorianTagWriteProtocol.cs index caa08a2..a45faf6 100644 --- a/src/AVEVA.Historian.Client/Wcf/HistorianTagWriteProtocol.cs +++ b/src/AVEVA.Historian.Client/Wcf/HistorianTagWriteProtocol.cs @@ -145,7 +145,8 @@ internal static class HistorianTagWriteProtocol double maxRaw = DefaultMaxRaw, uint storageRateMs = DefaultStorageRateMs, bool applyScaling = false, - Models.HistorianStorageType storageType = Models.HistorianStorageType.Cyclic) + Models.HistorianStorageType storageType = Models.HistorianStorageType.Cyclic, + double integralDivisor = 1.0) { if (storageRateMs == 0) { @@ -188,7 +189,7 @@ internal static class HistorianTagWriteProtocol WriteCompactAscii(w, engineeringUnit ?? string.Empty); // var w.Write(IntegralDivisorMagic); // uint32 (purpose unclear — captured constant) - w.Write(1.0); // double + w.Write(integralDivisor); // double IntegralDivisor (default 1.0) w.Write(applyScaling ? AnalogTrailerScalingEnabled : AnalogTrailerScalingDisabled); return ms.ToArray(); diff --git a/src/AVEVA.Historian.Client/Wcf/HistorianWcfTagWriteOrchestrator.cs b/src/AVEVA.Historian.Client/Wcf/HistorianWcfTagWriteOrchestrator.cs index ba62a9b..b43f72a 100644 --- a/src/AVEVA.Historian.Client/Wcf/HistorianWcfTagWriteOrchestrator.cs +++ b/src/AVEVA.Historian.Client/Wcf/HistorianWcfTagWriteOrchestrator.cs @@ -110,7 +110,8 @@ internal sealed class HistorianWcfTagWriteOrchestrator maxRaw: definition.MaxRaw, storageRateMs: definition.StorageRateMs, applyScaling: definition.ApplyScaling, - storageType: definition.StorageType); + storageType: definition.StorageType, + integralDivisor: definition.IntegralDivisor); bool ok = historyChannel.EnsureTags2( handle: handle, diff --git a/tests/AVEVA.Historian.Client.Tests/HistorianTagWriteProtocolTests.cs b/tests/AVEVA.Historian.Client.Tests/HistorianTagWriteProtocolTests.cs index 41b5625..bee045f 100644 --- a/tests/AVEVA.Historian.Client.Tests/HistorianTagWriteProtocolTests.cs +++ b/tests/AVEVA.Historian.Client.Tests/HistorianTagWriteProtocolTests.cs @@ -181,6 +181,33 @@ public sealed class HistorianTagWriteProtocolTests Assert.Equal(cyclic[11], delta[11]); } + [Fact] + public void SerializeAnalogCTagMetadata_NonDefaultIntegralDivisor_FlipsEightBytesBeforeTrailer() + { + byte[] @default = HistorianTagWriteProtocol.SerializeAnalogCTagMetadata( + tagName: "RetestSdkWriteIntDiv", + description: "x", + engineeringUnit: "test", + dateCreatedUtc: DateTime.FromFileTimeUtc(0x01dcdc34_5a1dff6dL)); + byte[] custom = HistorianTagWriteProtocol.SerializeAnalogCTagMetadata( + tagName: "RetestSdkWriteIntDiv", + description: "x", + engineeringUnit: "test", + dateCreatedUtc: DateTime.FromFileTimeUtc(0x01dcdc34_5a1dff6dL), + integralDivisor: 2.5); + + Assert.Equal(@default.Length, custom.Length); + // The 8 bytes immediately before the 2-byte trailer are the IntegralDivisor double. + ReadOnlySpan defaultDivisor = @default.AsSpan(@default.Length - 10, 8); + ReadOnlySpan customDivisor = custom.AsSpan(custom.Length - 10, 8); + Assert.Equal(1.0, BitConverter.ToDouble(defaultDivisor)); + Assert.Equal(2.5, BitConverter.ToDouble(customDivisor)); + // Bytes preceding the divisor are identical. + Assert.Equal( + Convert.ToHexString(@default.AsSpan(0, @default.Length - 10)), + Convert.ToHexString(custom.AsSpan(0, custom.Length - 10))); + } + [Fact] public void SerializeAnalogCTagMetadata_ApplyScalingTrue_FlipsTrailerSecondByte() { diff --git a/tests/AVEVA.Historian.Client.Tests/HistorianWcfCertOptionTests.cs b/tests/AVEVA.Historian.Client.Tests/HistorianWcfCertOptionTests.cs new file mode 100644 index 0000000..d484541 --- /dev/null +++ b/tests/AVEVA.Historian.Client.Tests/HistorianWcfCertOptionTests.cs @@ -0,0 +1,97 @@ +using System.IdentityModel.Selectors; +using System.Security.Cryptography.X509Certificates; +using System.ServiceModel; +using System.ServiceModel.Channels; +using System.ServiceModel.Security; +using AVEVA.Historian.Client; +using AVEVA.Historian.Client.Wcf; +using AVEVA.Historian.Client.Wcf.Contracts; + +namespace AVEVA.Historian.Client.Tests; + +public sealed class HistorianWcfCertOptionTests +{ + private static HistorianClientOptions BaseOptions(bool allowUntrusted = false, string? dnsIdentity = null) => + new() + { + Host = "10.0.0.1", + Port = HistorianClientOptions.DefaultPort, + Transport = HistorianTransport.RemoteTcpCertificate, + IntegratedSecurity = false, + UserName = "user", + Password = "pass", + AllowUntrustedServerCertificate = allowUntrusted, + ServerDnsIdentity = dnsIdentity, + }; + + [Fact] + public void ClientCredentialsHelper_Disabled_LeavesValidationModeAtDefault() + { + Binding binding = HistorianWcfBindingFactory.CreateMdasNetTcpBinding(TimeSpan.FromSeconds(5)); + ChannelFactory factory = new(binding, new EndpointAddress("net.tcp://10.0.0.1:32568/Hist")); + try + { + HistorianWcfClientCredentialsHelper.Configure(factory, BaseOptions(allowUntrusted: false)); + + X509ServiceCertificateAuthentication auth = factory.Credentials.ServiceCertificate.SslCertificateAuthentication + ?? factory.Credentials.ServiceCertificate.Authentication; + // Default validation mode is ChainTrust — explicitly NOT None / Custom. + Assert.NotEqual(X509CertificateValidationMode.None, auth.CertificateValidationMode); + Assert.Null(auth.CustomCertificateValidator); + } + finally + { + factory.Abort(); + } + } + + [Fact] + public void ClientCredentialsHelper_Enabled_InstallsAcceptAnyValidator() + { + Binding binding = HistorianWcfBindingFactory.CreateMdasNetTcpBinding(TimeSpan.FromSeconds(5)); + ChannelFactory factory = new(binding, new EndpointAddress("net.tcp://10.0.0.1:32568/Hist")); + try + { + HistorianWcfClientCredentialsHelper.Configure(factory, BaseOptions(allowUntrusted: true)); + + X509ServiceCertificateAuthentication auth = factory.Credentials.ServiceCertificate.SslCertificateAuthentication; + Assert.NotNull(auth); + Assert.Equal(X509CertificateValidationMode.Custom, auth.CertificateValidationMode); + Assert.Equal(X509RevocationMode.NoCheck, auth.RevocationMode); + Assert.NotNull(auth.CustomCertificateValidator); + Assert.IsAssignableFrom(auth.CustomCertificateValidator); + } + finally + { + factory.Abort(); + } + } + + [Fact] + public void CreateEndpointAddress_WithoutDnsIdentity_HasNullIdentity() + { + EndpointAddress address = HistorianWcfBindingFactory.CreateEndpointAddress("10.0.0.1", 32568, "Hist"); + Assert.Null(address.Identity); + } + + [Fact] + public void CreateEndpointAddress_WithDnsIdentity_AttachesDnsEndpointIdentity() + { + EndpointAddress address = HistorianWcfBindingFactory.CreateEndpointAddress("10.0.0.1", 32568, "HistCert", "localhost"); + Assert.NotNull(address.Identity); + DnsEndpointIdentity dns = Assert.IsType(address.Identity); + Assert.Equal("localhost", dns.IdentityClaim.Resource); + } + + [Fact] + public void CreateBindingPair_RemoteTcpCertificate_PropagatesServerDnsIdentity() + { + HistorianClientOptions options = BaseOptions(dnsIdentity: "localhost"); + var (_, historyEndpoint, _, retrievalEndpoint) = HistorianWcfBindingFactory.CreateBindingPair(options); + + DnsEndpointIdentity historyIdentity = Assert.IsType(historyEndpoint.Identity); + Assert.Equal("localhost", historyIdentity.IdentityClaim.Resource); + // The Retrieval endpoint uses plain MdasNetTcp without TLS — no DNS identity needed. + Assert.Null(retrievalEndpoint.Identity); + } +}