From ae9f906f525b4e6fabe13fbb67861a1204ae931b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 16 Jul 2026 03:56:19 -0400 Subject: [PATCH] review(b2-waveB): strip endpoint keys from driver-form config (H1) so DeviceConfig is sole endpoint source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer H1 (v3 regression): the endpoint→DeviceConfig split ADDED the endpoint to DeviceConfig but the driver forms still serialized it into DriverConfig at defaults. Single-device worked only by an STJ case-insensitive last-key-wins accident; a 2nd device on a single-top-level-Host driver (Modbus/S7) silently reverted to the driver-form default (host=127.0.0.1). Fix: Modbus/S7/OpcUaClient driver-form GetConfigJson strips the endpoint keys (host/port/unitId; host/port/rack/slot; scalar endpointUrl) from the serialized channel config, leaving DeviceConfig as the sole source (OpcUaClient keeps its endpointUrls failover list, which is driver-level policy). Enum-serialization verified CLEAN fleet-wide; ImportTagsAsync all-or-nothing verified; RawTree wiring sound. Deferred (documented in PR): H2 pre-existing Modbus/S7 form TimeSpan vs factory *Ms-int DTO mismatch (authored durations dropped — predates v3); M1 CSV export drops non-columned alarm sub-keys; M2 blank flag column clobbers a base-JSON isHistorized/isArray; M3 OpcUaClient device/driver endpoint precedence; L1 unused LoadMergedProbeConfigAsync; L5 refresh collapses sibling expansion. Claude-Session: https://claude.ai/code/session_01LVneM3eh1UtJxEisFXgmox --- .../Shared/Drivers/Forms/ModbusDriverForm.razor | 17 ++++++++++++++--- .../Drivers/Forms/OpcUaClientDriverForm.razor | 11 ++++++++++- .../Shared/Drivers/Forms/S7DriverForm.razor | 16 +++++++++++++--- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor index da9036ba..f0959d1a 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/ModbusDriverForm.razor @@ -3,6 +3,7 @@ ModbusDriverPage and by DriverConfigModal. Exposes GetConfigJson() for the parent to pull at save + Test-connect time, and fires DriverConfigJsonChanged for @bind support. *@ @using System.Text.Json +@using System.Text.Json.Nodes @using System.Text.Json.Serialization @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.Driver.Modbus @@ -241,10 +242,20 @@ } } - /// Serializes the current channel/protocol config to camelCase JSON (endpoint excluded from the UI, - /// carried at harmless defaults; the device's DeviceConfig merges its endpoint up at deploy/probe time). + // v3: the endpoint lives on the Device (DeviceConfig), not the driver. Strip the endpoint keys from + // the serialized channel config so DeviceConfig is the SOLE endpoint source — otherwise a stale + // driver-level default (host=127.0.0.1) is carried in DriverConfig and shadows the device on any + // driver that reads a single top-level Host, silently, as soon as a 2nd device exists (reviewer H1). + private static readonly string[] EndpointKeys = ["host", "port", "unitId"]; + + /// Serializes the current channel/protocol config to camelCase JSON, with the endpoint keys + /// removed (the device's DeviceConfig is the sole endpoint source, merged up at deploy/probe time). public string GetConfigJson() - => JsonSerializer.Serialize(_form.ToOptions(), _jsonOpts); + { + var node = JsonSerializer.SerializeToNode(_form.ToOptions(), _jsonOpts)!.AsObject(); + foreach (var key in EndpointKeys) node.Remove(key); + return node.ToJsonString(_jsonOpts); + } private async Task EmitAsync() { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor index 53f0611c..1dace2bb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/OpcUaClientDriverForm.razor @@ -2,6 +2,7 @@ OpcUaClientDeviceForm (Device.DeviceConfig) in v3; the EndpointUrls FAILOVER LIST stays here (it is a driver-level policy). Hosted by the routed page + DriverConfigModal. *@ @using System.Text.Json +@using System.Text.Json.Nodes @using System.Text.Json.Serialization @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient @@ -232,7 +233,15 @@ } public string GetConfigJson() - => JsonSerializer.Serialize(_form.OpcUa.ToRecord(_endpoints.Select(r => r.ToUrl()).ToList()), _jsonOpts); + { + // v3 (reviewer H1): the convenience scalar endpoint lives on the Device (DeviceConfig). Strip the + // scalar "endpointUrl" so the device is its sole source; the "endpointUrls" failover list stays + // here (it is a driver-level policy and wins over the scalar at runtime when populated). + var node = JsonSerializer.SerializeToNode( + _form.OpcUa.ToRecord(_endpoints.Select(r => r.ToUrl()).ToList()), _jsonOpts)!.AsObject(); + node.Remove("endpointUrl"); + return node.ToJsonString(_jsonOpts); + } private async Task EmitAsync() { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/S7DriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/S7DriverForm.razor index d450f73b..da936e66 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/S7DriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/S7DriverForm.razor @@ -3,6 +3,7 @@ S7DriverPage and by DriverConfigModal. Exposes GetConfigJson() for the parent to pull at save + Test-connect time, and fires DriverConfigJsonChanged for @bind support. *@ @using System.Text.Json +@using System.Text.Json.Nodes @using System.Text.Json.Serialization @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.Driver.S7 @@ -98,10 +99,19 @@ } } - /// Serializes the current protocol config to camelCase JSON (endpoint excluded from the UI, - /// carried at harmless defaults; the device's DeviceConfig merges its endpoint up at deploy/probe time). + // v3: the endpoint lives on the Device (DeviceConfig). Strip the endpoint keys so DeviceConfig is the + // sole endpoint source — a stale driver-level default would otherwise shadow the device on the + // single-top-level-Host read path as soon as a 2nd device exists (reviewer H1). + private static readonly string[] EndpointKeys = ["host", "port", "rack", "slot"]; + + /// Serializes the current protocol config to camelCase JSON, with the endpoint keys removed + /// (the device's DeviceConfig is the sole endpoint source, merged up at deploy/probe time). public string GetConfigJson() - => JsonSerializer.Serialize(_form.ToOptions(), _jsonOpts); + { + var node = JsonSerializer.SerializeToNode(_form.ToOptions(), _jsonOpts)!.AsObject(); + foreach (var key in EndpointKeys) node.Remove(key); + return node.ToJsonString(_jsonOpts); + } private async Task EmitAsync() {