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
|
self.wifi_manager._disconnected_checks = 0
|
||||||
except subprocess.CalledProcessError as e:
|
except subprocess.CalledProcessError as e:
|
||||||
logger.error(f"NetworkManager restart failed (rc={e.returncode}); "
|
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:
|
except Exception as e:
|
||||||
logger.error(f"NetworkManager restart error: {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:
|
else:
|
||||||
self._consecutive_internet_failures = 0
|
self._consecutive_internet_failures = 0
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -144,6 +144,8 @@ class WiFiManager:
|
|||||||
|
|
||||||
# Timestamp set when AP mode is enabled; used for the idle-timeout check
|
# Timestamp set when AP mode is enabled; used for the idle-timeout check
|
||||||
self._ap_enabled_at: Optional[float] = None
|
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}, "
|
logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, "
|
||||||
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
||||||
@@ -691,9 +693,8 @@ class WiFiManager:
|
|||||||
|
|
||||||
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."""
|
||||||
import re as _re
|
|
||||||
ssid = str(self.config.get("ap_ssid", DEFAULT_AP_SSID))
|
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")
|
logger.warning(f"AP SSID '{ssid}' is invalid, falling back to default")
|
||||||
ssid = DEFAULT_AP_SSID
|
ssid = DEFAULT_AP_SSID
|
||||||
try:
|
try:
|
||||||
@@ -705,10 +706,6 @@ class WiFiManager:
|
|||||||
channel = DEFAULT_AP_CHANNEL
|
channel = DEFAULT_AP_CHANNEL
|
||||||
return ssid, 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:
|
def _setup_iptables_redirect(self) -> bool:
|
||||||
"""
|
"""
|
||||||
Add port 80 → 5000 redirect rules for the captive portal.
|
Add port 80 → 5000 redirect rules for the captive portal.
|
||||||
@@ -936,14 +933,14 @@ class WiFiManager:
|
|||||||
if r.returncode == 0:
|
if r.returncode == 0:
|
||||||
logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
|
logger.debug("Internet connectivity confirmed via ping 8.8.8.8")
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except (subprocess.SubprocessError, OSError):
|
||||||
pass
|
pass
|
||||||
try:
|
try:
|
||||||
import urllib.request as _ureq
|
import urllib.request as _ureq
|
||||||
_ureq.urlopen("http://connectivity-check.ubuntu.com/", timeout=timeout)
|
_ureq.urlopen("http://connectivity-check.ubuntu.com/", timeout=timeout)
|
||||||
logger.debug("Internet connectivity confirmed via HTTP check")
|
logger.debug("Internet connectivity confirmed via HTTP check")
|
||||||
return True
|
return True
|
||||||
except Exception:
|
except OSError:
|
||||||
pass
|
pass
|
||||||
logger.debug("Internet connectivity check failed (both ping and HTTP)")
|
logger.debug("Internet connectivity check failed (both ping and HTTP)")
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -1720,7 +1720,8 @@ def execute_system_action():
|
|||||||
capture_output=True, text=True, timeout=15)
|
capture_output=True, text=True, timeout=15)
|
||||||
return jsonify({
|
return jsonify({
|
||||||
'status': 'success' if result.returncode == 0 else 'error',
|
'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,
|
'returncode': result.returncode,
|
||||||
'stdout': result.stdout,
|
'stdout': result.stdout,
|
||||||
'stderr': result.stderr
|
'stderr': result.stderr
|
||||||
|
|||||||
Reference in New Issue
Block a user