Avoid Wi-Fi restarts for disabled profiles Fix #3

docs/agent/architecture.md

@@ -159,7 +159,7 @@ Remote reboot, SSH stop/disconnect, and host-key rotate/reset use deferred contr
 
 `wifi_config` owns a fixed-width versioned NVS schema with four prioritized station profiles and AP policy `off`, `fallback`, or `always`. Missing configuration generates per-device defaults including a random AP password. Invalid stored data is generally left untouched while RAM defaults are used.
 
-`wifi_manager` is a permanent task with one bounded command/event queue. ESP-IDF callbacks only copy compact events into the queue. The task owns association, DHCP deadlines, profile failover, AP policy, retries/backoff, and next-profile requests. It also reconciles against authoritative driver/netif state so dropped events do not permanently wedge policy. ESP-IDF Wi-Fi storage is RAM-only; the application blob is authoritative, and edits require explicit save. Start/stop—including local controls—intentionally update the RAM `enabled_at_boot` field. Working-configuration copies contain PSKs and must be securely wiped; routine status and the local UI use secret-free snapshots.
+`wifi_manager` is a permanent task with one bounded command/event queue. ESP-IDF callbacks only copy compact events into the queue. The task owns association, DHCP deadlines, profile failover, AP policy, retries/backoff, and next-profile requests. It also reconciles against authoritative driver/netif state so dropped events do not permanently wedge policy. ESP-IDF Wi-Fi storage is RAM-only; the application blob is authoritative, and edits require explicit save. Edits to disabled station profiles are staged in RAM without restarting the radio; enabling/disabling a profile or changing enabled station/AP policy restarts it asynchronously. Start/stop—including local controls—intentionally update the RAM `enabled_at_boot` field. Working-configuration copies contain PSKs and must be securely wiped; routine status and the local UI use secret-free snapshots.
 
 Persistent namespaces/blobs include:
 

docs/agent/current-state.md

@@ -46,13 +46,13 @@ These observations should be checked when touching the relevant area; they are n
 
 ## Active Task
 
-- **Objective:** Keep UART1 configuration independent of USB CDC host line coding and move the Phase 0 RS-232 signal status command under `debug`.
-- **Relevant files:** `src/usb_cdc_transport.{c,h}`, `src/usb_console.c`, `src/rs232_hw_test.c`, `src/console_completion.c`, related documentation.
-- **Findings:** The USB transport converted cached CDC line coding into `serial_config_t` and called `serial_service_apply_config()` after writer acquisition. Separately, root `status` was a Phase 0 diagnostic that needed to claim the RS-232 port and therefore failed while the production serial service owned it.
-- **Decision:** Treat CDC line coding as diagnostic metadata only. UART1 configuration remains exclusively controlled by explicit `serial` commands and their NVS persistence. Keep all Phase 0 RS-232 diagnostics, including signal status, under `debug`.
-- **Changes completed:** Removed the pending line-coding apply path and its counters; retained the latest host setting for `usb status`. Replaced root `status` with `debug status`, updated completion and documentation.
-- **Remaining work:** Target-hardware verification: acquire/release USB writer ownership after changing a host terminal's line coding and confirm UART1 remains at the configured framing. Verify `debug status` works after `serial stop` and root `status` is unknown.
-- **Risks / things to remember:** Host terminal line-coding selectors no longer configure the physical RS-232 port; use `serial set`/`serial save` instead. `debug status` remains ownership-protected and requires UART1 to be stopped. `pio run` passed after both changes.
+- **Objective:** Keep UART1 configuration independent of USB CDC host line coding, keep Phase 0 RS-232 status under `debug`, and avoid disconnecting Wi-Fi for edits to disabled profiles.
+- **Relevant files:** `src/usb_cdc_transport.{c,h}`, `src/usb_console.c`, `src/rs232_hw_test.c`, `src/console_completion.c`, `src/wifi_manager.{c,h}`, related documentation.
+- **Findings:** The USB transport converted cached CDC line coding into `serial_config_t` and called `serial_service_apply_config()` after writer acquisition. Root `status` was a Phase 0 diagnostic that needed to claim the RS-232 port and therefore failed while the production serial service owned it. Every Wi-Fi working-configuration update queued a radio restart, including edits to profiles disabled in both the old and new configurations.
+- **Decision:** Treat CDC line coding as diagnostic metadata only. UART1 configuration remains exclusively controlled by explicit `serial` commands and their NVS persistence. Keep all Phase 0 RS-232 diagnostics, including signal status, under `debug`. Treat disabled Wi-Fi profiles as staged configuration: their edits do not restart the radio; enable/disable transitions and changes to enabled station/AP policy retain controlled asynchronous restart behavior.
+- **Changes completed:** Removed the pending line-coding apply path and its counters; retained the latest host setting for `usb status`. Replaced root `status` with `debug status`, updated completion and documentation. Added effective Wi-Fi policy comparison before queueing `MESSAGE_COMMAND_APPLY`.
+- **Remaining work:** Target-hardware verification: acquire/release USB writer ownership after changing a host terminal's line coding and confirm UART1 remains at the configured framing. Verify `debug status` works after `serial stop` and root `status` is unknown. While connected through Wi-Fi, edit a disabled profile and its secret without a reconnect; then enable it and verify the expected reconnect.
+- **Risks / things to remember:** Host terminal line-coding selectors no longer configure the physical RS-232 port; use `serial set`/`serial save` instead. `debug status` remains ownership-protected and requires UART1 to be stopped. Enabling a profile may disconnect an SSH administrative session. `pio run` passed after all changes.
 
 ### Handoff template
 

