mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-05-14 09:33:32 +00:00
fix: address five review findings (NM retry loop, start_display message, code quality)
- wifi_monitor_daemon: reset _consecutive_internet_failures = 0 in both NM-restart exception handlers; previously both left the counter at threshold, causing an immediate retry on the next iteration instead of waiting another full backoff period - api_v3: fix start_display failure message — when mode is set and systemctl returns non-zero, message now includes the failure reason and a hint rather than always reporting success phrasing - wifi_manager: move _redirect_backend from class variable to instance variable in __init__ alongside _ap_enabled_at; class-level default shadowed correctly in practice (single instance) but was misleading - wifi_manager: narrow broad except Exception in _check_internet_connectivity to (subprocess.SubprocessError, OSError) for ping and OSError for HTTP (urllib.error.URLError is an OSError subclass in Python 3) - wifi_manager: remove redundant local 'import re as _re' in _validate_ap_config; re is already imported at module level (line 37) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -153,10 +153,12 @@ class WiFiMonitorDaemon:
|
||||
self.wifi_manager._disconnected_checks = 0
|
||||
except subprocess.CalledProcessError as e:
|
||||
logger.error(f"NetworkManager restart failed (rc={e.returncode}); "
|
||||
"keeping failure counter unchanged")
|
||||
"resetting failure counter to avoid tight retry loop")
|
||||
self._consecutive_internet_failures = 0
|
||||
except Exception as e:
|
||||
logger.error(f"NetworkManager restart error: {e}; "
|
||||
"keeping failure counter unchanged")
|
||||
"resetting failure counter to avoid tight retry loop")
|
||||
self._consecutive_internet_failures = 0
|
||||
else:
|
||||
self._consecutive_internet_failures = 0
|
||||
else:
|
||||
|
||||
@@ -144,6 +144,8 @@ class WiFiManager:
|
||||
|
||||
# Timestamp set when AP mode is enabled; used for the idle-timeout check
|
||||
self._ap_enabled_at: Optional[float] = None
|
||||
# Which redirect backend was used (iptables/nftables/None); set per-instance
|
||||
self._redirect_backend: Optional[str] = None
|
||||
|
||||
logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, "
|
||||
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
||||
@@ -691,9 +693,8 @@ class WiFiManager:
|
||||
|
||||
def _validate_ap_config(self) -> Tuple[str, int]:
|
||||
"""Return a sanitized (ssid, channel) pair from config, falling back to defaults."""
|
||||
import re as _re
|
||||
ssid = str(self.config.get("ap_ssid", DEFAULT_AP_SSID))
|
||||
if not ssid or len(ssid) > 32 or not _re.match(r'^[\x20-\x7E]+$', ssid):
|
||||
if not ssid or len(ssid) > 32 or not re.match(r'^[\x20-\x7E]+$', ssid):
|
||||
logger.warning(f"AP SSID '{ssid}' is invalid, falling back to default")
|
||||
ssid = DEFAULT_AP_SSID
|
||||
try:
|
||||
@@ -705,10 +706,6 @@ class WiFiManager:
|
||||
channel = DEFAULT_AP_CHANNEL
|
||||
return ssid, channel
|
||||
|
||||
# Tracks which redirect backend was used so teardown uses the same one.
|
||||
# Value is "iptables", "nftables", or None (not set up).
|
||||
_redirect_backend: Optional[str] = None
|
||||
|
||||
def _setup_iptables_redirect(self) -> bool:
|
||||
"""
|
||||
Add port 80 → 5000 redirect rules for the captive portal.
|
||||
@@ -936,14 +933,14 @@ class WiFiManager:
|
||||
if r.returncode == 0:
|
||||
logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
|
||||
return True
|
||||
except Exception:
|
||||
except (subprocess.SubprocessError, OSError):
|
||||
pass
|
||||
try:
|
||||
import urllib.request as _ureq
|
||||
_ureq.urlopen("http://connectivity-check.ubuntu.com/", timeout=timeout)
|
||||
logger.debug("Internet connectivity confirmed via HTTP check")
|
||||
return True
|
||||
except Exception:
|
||||
except OSError:
|
||||
pass
|
||||
logger.debug("Internet connectivity check failed (both ping and HTTP)")
|
||||
return False
|
||||
|
||||
@@ -1720,7 +1720,8 @@ def execute_system_action():
|
||||
capture_output=True, text=True, timeout=15)
|
||||
return jsonify({
|
||||
'status': 'success' if result.returncode == 0 else 'error',
|
||||
'message': f'Started display in {mode} mode',
|
||||
'message': f'Started display in {mode} mode' if result.returncode == 0
|
||||
else f'Failed to start display in {mode} mode: {result.stderr.strip() or "check sudo systemctl status ledmatrix.service"}',
|
||||
'returncode': result.returncode,
|
||||
'stdout': result.stdout,
|
||||
'stderr': result.stderr
|
||||
|
||||
Reference in New Issue
Block a user