Skip to content

feat(network): add LAN static IP/subnet/gateway configuration (closes #205)#211

Merged
tylerkron merged 3 commits into
mainfrom
claude/sweet-bhabha-15bb07
May 16, 2026
Merged

feat(network): add LAN static IP/subnet/gateway configuration (closes #205)#211
tylerkron merged 3 commits into
mainfrom
claude/sweet-bhabha-15bb07

Conversation

@tylerkron
Copy link
Copy Markdown
Contributor

Summary

  • Add nullable StaticIP / SubnetMask / Gateway (IPAddress?) to NetworkConfiguration, with a 7-arg constructor and Clone() carrying them; null = "leave unchanged" so DHCP-only callers see no behavior change.
  • Add ScpiMessageProducer.SetLanAddress / SetLanMask / SetLanGateway (quoted form) plus Get* and GetLanConfigured* query variants matching the firmware's SYSTem:COMMunicate:LAN:{ADDRess,MASK,GATEway} and :CONFigure: endpoints.
  • Wire the three setters into DaqifiStreamingDevice.UpdateNetworkConfigurationAsync between password and APPLY (firmware reads runtime WiFi settings at APPLY time — see SCPILAN.c#L531). Only non-null fields are emitted.
  • Update INetworkConfigurable step-list docstring.

Closes #205. Parent: #52 (Phase 7.1).

Firmware verification

Cross-checked against daqifi-nyquist-firmware:

  • SCPI_LANAddrSet / MaskSet / GatewaySet accept the address as a SCPI_SafeParamString payload (quoted form is what existing SSID code uses and what the firmware accepts).
  • SCPI_LANSettingsApply calls wifi_manager_UpdateNetworkSettings(pRunTimeWifiSettings), so the setters MUST run before APPLY — not just before SAVE as the issue text loosely states.

Test plan

  • dotnet build Daqifi.Core.sln — clean (no new warnings).
  • dotnet test Daqifi.Core.sln — 986 passed / 0 failed on net9.0 and net10.0.
  • New SCPI producer tests (round-trip + null-arg guards + query variants).
  • New NetworkConfiguration tests (defaults, full ctor, assign/clear, Clone with and without static IPs).
  • New INetworkConfigurable tests (full static IP, none → no static commands, partial → only non-null commands, local-cache update, ordering: ADDRess/MASK/GATEway all precede APPLY).
  • Manual: send full static-IP UpdateNetworkConfigurationAsync against a real Nyquist device and confirm the device comes back up on the requested address after restart.

🤖 Generated with Claude Code

…205)

Extend NetworkConfiguration with nullable StaticIP/SubnetMask/Gateway,
add SetLanAddress/Mask/Gateway producers (and Get/GetConfigured query
variants), and stage the static-IP commands in
UpdateNetworkConfigurationAsync between password and APPLY so the
firmware's runtime WiFi settings hold them when LAN:APPLY runs. Null
fields are skipped so DHCP-only flows are unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tylerkron tylerkron requested a review from a team as a code owner May 16, 2026 04:06
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Add LAN static IP/subnet/gateway configuration support

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add nullable StaticIP, SubnetMask, Gateway properties to NetworkConfiguration with 7-arg
  constructor and Clone() support
• Implement SetLanAddress, SetLanMask, SetLanGateway SCPI producers with null-argument guards
• Add GetLanAddress, GetLanMask, GetLanGateway and GetLanConfigured* query variants for
  reading LAN settings
• Stage static IP commands in UpdateNetworkConfigurationAsync between password and APPLY, skipping
  null fields for DHCP-only flows
• Update INetworkConfigurable docstring to document static IP configuration step
Diagram
flowchart LR
  NC["NetworkConfiguration<br/>+ StaticIP?<br/>+ SubnetMask?<br/>+ Gateway?"]
  SCPI["ScpiMessageProducer<br/>+ SetLanAddress<br/>+ SetLanMask<br/>+ SetLanGateway<br/>+ Get* queries"]
  DSD["DaqifiStreamingDevice<br/>UpdateNetworkConfigurationAsync"]
  FW["Firmware<br/>SYSTem:COMMunicate:LAN"]
  
  NC -- "7-arg ctor + Clone" --> SCPI
  SCPI -- "quoted form commands" --> DSD
  DSD -- "stage before APPLY" --> FW
Loading

Grey Divider

File Changes

1. src/Daqifi.Core/Device/Network/NetworkConfiguration.cs ✨ Enhancement +67/-2

Add static IP properties and extended constructor

• Add three nullable IPAddress? properties: StaticIP, SubnetMask, Gateway with XML
 documentation
• Add 7-argument constructor accepting WiFi settings plus static IP parameters
• Update Clone() method to preserve all three static IP fields
• Update existing 4-argument constructor docstring to clarify it handles WiFi settings only

src/Daqifi.Core/Device/Network/NetworkConfiguration.cs


2. src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs ✨ Enhancement +106/-0

Add LAN address/mask/gateway SCPI producers and queries

• Add SetLanAddress(IPAddress), SetLanMask(IPAddress), SetLanGateway(IPAddress) methods with
 null-argument validation
• Add GetLanAddress, GetLanMask, GetLanGateway query properties for active settings
• Add GetLanConfiguredAddress, GetLanConfiguredMask, GetLanConfiguredGateway query properties
 for staged settings
• Include comprehensive XML documentation for all new methods explaining firmware behavior and
 command format

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs


3. src/Daqifi.Core/Device/DaqifiStreamingDevice.cs ✨ Enhancement +19/-0

Stage static IP commands before APPLY in network update

• Insert conditional static IP command staging in UpdateNetworkConfigurationAsync between password
 and APPLY
• Skip null fields to preserve DHCP-only behavior for callers not providing static IP
• Update local _networkConfiguration cache with all three static IP fields after APPLY
• Add inline comment explaining firmware timing requirement

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs


View more (4)
4. src/Daqifi.Core/Device/Network/INetworkConfigurable.cs 📝 Documentation +1/-0

Document static IP configuration step in interface

• Add new step to docstring list describing static IP, subnet mask, and gateway configuration
• Clarify that only non-null fields are sent to device

src/Daqifi.Core/Device/Network/INetworkConfigurable.cs


5. src/Daqifi.Core.Tests/Communication/Producers/ScpiMessageProducerTests.cs 🧪 Tests +91/-0

Add SCPI producer tests for LAN address/mask/gateway

• Add 9 new test cases for SetLanAddress, SetLanMask, SetLanGateway covering command format
 and null-argument exceptions
• Add 6 new test cases for GetLan* and GetLanConfigured* query properties verifying correct SCPI
 syntax
• Import System.Net for IPAddress usage

src/Daqifi.Core.Tests/Communication/Producers/ScpiMessageProducerTests.cs


6. src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs 🧪 Tests +100/-0

Add NetworkConfiguration static IP property tests

• Add test for 7-argument constructor with static IP fields
• Add test for assigning and clearing static IP properties
• Add two tests for Clone() method: one preserving static IP values, one preserving null values
• Update default constructor test to verify static IP fields are null
• Import System.Net for IPAddress usage

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs


7. src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs 🧪 Tests +108/-0

Add network configuration static IP integration tests

• Add test verifying static IP commands (ADDRess, MASK, GATEway) are sent before APPLY
• Add test confirming no static IP commands sent when all fields are null
• Add test verifying only non-null static IP fields are sent (partial configuration)
• Add test confirming local device cache is updated with static IP values after command execution

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented May 16, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (2)

Grey Divider


Action required

1. NetworkConfiguration copy constructor missing 📎 Requirement gap ≡ Correctness
Description
The updated NetworkConfiguration adds static LAN fields but does not implement a copy constructor
to preserve StaticIP/SubnetMask/Gateway during duplication as required by the checklist. This
can cause callers expecting copy-constructor semantics to lose the new fields or be unable to
round-trip them consistently.
Code

src/Daqifi.Core/Device/Network/NetworkConfiguration.cs[R96-128]

+        public NetworkConfiguration(
+            WifiMode mode,
+            WifiSecurityType securityType,
+            string ssid,
+            string password,
+            IPAddress? staticIP,
+            IPAddress? subnetMask,
+            IPAddress? gateway)
+        {
+            Mode = mode;
+            SecurityType = securityType;
+            Ssid = ssid;
+            Password = password;
+            StaticIP = staticIP;
+            SubnetMask = subnetMask;
+            Gateway = gateway;
+        }
+
        /// <summary>
        /// Creates a copy of this network configuration.
        /// </summary>
        /// <returns>A new <see cref="NetworkConfiguration"/> instance with the same values.</returns>
        public NetworkConfiguration Clone()
        {
-            return new NetworkConfiguration(Mode, SecurityType, Ssid, Password);
+            return new NetworkConfiguration(
+                Mode,
+                SecurityType,
+                Ssid,
+                Password,
+                StaticIP,
+                SubnetMask,
+                Gateway);
        }
Evidence
PR Compliance ID 2 requires a copy constructor that preserves the new static LAN fields. In the
updated NetworkConfiguration implementation, only the default ctor, WiFi ctor, new 7-arg ctor, and
Clone() are present—no copy constructor is implemented.

Update NetworkConfiguration copy constructor and Clone() to round-trip new static LAN fields
src/Daqifi.Core/Device/Network/NetworkConfiguration.cs[96-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NetworkConfiguration` lacks a copy constructor that preserves the newly added `StaticIP` / `SubnetMask` / `Gateway` fields, which is required for lossless duplication semantics.

## Issue Context
The compliance checklist explicitly requires a copy constructor and that it round-trips the new static LAN fields (including nulls).

## Fix Focus Areas
- src/Daqifi.Core/Device/Network/NetworkConfiguration.cs[96-128]
- src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs[16-182]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Static IP tests miss copy-ctor 📎 Requirement gap ☼ Reliability
Description
NetworkConfigurationTests validate the new static LAN fields and Clone(), but do not test
copy-constructor behavior required by the checklist. This leaves the required duplication path
unverified and prone to regression.
Code

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs[R88-182]

+        [Fact]
+        public void StaticIPConstructor_SetsAllProperties()
+        {
+            // Arrange
+            var staticIP = IPAddress.Parse("10.0.0.5");
+            var subnet = IPAddress.Parse("255.255.255.0");
+            var gateway = IPAddress.Parse("10.0.0.1");
+
+            // Act
+            var config = new NetworkConfiguration(
+                WifiMode.ExistingNetwork,
+                WifiSecurityType.WpaPskPhrase,
+                "Net",
+                "Pass",
+                staticIP,
+                subnet,
+                gateway);
+
+            // Assert
+            Assert.Equal(WifiMode.ExistingNetwork, config.Mode);
+            Assert.Equal(WifiSecurityType.WpaPskPhrase, config.SecurityType);
+            Assert.Equal("Net", config.Ssid);
+            Assert.Equal("Pass", config.Password);
+            Assert.Equal(staticIP, config.StaticIP);
+            Assert.Equal(subnet, config.SubnetMask);
+            Assert.Equal(gateway, config.Gateway);
+        }
+
+        [Fact]
+        public void StaticIPProperties_CanBeAssignedAndCleared()
+        {
+            // Arrange
+            var config = new NetworkConfiguration
+            {
+                StaticIP = IPAddress.Parse("192.168.1.10"),
+                SubnetMask = IPAddress.Parse("255.255.255.0"),
+                Gateway = IPAddress.Parse("192.168.1.1")
+            };
+
+            // Act
+            config.StaticIP = null;
+            config.SubnetMask = null;
+            config.Gateway = null;
+
+            // Assert
+            Assert.Null(config.StaticIP);
+            Assert.Null(config.SubnetMask);
+            Assert.Null(config.Gateway);
+        }
+
+        [Fact]
+        public void Clone_PreservesStaticIPFields()
+        {
+            // Arrange
+            var original = new NetworkConfiguration(
+                WifiMode.ExistingNetwork,
+                WifiSecurityType.WpaPskPhrase,
+                "Net",
+                "Pass",
+                IPAddress.Parse("10.0.0.5"),
+                IPAddress.Parse("255.255.255.0"),
+                IPAddress.Parse("10.0.0.1"));
+
+            // Act
+            var clone = original.Clone();
+
+            // Assert
+            Assert.Equal(original.StaticIP, clone.StaticIP);
+            Assert.Equal(original.SubnetMask, clone.SubnetMask);
+            Assert.Equal(original.Gateway, clone.Gateway);
+
+            // Verify independence — replacing the clone's references should not
+            // affect the original.
+            clone.StaticIP = IPAddress.Parse("172.16.0.5");
+            Assert.NotEqual(original.StaticIP, clone.StaticIP);
+        }
+
+        [Fact]
+        public void Clone_PreservesNullStaticIPFields()
+        {
+            // Arrange
+            var original = new NetworkConfiguration(
+                WifiMode.SelfHosted,
+                WifiSecurityType.None,
+                "Net",
+                string.Empty);
+
+            // Act
+            var clone = original.Clone();
+
+            // Assert
+            Assert.Null(clone.StaticIP);
+            Assert.Null(clone.SubnetMask);
+            Assert.Null(clone.Gateway);
+        }
Evidence
PR Compliance ID 6 requires tests that validate copy constructor and Clone() preserve the new
static LAN fields. The added tests cover default nulls and Clone() behavior, but there are no
assertions exercising a copy constructor path.

Unit tests: NetworkConfigurationTests cover new fields and Clone() behavior
src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs[88-182]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Unit tests do not cover `NetworkConfiguration` copy-constructor behavior for `StaticIP` / `SubnetMask` / `Gateway` (including null preservation).

## Issue Context
The compliance checklist requires tests that validate both copy constructor and `Clone()` preserve values and nulls.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs[88-182]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. LAN setters don’t enforce IPv4 ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
SetLanAddress/SetLanMask/SetLanGateway accept any IPAddress and interpolate it directly
(effectively using ToString()), which can emit IPv6 or scoped address strings even though the SCPI
contract requires quoted IPv4 dotted-quad ("x.x.x.x"). As a result, non-IPv4 inputs can produce
malformed SCPI commands that the firmware is likely to reject or misapply.
Code

src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[R473-515]

+    public static IOutboundMessage<string> SetLanAddress(IPAddress address)
+    {
+        if (address == null)
+        {
+            throw new ArgumentNullException(nameof(address));
+        }
+
+        return new ScpiMessage($"SYSTem:COMMunicate:LAN:ADDRess \"{address}\"");
+    }
+
+    /// <summary>
+    /// Creates a command message to set the subnet mask for the LAN.
+    /// </summary>
+    /// <param name="mask">The subnet mask to assign to the device.</param>
+    /// <remarks>
+    /// Command: SYSTem:COMMunicate:LAN:MASK "x.x.x.x"
+    /// </remarks>
+    public static IOutboundMessage<string> SetLanMask(IPAddress mask)
+    {
+        if (mask == null)
+        {
+            throw new ArgumentNullException(nameof(mask));
+        }
+
+        return new ScpiMessage($"SYSTem:COMMunicate:LAN:MASK \"{mask}\"");
+    }
+
+    /// <summary>
+    /// Creates a command message to set the default gateway for the LAN.
+    /// </summary>
+    /// <param name="gateway">The default gateway to assign to the device.</param>
+    /// <remarks>
+    /// Command: SYSTem:COMMunicate:LAN:GATEway "x.x.x.x"
+    /// </remarks>
+    public static IOutboundMessage<string> SetLanGateway(IPAddress gateway)
+    {
+        if (gateway == null)
+        {
+            throw new ArgumentNullException(nameof(gateway));
+        }
+
+        return new ScpiMessage($"SYSTem:COMMunicate:LAN:GATEway \"{gateway}\"");
+    }
Evidence
PR Compliance ID 3 requires the LAN static-IP commands to use quoted IPv4 "x.x.x.x" formatting, but
the implementation builds the SCPI payload by injecting the provided IPAddress into the command
string without validating it is IPv4; therefore, IPv6 (including link-local addresses with a scope)
would serialize into a non-dotted-quad form that violates the documented contract. This expectation
is reinforced by other networking logic in the codebase (e.g., device discovery) that explicitly
restricts itself to IPv4 addresses, indicating these setters should enforce the same IPv4-only
boundary.

Add SCPI producer methods for LAN static address/mask/gateway commands with exact command formatting
src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[473-515]
src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[463-515]
src/Daqifi.Core/Device/Discovery/WiFiDeviceFinder.cs[399-414]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The LAN static IP SCPI setters (`SetLanAddress`, `SetLanMask`, `SetLanGateway`) accept any `IPAddress` and format it via string interpolation / `ToString()`, which can yield IPv6 or scoped strings even though the required SCPI payload format is quoted IPv4 dotted-quad ("x.x.x.x"). Update the API boundary to enforce (or safely normalize to) IPv4-only inputs so the producer never emits invalid SCPI commands.

## Issue Context
- The firmware command format (PR Compliance ID 3 / checklist) explicitly requires quoted IPv4 dotted-quad values for these LAN commands.
- Today the producer methods document IPv4 formatting but do not enforce it, so callers can pass IPv6 (including link-local + scope) and generate payloads that violate the command contract.
- The broader codebase already assumes IPv4 for networking (e.g., discovery logic filters to IPv4), so these setters should align with that contract.
- Consider whether special values like `IPAddress.Any` / `IPAddress.None` are meaningful for the device; if not, reject them as invalid as well.
- Add unit tests to ensure IPv6 inputs throw for each setter.

## Fix Focus Areas
- src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs[463-515]
- src/Daqifi.Core.Tests/Communication/Producers/ScpiMessageProducerTests.cs[240-329]
- src/Daqifi.Core/Device/Discovery/WiFiDeviceFinder.cs[399-414]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. Static-IP tests don’t assert SAVE ✓ Resolved 📎 Requirement gap ☼ Reliability
Description
The new INetworkConfigurable integration test asserts static LAN commands precede APPLY, but
does not assert they precede SaveNetworkLan as required. This leaves the required ordering
relative to SAVE unverified.
Code

src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs[R155-185]

+        [Fact]
+        public async Task UpdateNetworkConfigurationAsync_WithStaticIP_SendsAddressMaskGatewayBeforeApply()
+        {
+            // Arrange
+            var device = new TestableDaqifiStreamingDevice("TestDevice");
+            device.Connect();
+            var config = new NetworkConfiguration(
+                WifiMode.ExistingNetwork,
+                WifiSecurityType.WpaPskPhrase,
+                "Net",
+                "Pass",
+                IPAddress.Parse("10.0.0.5"),
+                IPAddress.Parse("255.255.255.0"),
+                IPAddress.Parse("10.0.0.1"));
+
+            // Act
+            await device.UpdateNetworkConfigurationAsync(config);
+
+            // Assert
+            var sentCommands = device.SentMessages.Select(m => m.Data).ToList();
+
+            Assert.Contains("SYSTem:COMMunicate:LAN:ADDRess \"10.0.0.5\"", sentCommands);
+            Assert.Contains("SYSTem:COMMunicate:LAN:MASK \"255.255.255.0\"", sentCommands);
+            Assert.Contains("SYSTem:COMMunicate:LAN:GATEway \"10.0.0.1\"", sentCommands);
+
+            // Apply must follow all three (firmware reads runtime config at APPLY time).
+            var applyIndex = sentCommands.IndexOf("SYSTem:COMMunicate:LAN:APPLY");
+            Assert.True(applyIndex > sentCommands.IndexOf("SYSTem:COMMunicate:LAN:ADDRess \"10.0.0.5\""));
+            Assert.True(applyIndex > sentCommands.IndexOf("SYSTem:COMMunicate:LAN:MASK \"255.255.255.0\""));
+            Assert.True(applyIndex > sentCommands.IndexOf("SYSTem:COMMunicate:LAN:GATEway \"10.0.0.1\""));
+        }
Evidence
PR Compliance ID 7 requires tests to assert ordering such that static LAN commands precede
SaveNetworkLan. The added test validates ordering only relative to APPLY and contains no
ordering assertions involving SAVE.

Integration/interface tests: INetworkConfigurable apply sequence skips nulls and includes non-nulls in correct order
src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs[155-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The integration test for static IP ordering checks only that static LAN commands occur before `APPLY`, but does not assert they occur before `SYSTem:COMMunicate:LAN:SAVE`.

## Issue Context
The compliance checklist requires tests that assert ordering such that static LAN commands precede `SaveNetworkLan`.

## Fix Focus Areas
- src/Daqifi.Core.Tests/Device/Network/NetworkConfigurableTests.cs[155-185]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Null clears cached IPs ✓ Resolved 🐞 Bug ≡ Correctness
Description
UpdateNetworkConfigurationAsync treats null StaticIP/SubnetMask/Gateway as “leave unchanged” (skips
sending SCPI), but still writes those nulls into the device’s cached _networkConfiguration. This
makes INetworkConfigurable.NetworkConfiguration no longer represent the device’s current/static
settings after a partial update, even though the device was intentionally left unchanged for those
fields.
Code

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[R271-273]

+            _networkConfiguration.StaticIP = configuration.StaticIP;
+            _networkConfiguration.SubnetMask = configuration.SubnetMask;
+            _networkConfiguration.Gateway = configuration.Gateway;
Evidence
The code explicitly defines null as “leave unchanged” and conditionally skips sending SCPI commands,
but then unconditionally copies those nullable fields into the cached configuration, which can clear
previously known values despite no device change for those fields.

src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[238-273]
src/Daqifi.Core/Device/Network/NetworkConfiguration.cs[34-59]
src/Daqifi.Core/Device/Network/INetworkConfigurable.cs[11-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`NetworkConfiguration` documents `StaticIP/SubnetMask/Gateway == null` as "leave unchanged" on the device, and `UpdateNetworkConfigurationAsync` correctly skips emitting SCPI commands for null fields. However, the method then unconditionally assigns these nullable fields into `_networkConfiguration`, which clears previously known values even though the device was not changed.

## Issue Context
This breaks the meaning of `INetworkConfigurable.NetworkConfiguration` ("current network configuration") after partial updates and can cause callers to lose track of previously-set static networking values.

## Fix Focus Areas
- src/Daqifi.Core/Device/DaqifiStreamingDevice.cs[238-273]
- src/Daqifi.Core/Device/Network/INetworkConfigurable.cs[11-14]
- src/Daqifi.Core/Device/Network/NetworkConfiguration.cs[34-59]

## Implementation notes
- When updating `_networkConfiguration`, only overwrite `StaticIP/SubnetMask/Gateway` if the corresponding input property is non-null.
- Alternatively (more accurate), query back the staged/active values (e.g., via the new `GetLanConfigured*` / `GetLan*` queries) and update the local cache from device responses.
- Add/extend a unit test asserting that calling `UpdateNetworkConfigurationAsync` with null static fields does not clear previously cached static fields.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/Daqifi.Core/Device/Network/NetworkConfiguration.cs
Comment thread src/Daqifi.Core.Tests/Device/Network/NetworkConfigurationTests.cs
Comment thread src/Daqifi.Core/Communication/Producers/ScpiMessageProducer.cs
…IPv4

Address Qodo review on PR #211:
- Bug: UpdateNetworkConfigurationAsync was unconditionally overwriting
  the cached _networkConfiguration.StaticIP/SubnetMask/Gateway with the
  caller's input, which clobbered previously-set values when the caller
  passed null ("leave unchanged"). Only overwrite when non-null.
- SetLanAddress/Mask/Gateway now reject non-IPv4 IPAddress inputs via
  AddressFamily.InterNetwork — firmware's inet_addr is IPv4-only and
  silently mis-sets on IPv6 strings, matching the IPv4-only convention
  already enforced by WiFiDeviceFinder.
- Tests: add IPv6-rejection cases for the three setters, a
  cache-preservation regression test for partial-update semantics, and
  extend the apply-order test to assert SAVE follows the static-IP
  setters as well as APPLY.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tylerkron
Copy link
Copy Markdown
Contributor Author

Response to remaining Qodo summary items

#4 — Static-IP tests don't assert SAVE ordering ✅ Fixed in de52892. Extended UpdateNetworkConfigurationAsync_WithStaticIP_SendsAddressMaskGatewayBeforeApply to also assert that SYSTem:COMMunicate:LAN:SAVE follows each of the three static-IP setters.

#5 — Null clears cached IPs (real bug) ✅ Fixed in de52892. UpdateNetworkConfigurationAsync was unconditionally writing configuration.StaticIP/SubnetMask/Gateway into _networkConfiguration, which clobbered previously-known values when the caller passed null ("leave unchanged"). Now only overwrites when the input is non-null. Added UpdateNetworkConfigurationAsync_WithNullStaticFields_PreservesPreviouslyCachedValues as a regression test.

#1 + #2 — Copy constructor ❌ Pushing back (replied inline). There is no copy constructor on NetworkConfiguration to update; the round-trip path the class exposes is Clone(), which does carry the new fields and is covered by Clone_PreservesStaticIPFields / Clone_PreservesNullStaticIPFields. Adding a NetworkConfiguration(NetworkConfiguration other) ctor would be net-new public API duplicating Clone().

#3 — IPv4 enforcement ✅ Fixed in de52892 (replied inline). All three setters now reject non-IPv4 inputs via RequireIPv4 (AddressFamily.InterNetwork), matching the convention already in WiFiDeviceFinder and the firmware's inet_addr parsing.

Update the Network Configuration feature bullet and add a NetworkConfiguration
example showing StaticIP/SubnetMask/Gateway, including the null = "leave
unchanged" semantic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tylerkron tylerkron merged commit 5b9adc7 into main May 16, 2026
1 check passed
@tylerkron tylerkron deleted the claude/sweet-bhabha-15bb07 branch May 16, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Add LAN static IP/subnet/gateway configuration support

1 participant