docs/command_reference.md

@@ -105,7 +105,7 @@ Opening `/dev/ttyACM*` with DTR asserted creates the `usb-cdc` broker client, st
 | `wifi nslookup <host>` | Resolve and display unique IPv4/IPv6 addresses. |
 | `wifi traceroute <host> [max-hops]` | Run IPv4 ICMP traceroute with up to 30 hops. |
 
-`ping`, `nslookup`, and `traceroute` are root aliases. The four station-profile slots use lower priority values first. Passwords are not displayed by ordinary status output.
+`ping`, `nslookup`, and `traceroute` are root aliases. The four station-profile slots use lower priority values first. Edits to a disabled profile's SSID, priority, security mode, or secret are staged in RAM and do not interrupt the current Wi-Fi connection. Enabling or disabling a profile, changing an enabled profile, or changing AP policy/configuration applies the new radio policy and may reconnect Wi-Fi. Use `wifi save` to persist working changes. Passwords are not displayed by ordinary status output.
 
 ## HTTPS web terminal
 

src/wifi_manager.c

@@ -196,6 +196,48 @@ static void copy_working_config(wifi_app_config_t *config)
     unlock_shared();
 }
 
+static bool profiles_equal(const wifi_config_sta_profile_t *left,
+                           const wifi_config_sta_profile_t *right)
+{
+    return left->enabled == right->enabled &&
+           left->priority == right->priority &&
+           left->security == right->security &&
+           left->ssid_len == right->ssid_len &&
+           left->psk_len == right->psk_len &&
+           memcmp(left->ssid, right->ssid, left->ssid_len) == 0 &&
+           memcmp(left->psk, right->psk, left->psk_len) == 0;
+}
+
+/*
+ * Disabled profiles are staged configuration, not current radio policy. Their
+ * credentials and priority may be prepared without interrupting a live link.
+ */
+static bool config_requires_radio_restart(const wifi_app_config_t *current,
+                                          const wifi_app_config_t *candidate)
+{
+    if (current->ap_policy != candidate->ap_policy ||
+        current->ap_ssid_len != candidate->ap_ssid_len ||
+        current->ap_psk_len != candidate->ap_psk_len ||
+        current->ap_channel != candidate->ap_channel ||
+        memcmp(current->ap_ssid, candidate->ap_ssid, current->ap_ssid_len) != 0 ||
+        memcmp(current->ap_psk, candidate->ap_psk, current->ap_psk_len) != 0) {
+        return true;
+    }
+
+    for (size_t slot = 0U; slot < WIFI_CONFIG_STA_PROFILE_COUNT; ++slot) {
+        const wifi_config_sta_profile_t *old_profile = &current->profiles[slot];
+        const wifi_config_sta_profile_t *new_profile = &candidate->profiles[slot];
+        bool old_enabled = old_profile->enabled != 0U;
+        bool new_enabled = new_profile->enabled != 0U;
+
+        if (old_enabled != new_enabled ||
+            (old_enabled && !profiles_equal(old_profile, new_profile))) {
+            return true;
+        }
+    }
+    return false;
+}
+
 static bool connected_event_matches_active_profile(const manager_message_t *message)
 {
     bool matches = false;
@@ -1357,11 +1399,14 @@ esp_err_t wifi_manager_apply_working_config(const wifi_app_config_t *config)
         return ESP_ERR_INVALID_STATE;
     }
 
-    manager_message_t message = {.type = MESSAGE_COMMAND_APPLY};
     lock_shared();
-    if (!enqueue_message(&message)) {
-        unlock_shared();
-        return ESP_ERR_TIMEOUT;
+    bool restart_radio = config_requires_radio_restart(&s_shared.config, config);
+    if (restart_radio) {
+        manager_message_t message = {.type = MESSAGE_COMMAND_APPLY};
+        if (!enqueue_message(&message)) {
+            unlock_shared();
+            return ESP_ERR_TIMEOUT;
+        }
     }
     s_shared.config = *config;
     ++s_shared.snapshot.config_generation;

src/wifi_manager.h

@@ -83,8 +83,9 @@ esp_err_t wifi_manager_init(const wifi_app_config_t *config);
 esp_err_t wifi_manager_get_working_config(wifi_app_config_t *config);
 
 /*
- * Replaces the RAM working configuration. Application is asynchronous; when
- * Wi-Fi is running, the manager task restarts it using the newest generation.
+ * Replaces the RAM working configuration. Disabled-profile-only edits do not
+ * interrupt a running radio; changes to effective station/AP policy are
+ * applied asynchronously by restarting with the newest generation.
  */
 esp_err_t wifi_manager_apply_working_config(const wifi_app_config_t *config);