mirror of
https://github.com/MarekWo/mc-webui.git
synced 2026-07-03 16:31:28 +02:00
fix(channels): read channels from DB instead of iterating device slots
The TimeoutError-based fallback added in 1d47c9c only fires when
mc.commands.get_channel() actually raises — but on a sluggish device the
call returns an empty/falsy event without raising, so the loop walks
all dm._max_channels slots (40 on the firmware in production), each
empty result returns None, and the API yields just Public (or whatever
slot 0 happened to succeed on). The DB fallback never triggered and the
user kept seeing just Public after refresh.
The channels table in the DB is already the authoritative cache:
- _load_channel_secrets() syncs it on every device connect and prunes
stale rows,
- set_channel()/remove_channel() update it synchronously with the
device,
- _refresh_channel_secret() refreshes individual rows on per-send
refresh.
Drop the device-slot iteration in cli.get_channels() and read from the
DB. /api/channels response time becomes a single SELECT (<1 ms) and is
unaffected by device responsiveness — exactly what we wanted from the
fallback in the first place.
Also revert the TimeoutError re-raise in get_channel_info(): the
console `channels` and `add_channel` commands iterate slots and would
crash on the first slow one. Logging + None on failure is the right
behavior for slot iteration. The 3 s default timeout stays since it
still keeps individual slot probes cheap.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -14,7 +14,6 @@ import logging
|
||||
import struct
|
||||
import threading
|
||||
import time
|
||||
from concurrent.futures import TimeoutError as FuturesTimeoutError
|
||||
from typing import Optional, Any, Dict, List, Tuple
|
||||
from urllib.parse import urlparse, parse_qs
|
||||
|
||||
@@ -2196,8 +2195,10 @@ class DeviceManager:
|
||||
def get_channel_info(self, idx: int, timeout: float = 3) -> Optional[Dict]:
|
||||
"""Get info for a specific channel.
|
||||
|
||||
Raises TimeoutError when the device fails to respond within `timeout`
|
||||
seconds so callers can distinguish "device sluggish" from "empty slot".
|
||||
Short default timeout (3 s) means iterating empty slots stays cheap
|
||||
even when the device is sluggish. Returns None on any failure —
|
||||
callers iterating over slots can't distinguish "empty" from "stalled"
|
||||
and need the loop to keep moving regardless.
|
||||
"""
|
||||
if not self.is_connected:
|
||||
return None
|
||||
@@ -2219,9 +2220,6 @@ class DeviceManager:
|
||||
'secret': secret,
|
||||
'channel_idx': data.get('channel_idx', idx),
|
||||
}
|
||||
except FuturesTimeoutError:
|
||||
# Re-raise so caller can break the loop instead of hammering a stuck device
|
||||
raise
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to get channel {idx}: {e}")
|
||||
return None
|
||||
|
||||
+22
-57
@@ -5,7 +5,6 @@ Function signatures preserved for backward compatibility with api.py.
|
||||
"""
|
||||
|
||||
import logging
|
||||
from concurrent.futures import TimeoutError as FuturesTimeoutError
|
||||
from typing import Tuple, Optional, List, Dict
|
||||
from app.config import config
|
||||
|
||||
@@ -319,66 +318,32 @@ def check_connection() -> bool:
|
||||
# =============================================================================
|
||||
|
||||
def get_channels() -> Tuple[bool, List[Dict]]:
|
||||
"""Get list of configured channels.
|
||||
"""Get list of configured channels from the local cache.
|
||||
|
||||
When the USB device is briefly unresponsive a single get_channel_info()
|
||||
times out (3 s) and the rest of the slots would too — so we stop hitting
|
||||
the device and merge whatever we got with the locally cached channels in
|
||||
the DB. This guarantees the UI shows all channels instead of just Public.
|
||||
Reads from the channels table in the DB, which is kept in sync with the
|
||||
device by:
|
||||
- _load_channel_secrets() on every connect (also prunes stale rows),
|
||||
- set_channel() / remove_channel() updating the DB synchronously,
|
||||
- _refresh_channel_secret() refreshing single rows on per-send refresh.
|
||||
|
||||
Iterating device slots here used to cost 40 × USB round-trips on every
|
||||
/api/channels hit, and returned only "Public" the moment the firmware
|
||||
stalled mid-iteration (other slots came back empty without raising any
|
||||
timeout exception). The DB is the authoritative cache — use it.
|
||||
"""
|
||||
try:
|
||||
dm = _get_dm()
|
||||
channels = []
|
||||
seen_idx = set()
|
||||
device_partial = False
|
||||
|
||||
for idx in range(dm._max_channels):
|
||||
try:
|
||||
info = dm.get_channel_info(idx)
|
||||
except FuturesTimeoutError:
|
||||
logger.warning(
|
||||
f"get_channels: device timeout at slot {idx} — "
|
||||
f"falling back to DB for remaining slots"
|
||||
)
|
||||
device_partial = True
|
||||
break
|
||||
|
||||
if info and info.get('name'):
|
||||
channels.append({
|
||||
'index': idx,
|
||||
'name': info.get('name', ''),
|
||||
'key': info.get('secret', info.get('key', '')),
|
||||
})
|
||||
seen_idx.add(idx)
|
||||
# Keep the DB in sync with what the device just told us
|
||||
try:
|
||||
secret_hex = info.get('secret', '') or None
|
||||
dm.db.upsert_channel(idx, info.get('name', ''), secret_hex)
|
||||
except Exception as e:
|
||||
logger.debug(f"upsert_channel({idx}) failed: {e}")
|
||||
|
||||
if device_partial:
|
||||
try:
|
||||
for row in dm.db.get_channels():
|
||||
db_idx = row.get('idx')
|
||||
if db_idx is None or db_idx in seen_idx:
|
||||
continue
|
||||
name = row.get('name') or ''
|
||||
if not name:
|
||||
continue
|
||||
channels.append({
|
||||
'index': db_idx,
|
||||
'name': name,
|
||||
'key': row.get('secret', '') or '',
|
||||
})
|
||||
channels.sort(key=lambda c: c['index'])
|
||||
logger.info(
|
||||
f"get_channels: returned {len(channels)} channels "
|
||||
f"({len(seen_idx)} from device + DB fallback)"
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error(f"get_channels DB fallback failed: {e}")
|
||||
|
||||
rows = dm.db.get_channels()
|
||||
channels = [
|
||||
{
|
||||
'index': row['idx'],
|
||||
'name': row['name'] or '',
|
||||
'key': row['secret'] or '',
|
||||
}
|
||||
for row in rows
|
||||
if row.get('name')
|
||||
]
|
||||
channels.sort(key=lambda c: c['index'])
|
||||
return True, channels
|
||||
except Exception as e:
|
||||
logger.error(f"get_channels error: {e}")
|
||||
|
||||
Reference in New Issue
Block a user