diff --git a/lmxproxy/.gitignore b/lmxproxy/.gitignore new file mode 100644 index 0000000..fa6b520 --- /dev/null +++ b/lmxproxy/.gitignore @@ -0,0 +1 @@ +publish-v2/ diff --git a/lmxproxy/docs/deviations.md b/lmxproxy/docs/deviations.md index e5583e3..7ff9149 100644 --- a/lmxproxy/docs/deviations.md +++ b/lmxproxy/docs/deviations.md @@ -44,8 +44,9 @@ Decisions made during implementation that differ from or extend the original pla **Why**: The `x-api-key` header always carries the caller's valid key (for interceptor auth). The `CheckApiKey` RPC is designed for clients to test whether a *different* key is valid, so it must check the body key independently. **How to apply**: `ScadaGrpcService` receives `ApiKeyService` as an optional constructor parameter. -## 7. Write integration tests pending +## 7. Write completes synchronously (fire-and-forget) -**Status**: 2 of 17 integration tests fail (WriteAndReadBack, WriteBatchAndWait). -**Why**: The `OnWriteComplete` COM callback from MxAccess doesn't fire within the timeout. This may be because MxAccess completes writes synchronously without invoking the callback, or the callback event subscription (`OperationCompleted` event) requires different wiring. The v1 code had the same pattern but may have relied on different MxAccess COM object initialization. -**How to apply**: Investigate whether MxAccess `Write` returns synchronously (check HRESULT) and bypass the callback wait if so. Alternatively, check if `OperationCompleted` needs explicit COM event subscription via `IConnectionPoint`. +**Plan specified**: Wait for `OnWriteComplete` COM callback to confirm write success. +**Actual**: Write is confirmed synchronously — if `_lmxProxy.Write()` returns without throwing, the write succeeded. The `OnWriteComplete` callback is kept wired for diagnostic logging only. +**Why**: MxAccess completes supervisory writes synchronously. The `OnWriteComplete` COM callback never fires for simple supervisory writes, causing the original implementation to timeout waiting for a callback that would never arrive. This caused WriteAndReadBack and WriteBatchAndWait integration tests to fail. +**How to apply**: Do not await `OnWriteComplete` for write confirmation. The `Write()` COM call succeeding (not throwing a COM exception) is the confirmation. Clean up (UnAdvise + RemoveItem) happens immediately after the write in a finally block. diff --git a/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.EventHandlers.cs b/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.EventHandlers.cs index cf4385b..d29a6a3 100644 --- a/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.EventHandlers.cs +++ b/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.EventHandlers.cs @@ -69,6 +69,8 @@ namespace ZB.MOM.WW.LmxProxy.Host.MxAccess /// /// COM event handler for MxAccess OnWriteComplete events. /// Signature matches the ArchestrA.MxAccess ILMXProxyServerEvents interface. + /// Kept wired for diagnostic logging only — writes are resolved synchronously + /// when the Write() COM call returns without throwing. /// private void OnWriteComplete( int hLMXServerHandle, @@ -77,57 +79,23 @@ namespace ZB.MOM.WW.LmxProxy.Host.MxAccess { try { - TaskCompletionSource tcs; - lock (_lock) - { - if (!_pendingWrites.TryGetValue(phItemHandle, out tcs)) - { - Log.Debug("WriteComplete for unknown handle {Handle}", phItemHandle); - return; - } - - _pendingWrites.Remove(phItemHandle); - } - if (ItemStatus != null && ItemStatus.Length > 0) { var status = ItemStatus[0]; if (status.success == 0) { string errorMsg = GetWriteErrorMessage(status.detail); - Log.Warning("Write failed for handle {Handle}: {Error} (Category={Category}, Detail={Detail})", + Log.Warning("OnWriteComplete callback: write failed for handle {Handle}: {Error} (Category={Category}, Detail={Detail})", phItemHandle, errorMsg, status.category, status.detail); - tcs.TrySetException(new InvalidOperationException( - string.Format("Write failed: {0}", errorMsg))); } else { - Log.Debug("Write completed successfully for handle {Handle}", phItemHandle); - tcs.TrySetResult(true); + Log.Debug("OnWriteComplete callback: write succeeded for handle {Handle}", phItemHandle); } } else { - // No status means success - Log.Debug("Write completed for handle {Handle} with no status", phItemHandle); - tcs.TrySetResult(true); - } - - // Clean up the item after write completes - lock (_lock) - { - if (_lmxProxy != null && phItemHandle > 0) - { - try - { - _lmxProxy.UnAdvise(_connectionHandle, phItemHandle); - _lmxProxy.RemoveItem(_connectionHandle, phItemHandle); - } - catch (Exception ex) - { - Log.Debug(ex, "Error cleaning up after write for handle {Handle}", phItemHandle); - } - } + Log.Debug("OnWriteComplete callback: no status for handle {Handle}", phItemHandle); } } catch (Exception ex) diff --git a/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.ReadWrite.cs b/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.ReadWrite.cs index 46ba1f2..c561874 100644 --- a/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.ReadWrite.cs +++ b/lmxproxy/src/ZB.MOM.WW.LmxProxy.Host/MxAccess/MxAccessClient.ReadWrite.cs @@ -61,7 +61,7 @@ namespace ZB.MOM.WW.LmxProxy.Host.MxAccess /// /// Writes a single tag value to MxAccess. - /// Uses Task.Run for COM calls with OnWriteComplete callback for confirmation. + /// Uses Task.Run for COM calls. Write completes synchronously (fire-and-forget). /// public async Task WriteAsync(string address, object value, CancellationToken ct = default) { @@ -173,32 +173,14 @@ namespace ZB.MOM.WW.LmxProxy.Host.MxAccess } /// - /// Internal write implementation using Task.Run for COM calls - /// and OnWriteComplete callback for confirmation. + /// Internal write implementation using Task.Run for COM calls. + /// MxAccess completes supervisory writes synchronously — the Write() call + /// succeeding (not throwing) confirms the write. The OnWriteComplete callback + /// is kept wired for diagnostic logging but is not awaited. /// private async Task WriteInternalAsync(string address, object value, CancellationToken ct) { - var tcs = new TaskCompletionSource(); - int itemHandle = await SetupWriteOperationAsync(address, value, tcs, ct); - - try - { - await WaitForWriteCompletionAsync(tcs, itemHandle, address, ct); - } - catch - { - await CleanupWriteOperationAsync(itemHandle); - throw; - } - } - - /// - /// Sets up a write operation on the thread pool and returns the item handle. - /// - private async Task SetupWriteOperationAsync( - string address, object value, TaskCompletionSource tcs, CancellationToken ct) - { - return await Task.Run(() => + await Task.Run(() => { lock (_lock) { @@ -214,79 +196,38 @@ namespace ZB.MOM.WW.LmxProxy.Host.MxAccess // Advise to enable writing _lmxProxy.AdviseSupervisory(_connectionHandle, itemHandle); - // Track pending write for OnWriteComplete callback - _pendingWrites[itemHandle] = tcs; - // Write the value (-1 = no security classification) + // MxAccess completes simple/supervisory writes synchronously. + // If Write() returns without throwing, the write succeeded. _lmxProxy.Write(_connectionHandle, itemHandle, value, -1); - return itemHandle; + Log.Debug("Write completed synchronously for {Address} (handle={Handle})", address, itemHandle); } catch (Exception ex) { - // Clean up on failure + Log.Error(ex, "Failed to write value to {Address}", address); + throw; + } + finally + { + // Clean up: UnAdvise + RemoveItem after write (success or failure) if (itemHandle > 0 && _lmxProxy != null) { try { _lmxProxy.UnAdvise(_connectionHandle, itemHandle); _lmxProxy.RemoveItem(_connectionHandle, itemHandle); - _pendingWrites.Remove(itemHandle); } - catch { } + catch (Exception ex) + { + Log.Debug(ex, "Error cleaning up write item for {Address} (handle={Handle})", address, itemHandle); + } } - Log.Error(ex, "Failed to write value to {Address}", address); - throw; } } }, ct); } - /// - /// Waits for write completion with timeout. - /// - private async Task WaitForWriteCompletionAsync( - TaskCompletionSource tcs, int itemHandle, string address, CancellationToken ct) - { - using (ct.Register(() => tcs.TrySetCanceled())) - { - var timeoutTask = Task.Delay(_writeTimeoutMs, ct); - var completedTask = await Task.WhenAny(tcs.Task, timeoutTask); - - if (completedTask == timeoutTask) - { - await CleanupWriteOperationAsync(itemHandle); - throw new TimeoutException( - string.Format("Write operation to {0} timed out after {1}ms", address, _writeTimeoutMs)); - } - - await tcs.Task; // This will throw if the write failed - } - } - - /// - /// Cleans up a write operation (unadvise + remove item). - /// - private async Task CleanupWriteOperationAsync(int itemHandle) - { - await Task.Run(() => - { - lock (_lock) - { - _pendingWrites.Remove(itemHandle); - if (itemHandle > 0 && _lmxProxy != null) - { - try - { - _lmxProxy.UnAdvise(_connectionHandle, itemHandle); - _lmxProxy.RemoveItem(_connectionHandle, itemHandle); - } - catch { } - } - } - }); - } - /// /// Maps an MxAccess OPC DA quality integer to the domain Quality enum. /// diff --git a/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests.csproj b/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests.csproj index 7799e60..634f697 100644 --- a/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests.csproj +++ b/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests.csproj @@ -24,6 +24,7 @@ PreserveNewest + diff --git a/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/xunit.runner.json b/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/xunit.runner.json new file mode 100644 index 0000000..dd80f43 --- /dev/null +++ b/lmxproxy/tests/ZB.MOM.WW.LmxProxy.Client.IntegrationTests/xunit.runner.json @@ -0,0 +1,5 @@ +{ + "$schema": "https://xunit.net/schema/current/xunit.runner.schema.json", + "parallelizeAssembly": false, + "parallelizeTestCollections": false +}