mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-05-25 13:43:31 +00:00
fix(wifi): address four review findings in wifi_manager.py
IP parsing (line 476): use partition(':') so bare "ip/mask" lines
(no field-label prefix) are handled without IndexError; falls back to
the full string when no ':' is present before splitting on '/'.
AP-mode override comment (line 503): add one-line explanation above
the wifi_connected/ssid/ip_address clear so maintainers know why the
fields are reset while wlan0 reports as "connected".
Stale force-flag cleanup (__init__): remove a left-over
_FORCE_AP_FLAG_PATH from a prior crash on first instantiation per
process (guarded by class-level _startup_cleanup_done so the nmcli
AP-state check only runs once, not on every per-request instantiation).
Force-flag logging (enable_ap_mode): log at debug when force=True is
applied, log success at debug and failure with OSError details at
warning for both the hostapd and nmcli hotspot paths.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -151,6 +151,18 @@ class WiFiManager:
|
|||||||
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
||||||
f"interface: {self._wifi_interface}, trixie: {self._is_trixie}")
|
f"interface: {self._wifi_interface}, trixie: {self._is_trixie}")
|
||||||
|
|
||||||
|
# Once per process: remove a stale force-AP flag left by a prior crash.
|
||||||
|
# Guard with a class-level flag so the nmcli AP-state check only runs
|
||||||
|
# once even though WiFiManager is instantiated per-request.
|
||||||
|
if not WiFiManager._startup_cleanup_done:
|
||||||
|
WiFiManager._startup_cleanup_done = True
|
||||||
|
if self._FORCE_AP_FLAG_PATH.exists() and not self._is_ap_mode_active():
|
||||||
|
try:
|
||||||
|
self._FORCE_AP_FLAG_PATH.unlink(missing_ok=True)
|
||||||
|
logger.debug("Removed stale force-AP flag on startup (AP not active)")
|
||||||
|
except OSError as exc:
|
||||||
|
logger.warning(f"Could not remove stale force-AP flag: {exc}")
|
||||||
|
|
||||||
def _show_led_message(self, message: str, duration: int = 5):
|
def _show_led_message(self, message: str, duration: int = 5):
|
||||||
"""
|
"""
|
||||||
Show a WiFi status message on the LED display.
|
Show a WiFi status message on the LED display.
|
||||||
@@ -474,7 +486,10 @@ class WiFiManager:
|
|||||||
if result.returncode == 0:
|
if result.returncode == 0:
|
||||||
for line in result.stdout.strip().split('\n'):
|
for line in result.stdout.strip().split('\n'):
|
||||||
if '/' in line:
|
if '/' in line:
|
||||||
ip_address = line.split(':', 1)[1].split('/')[0].strip()
|
# nmcli -t output is "IP4.ADDRESS[1]:x.x.x.x/prefix";
|
||||||
|
# bare "x.x.x.x/prefix" is also accepted defensively.
|
||||||
|
_, sep, rest = line.partition(':')
|
||||||
|
ip_address = (rest if sep else line).split('/')[0].strip()
|
||||||
break
|
break
|
||||||
|
|
||||||
# Final fallback: Get signal strength by matching SSID in WiFi list
|
# Final fallback: Get signal strength by matching SSID in WiFi list
|
||||||
@@ -500,6 +515,8 @@ class WiFiManager:
|
|||||||
|
|
||||||
# Check if AP mode is active
|
# Check if AP mode is active
|
||||||
ap_active = self._is_ap_mode_active()
|
ap_active = self._is_ap_mode_active()
|
||||||
|
# wlan0 shows as "connected" in AP mode; clear client-station fields so
|
||||||
|
# callers don't mistake the AP for an outbound WiFi connection.
|
||||||
if ap_active and wifi_connected:
|
if ap_active and wifi_connected:
|
||||||
wifi_connected = False
|
wifi_connected = False
|
||||||
ssid = None
|
ssid = None
|
||||||
@@ -697,6 +714,8 @@ class WiFiManager:
|
|||||||
_IP_FORWARD_SAVE_PATH = Path("/tmp/ledmatrix_ip_forward_saved") # nosec B108 - process-specific named file; device is single-user RPi
|
_IP_FORWARD_SAVE_PATH = Path("/tmp/ledmatrix_ip_forward_saved") # nosec B108 - process-specific named file; device is single-user RPi
|
||||||
# Written when AP mode is manually force-enabled; prevents daemon auto-disable
|
# Written when AP mode is manually force-enabled; prevents daemon auto-disable
|
||||||
_FORCE_AP_FLAG_PATH = Path("/tmp/ledmatrix_force_ap_active") # nosec B108 - process-specific named file; device is single-user RPi
|
_FORCE_AP_FLAG_PATH = Path("/tmp/ledmatrix_force_ap_active") # nosec B108 - process-specific named file; device is single-user RPi
|
||||||
|
# Ensures the startup stale-flag cleanup runs once per process, not per instantiation
|
||||||
|
_startup_cleanup_done: bool = False
|
||||||
|
|
||||||
def _validate_ap_config(self) -> Tuple[str, int]:
|
def _validate_ap_config(self) -> Tuple[str, int]:
|
||||||
"""Return a sanitized (ssid, channel) pair from config, falling back to defaults."""
|
"""Return a sanitized (ssid, channel) pair from config, falling back to defaults."""
|
||||||
@@ -1896,6 +1915,9 @@ class WiFiManager:
|
|||||||
if not force and self._is_ethernet_connected():
|
if not force and self._is_ethernet_connected():
|
||||||
return False, "Cannot enable AP mode while Ethernet is connected"
|
return False, "Cannot enable AP mode while Ethernet is connected"
|
||||||
|
|
||||||
|
if force:
|
||||||
|
logger.debug(f"enable_ap_mode: force=True — WiFi/Ethernet guards bypassed; will create {self._FORCE_AP_FLAG_PATH}")
|
||||||
|
|
||||||
# Try hostapd/dnsmasq first (captive portal mode)
|
# Try hostapd/dnsmasq first (captive portal mode)
|
||||||
if self.has_hostapd and self.has_dnsmasq:
|
if self.has_hostapd and self.has_dnsmasq:
|
||||||
result = self._enable_ap_mode_hostapd()
|
result = self._enable_ap_mode_hostapd()
|
||||||
@@ -1904,8 +1926,9 @@ class WiFiManager:
|
|||||||
if force:
|
if force:
|
||||||
try:
|
try:
|
||||||
self._FORCE_AP_FLAG_PATH.touch()
|
self._FORCE_AP_FLAG_PATH.touch()
|
||||||
except OSError:
|
logger.debug(f"Force-AP flag created: {self._FORCE_AP_FLAG_PATH}")
|
||||||
pass
|
except OSError as exc:
|
||||||
|
logger.warning(f"Failed to create force-AP flag {self._FORCE_AP_FLAG_PATH}: {exc}")
|
||||||
return result
|
return result
|
||||||
|
|
||||||
# Fallback to nmcli hotspot (simpler, no captive portal)
|
# Fallback to nmcli hotspot (simpler, no captive portal)
|
||||||
@@ -1918,8 +1941,9 @@ class WiFiManager:
|
|||||||
if force:
|
if force:
|
||||||
try:
|
try:
|
||||||
self._FORCE_AP_FLAG_PATH.touch()
|
self._FORCE_AP_FLAG_PATH.touch()
|
||||||
except OSError:
|
logger.debug(f"Force-AP flag created: {self._FORCE_AP_FLAG_PATH}")
|
||||||
pass
|
except OSError as exc:
|
||||||
|
logger.warning(f"Failed to create force-AP flag {self._FORCE_AP_FLAG_PATH}: {exc}")
|
||||||
return result
|
return result
|
||||||
|
|
||||||
return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
|
return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
|
||||||
|
|||||||
Reference in New Issue
Block a user