diff --git a/meshterm/core/remote_config.py b/meshterm/core/remote_config.py index efd4308..117c306 100644 --- a/meshterm/core/remote_config.py +++ b/meshterm/core/remote_config.py @@ -28,6 +28,13 @@ three of its shapes only make sense against that source: ``af`` both read and write ``airtime_factor``, so writing one stales the other (:attr:`RemoteSetting.overlaps`). +The catalog is not the whole page, though: it is what *any* MeshCore build might expose, +and a node can have settings beyond it (a third-party firmware's own knobs). Those are never +listed here and never probed — they are learned one node at a time from commands the reader +actually ran (:func:`parse_setting_command`, :func:`discovered_setting`), which is why +:attr:`RemoteSetting.discovered` exists and why :func:`learnable_from_get` has to care that +``get`` matches keys by prefix and ``set`` does not. + Replies are parsed *loosely* on purpose: repeater firmware answers tersely and has changed its phrasing across versions (``"> 20"``, ``"tx: 20"``, ``"OK - repeat is now ON"``), so values are extracted rather than pattern-matched. Firmware without a given key answers @@ -118,6 +125,9 @@ class RemoteSetting: part: This setting's position within its composite's comma-joined value. overlaps: Keys that are another spelling of the same firmware value, whose cached reading a write to this one makes stale. + discovered: Whether this setting is not in the catalog at all, but was learned from + a command the reader ran on *one node's* command line (see + :func:`discovered_setting`). Never true of a catalog entry. """ key: str @@ -139,6 +149,7 @@ class RemoteSetting: composite: str = "" part: int = 0 overlaps: tuple[str, ...] = () + discovered: bool = False @property def get_command(self) -> str: @@ -671,6 +682,134 @@ def composite_members(composite: str) -> list[RemoteSetting]: return sorted((s for s in REPEATER_SETTINGS if s.composite == composite), key=lambda s: s.part) +#: The category a discovered setting sorts under, and the heading its section takes. +DISCOVERED_CATEGORY = "Extra" + +#: The description lane's text for a discovered setting. The catalog has no explanation to +#: give — all it knows is that this node answered the key — so the row says exactly that, and +#: says it short: the lane is shared with descriptions written to fit a 72-column screen. +DISCOVERED_HELP = "Found on this node" + +#: Keys whose value is never written to disk, however it was asked for. ``prv.key`` is the +#: node's own identity: the firmware answers it on the serial console only (``sender_timestamp +#: == 0`` in ``handleGetCmd``), it is a fact rather than a setting, and a reply that reached +#: us anyway must not be the first place a repeater's private key lands in a file. The +#: command line still runs the command and still prints the answer; only the remembering +#: stops here. +NEVER_STORED = frozenset({"prv.key"}) + + +@dataclass(frozen=True, slots=True) +class SettingCommand: + """A remote-CLI command recognised as a read or a write of one named setting. + + Attributes: + verb: ``"get"`` or ``"set"``. + key: The setting's key, as the reader spelled it. + value: What a ``set`` writes; ``""`` for a ``get``. + """ + + verb: str + key: str + value: str = "" + + +def parse_setting_command(command: str) -> SettingCommand | None: + """Read one typed CLI command as a setting read/write, or ``None`` if it is neither. + + The grammar is the filter, and deliberately so: ``get `` and ``set `` + are the firmware's own way of saying *this is a setting*, so recognising them costs no + list of what to exclude. Every top-level verb — ``reboot``, ``advert``, ``clock sync``, + ``start ota``, ``powersaving``, ``setperm`` — simply isn't this shape and falls out. + A key is held to the spelling the firmware's own keys use, so a mistyped line can't + invent one. + """ + parts = command.strip().split() + if len(parts) < 2 or parts[0] not in ("get", "set"): + return None + key = parts[1] + if not re.fullmatch(r"[A-Za-z][A-Za-z0-9._]*", key): + return None + if parts[0] == "get": + return SettingCommand("get", key) if len(parts) == 2 else None + value = command.strip()[len("set") :].strip()[len(key) :].strip() + return SettingCommand("set", key, value) if value else None + + +def get_match_keys() -> list[str]: + """Every spelling a firmware ``get`` matches on, longest first. + + A composite's members share their composite's key (``get radio``); verb-shaped settings + aren't ``get`` keys at all. + """ + keys = {s.composite or s.key for s in REPEATER_SETTINGS if s.readable and not s.verb} + return sorted(keys, key=len, reverse=True) + + +def shadowing_key(key: str) -> str | None: + """The catalog ``get`` key that would answer a ``get key`` on firmware lacking ``key``. + + ``handleGetCmd`` compares with ``memcmp(config, "radio", 5)`` — no trailing space — so a + key it doesn't know falls into the branch of any *shorter* key that prefixes it, and + answers with that one's value. ``get radio.rxps`` on stock firmware comes back + ``> 910.525,62.5,7,5``: not an error, and not a value of the key that was asked for. + ``handleSetCmd`` matches ``"radio "`` *with* the space, so a write has no such shadow. + """ + return next((k for k in get_match_keys() if key != k and key.startswith(k)), None) + + +def learnable_from_get(key: str, reply: str, known: Mapping[str, str]) -> bool: + """Whether a ``get key`` reply really came from ``key`` and not from a shorter sibling. + + Only asked of a key the catalog doesn't have, where there is no typing to reject a + wrong-shaped answer with. The reply is trusted when nothing shadows the key + (:func:`shadowing_key`), or when it cannot be the shadow's answer — it doesn't parse as + the shadow's type, or it parses to something other than what the shadow last said. A + shadow whose own value was never read leaves the question unanswerable, and an + unanswerable question is not a discovery. + + Args: + key: The key the reader asked for. + reply: What the node answered. + known: The node's last-read values, keyed as the cache keys them. + """ + shadow = shadowing_key(key) + if shadow is None: + return True + members = composite_members(shadow) + if not members: + spec = get_setting(shadow) + members = [spec] if spec is not None else [] + if not members: # pragma: no cover - every get key is a catalog key + return True + parsed = {m.key: parse_reply_value(m, reply) for m in members} + if any(value is None for value in parsed.values()): + return True # the reply isn't even shaped like the shadow's answer + if any(known.get(k) is None for k in parsed): + return False # it could be the shadow's answer, and there is nothing to compare + return any(known[k] != value for k, value in parsed.items()) + + +def discovered_setting(key: str) -> RemoteSetting: + """A catalog entry for a key the catalog hasn't got, which one node said it has. + + Everything downstream of the catalog — the read plan, the reply parser, the editor's + lanes, staging, :func:`write_plan` — speaks :class:`RemoteSetting`, so a discovered key + becomes one rather than growing a second path beside it. What it carries is only what + was actually learned: the key as the node spells it, and a string value. No range, no + words, no options — the firmware never said, and guessing is how a row would start + lying about a node whose build we have never seen. + """ + return RemoteSetting( + key=key, + label=key, + help=DISCOVERED_HELP, + category=DISCOVERED_CATEGORY, + kind="str", + discovered=True, + ) + + def reply_is_error(reply: str) -> bool: """Whether a reply text reads as the firmware refusing the command. @@ -884,6 +1023,11 @@ def write_plan(pending: Mapping[str, str], known: Mapping[str, str]) -> list[Wri else: joined = ",".join(str(values[m.key]) for m in members) writes.append(Write(f"set {spec.composite} {joined}", dict(values))) # type: ignore[arg-type] + # A staged key the catalog hasn't got is a discovered one (nothing else can stage a + # row), and it is written the only way anything knows how: its own plain `set`. + for key, value in pending.items(): + if get_setting(key) is None: + writes.append(Write(f"set {key} {value}", {key: value})) return writes diff --git a/meshterm/core/remote_store.py b/meshterm/core/remote_store.py index 65a1765..fb56042 100644 --- a/meshterm/core/remote_store.py +++ b/meshterm/core/remote_store.py @@ -35,11 +35,16 @@ class CachedValue: supported: ``False`` when the node answered the read with an error — its firmware has no such setting (a board without a front-end module, a build without a bridge). Kept apart from *never read*, which is no entry at all. + discovered: ``True`` for a setting the catalog hasn't got, learned from a command + the reader ran on *this node's* command line. It is the row's whole claim to + exist, so a node that stops answering the key loses the row rather than + earning an ``n/a`` — the catalog is what says a key *might* be there. """ value: str read_at: datetime | None supported: bool = True + discovered: bool = False class RemoteStore: @@ -84,7 +89,10 @@ class RemoteStore: except ValueError: read_at = None out[key] = CachedValue( - value=str(entry.get("value", "")), read_at=read_at, supported=supported + value=str(entry.get("value", "")), + read_at=read_at, + supported=supported, + discovered=bool(entry.get("discovered", False)), ) return out @@ -92,6 +100,10 @@ class RemoteStore: """Cache one setting's value for ``node``, stamped now.""" self._put(node, key, {"value": value, "read_at": utcnow().isoformat()}) + def remember_discovered(self, node: Contact, key: str, value: str) -> None: + """Cache a setting the catalog hasn't got, which ``node`` has just proved it has.""" + self._put(node, key, {"value": value, "read_at": utcnow().isoformat(), "discovered": True}) + def remember_unsupported(self, node: Contact, key: str) -> None: """Record that ``node`` answered a read of ``key`` with an error, stamped now.""" self._put(node, key, {"unsupported": True, "read_at": utcnow().isoformat()}) @@ -104,10 +116,18 @@ class RemoteStore: self._write(records) def _put(self, node: Contact, key: str, entry: dict) -> None: - """Store one setting's cache entry for ``node``.""" + """Store one setting's cache entry for ``node``, keeping it discovered if it was. + + A discovered row is refreshed by the same reads and writes as any other — through + :meth:`remember_setting` — and that must not quietly demote it to a catalog row it + has no entry for, which would leave a row nothing draws. + """ records = self._load_all() record = records.setdefault(admin_key(node), {}) settings = record.setdefault("settings", {}) + previous = settings.get(key) + if isinstance(previous, dict) and previous.get("discovered"): + entry = {**entry, "discovered": True} settings[key] = entry self._write(records) diff --git a/meshterm/ui/remote_cli.py b/meshterm/ui/remote_cli.py index b1304f6..94b27a0 100644 --- a/meshterm/ui/remote_cli.py +++ b/meshterm/ui/remote_cli.py @@ -13,8 +13,9 @@ The input line behaves like a shell: :class:`~meshterm.core.remote_store.RemoteStore`) so last week's incantation is one keystroke away next session. * **Tab** completes the current text against the known repeater commands — the fixed - verbs plus every ``get``/``set`` spelling in the settings catalog — and the best - match previews muted, ghost-text style, after the cursor. + verbs, every ``get``/``set`` spelling in the settings catalog, and any key *this node* + taught us here on an earlier visit — and the best match previews muted, ghost-text + style, after the cursor. * **Enter** sends. The transcript keeps every exchange of the session, newest at the bottom, and the view follows the prompt as it grows. @@ -25,7 +26,7 @@ the radio, the history store, and the reply plumbing) and feeds replies back thr from __future__ import annotations -from collections.abc import Callable +from collections.abc import Callable, Iterable from rich.console import Group from rich.text import Text @@ -70,6 +71,7 @@ class RemoteCliScreen(Screen): history: list[str], send: Callable[[str], None], session: object, + extra_keys: Iterable[str] = (), ) -> None: """Create the command line over a node's persisted history. @@ -78,6 +80,9 @@ class RemoteCliScreen(Screen): history: The node's prior commands, oldest first (recalled with ↑). send: Commits one command (owner-guarded; no-op while one is flying). session: The running TUI session (for repaints). + extra_keys: Settings *this node* has that the catalog hasn't, found here on an + earlier visit. They complete like any other key: the prompt is where they + were learned, so it is where they should be one Tab away. """ super().__init__() self.title = f"Command line — {node_label}" @@ -89,7 +94,13 @@ class RemoteCliScreen(Screen): self._history = list(history) self._recall: int | None = None # index into history while ↑/↓ browse it self._draft = "" # what was typed before recall began, restored on ↓ past the end - self._completions = known_commands() + self._completions = sorted( + { + *known_commands(), + *(f"get {key}" for key in extra_keys), + *(f"set {key} " for key in extra_keys), + } + ) self.busy = False self._spinner = Spinner() self._prompt_line = 0 # body index of the input line (pinned into view) diff --git a/meshterm/ui/repeater_admin.py b/meshterm/ui/repeater_admin.py index 678aa33..0f7dced 100644 --- a/meshterm/ui/repeater_admin.py +++ b/meshterm/ui/repeater_admin.py @@ -21,6 +21,16 @@ Beyond the settings, the action rows cover the box itself — advert, clock sync admin password, reboot, each behind its own floating confirmation — and the **Command line** opens the readline-style remote CLI (:mod:`meshterm.ui.remote_cli`) for anything the catalog doesn't spell. + +That command line is also where the page *grows*. Third-party builds carry settings the +catalog has never heard of, and MeshTerm can't know which build a node is running without +asking it — every question being one paced round trip, and every guessed row a dead ``n/a`` +on everyone else's page. So nothing is probed speculatively: a ``get``/``set`` the reader +ran here, which *this node* answered, earns a row on *this node's* page under **Extra** +(:func:`learn_from_cli`). The round trip was one the reader was spending anyway, the catalog +doesn't grow, and no other node's page changes. A row the node stops answering goes again — +which is what tidies up after a board swap onto the same identity — and ``Del`` forgets one +without waiting for a read. """ from __future__ import annotations @@ -36,12 +46,18 @@ from rich.text import Text from ..core.models import Contact from ..core.remote_config import ( + DISCOVERED_CATEGORY, + NEVER_STORED, REPEATER_SETTINGS, RemoteSetting, + composite_members, composite_reads, + discovered_setting, get_setting, + learnable_from_get, normalize_value, parse_reply_value, + parse_setting_command, range_hint, read_plan, reply_is_error, @@ -54,6 +70,7 @@ from .menus import ( PICK_LOCATION_LABEL, confirm_discard, exit_rows, + fit_cells, icon_lane, lane_header, lane_row, @@ -63,7 +80,7 @@ from .menus import ( ) from .trace_screen import TracingDialog from .tui import Choice, Separator -from .tui.select import SelectScreen, splice_hint +from .tui.select import DeleteRequest, SelectScreen, splice_hint from .tui.spinner import Spinner, spinner_interval if TYPE_CHECKING: @@ -88,6 +105,50 @@ _CANCEL = "__cancel__" #: The footer atom naming the chord that re-reads the highlighted setting. READ_ONE_HINT = "^R read" +#: The footer atom naming the key that drops a discovered row (surfaced on those rows only). +FORGET_HINT = "Del forget" + +#: The narrowest the value lane may be squeezed to for a discovered setting — a floor, so a +#: page whose catalog rows are all ``?`` still shows something of what the node answered. +_EXTRA_VALUE_MIN = 14 + + +def _extra_value(text: str, width: int) -> str: + """One discovered value, fitted for the lane: whitespace collapsed, a long reply cut. + + A discovered setting's value is whatever the node said, and a build that answers a whole + diagnostic line (``desired=off effective=off supported=yes …``) would otherwise set the + value lane's width — one width, for every row on the page — and push each setting's + description off the edge. The row is cut here instead of the page being reshaped around + it; the command line is where the whole reply is read. + """ + flat = " ".join(text.split()) + return fit_cells(flat, width) if cell_len(flat) > width else flat + + +def _spec_for(key: str, cache: dict) -> RemoteSetting | None: + """The setting ``key`` names on this node: the catalog's, or its own discovered one.""" + spec = get_setting(key) + if spec is not None: + return spec + cached = cache.get(key) + return discovered_setting(key) if cached is not None and cached.discovered else None + + +def _discovered_specs(cache: dict) -> list[RemoteSetting]: + """This node's discovered settings, in key order — rows the catalog knows nothing about.""" + return [discovered_setting(key) for key, cached in sorted(cache.items()) if cached.discovered] + + +def _all_specs(cache: dict) -> list[RemoteSetting]: + """Every setting this node's page draws: the catalog, plus what it taught us itself.""" + return [*REPEATER_SETTINGS, *_discovered_specs(cache)] + + +def _extra_keys(cache: dict) -> frozenset[str]: + """The keys on this node's page that came from the node rather than from the catalog.""" + return frozenset(key for key, cached in cache.items() if cached.discovered) + @dataclass(frozen=True, slots=True) class ReadOne: @@ -121,6 +182,9 @@ class AdminMenu(SelectScreen): floating = False + #: The keys on this node's page that the catalog hasn't got, refreshed with the rows. + extra_keys: frozenset[str] = frozenset() + def __init__(self, title: str, items: list, **kwargs: Any) -> None: """Build the page's list, its rows ending at the edge rather than sliding under ←→. @@ -134,14 +198,28 @@ class AdminMenu(SelectScreen): kwargs.setdefault("hscroll", False) super().__init__(title, items, **kwargs) - def _readable_key(self) -> str | None: - """The highlighted row's setting key, when it is a setting the node can be asked.""" + def _row_key(self) -> str | None: + """The highlighted row's setting key, or ``None`` on an action row.""" current = self._current_choice() if current is None or not isinstance(current.value, str): return None - spec = get_setting(current.value) + return current.value + + def _readable_key(self) -> str | None: + """The highlighted row's setting key, when it is a setting the node can be asked.""" + key = self._row_key() + if key is None: + return None + if key in self.extra_keys: + return key # a discovered row is a string setting, and always re-readable + spec = get_setting(key) return spec.key if spec is not None and spec.readable else None + def _forgettable_key(self) -> str | None: + """The highlighted row's key, when it is a discovered row Del can drop.""" + key = self._row_key() + return key if key is not None and key in self.extra_keys else None + @property def footer_hint(self) -> str: # type: ignore[override] """The list's own hint, plus the read atom while a readable setting is highlighted.""" @@ -150,16 +228,26 @@ class AdminMenu(SelectScreen): @property def fkey_lane(self): - """The list's lane with ``Read`` on F3 — dim on a row there is nothing to read. + """The list's lane with ``Read`` on F3 and ``Forget`` behind it — dim where inert. F1/F2 are this grouped list's section jumps and F4/F5 the pager; F3 is the slot a - select list spends on its own verb. Dim rather than empty on an action row: reading - is a thing on this screen, just not for the row the cursor is on. + select list spends on its own verb. Read and Forget are the same slot's two halves + because they are the same subject — *this row* — and they are never both live: a + catalog row can be read and not forgotten, and only a discovered row can be dropped. + Dim rather than empty on an action row: both are things on this screen, just not for + the row the cursor is on. """ from .tui.fkeys import FPair lane = list(super().fkey_lane) - lane[2] = FPair("Read", "retry", enabled=self._readable_key() is not None) + lane[2] = FPair( + "Read", + "retry", + opp_label="Forget", + opp_action="delete", + enabled=self._readable_key() is not None, + opp_enabled=self._forgettable_key() is not None, + ) return lane def handle(self, action: str, data: str = "") -> None: @@ -292,7 +380,9 @@ async def _admin_session(ctx: AppContext, device: Device, node: Contact) -> dict title, items, footer_hint="↑↓ move · type to filter · Enter select · Esc back", + delete_hint=FORGET_HINT, ) + menu.extra_keys = _extra_keys(cache) async with session.stay(menu) as visit: while True: choice = await visit.result() @@ -304,13 +394,17 @@ async def _admin_session(ctx: AppContext, device: Device, node: Contact) -> dict continue return {"node": node.name, "applied": applied} if isinstance(choice, ReadOne): - spec = get_setting(choice.key) + spec = _spec_for(choice.key, cache) if spec is not None: await read_settings(ctx, device, node, [spec]) + elif isinstance(choice, DeleteRequest): + await _forget_discovered(ctx, node, str(choice.value), pending) elif choice == _APPLY: applied += await _apply(ctx, device, node, pending) elif choice == _READ: - await read_settings(ctx, device, node, REPEATER_SETTINGS) + # This node's page, not the catalog: a discovered row is refreshed by the + # same sweep as everything else, and drops off it if the node stops answering. + await read_settings(ctx, device, node, _all_specs(cache)) elif choice == _LOCATION: await _stage_location(ctx, cache, pending) elif choice == _CLI: @@ -349,7 +443,7 @@ async def _admin_session(ctx: AppContext, device: Device, node: Contact) -> dict danger=True, ) else: # a setting key - spec = get_setting(str(choice)) + spec = _spec_for(str(choice), cache) if spec is not None and not spec.writable: # A read-only fact has nothing to stage; asking again is all Enter can do. await read_settings(ctx, device, node, [spec]) @@ -359,6 +453,7 @@ async def _admin_session(ctx: AppContext, device: Device, node: Contact) -> dict # the values are the ones the action just produced. cache = ctx.remote_store.settings(node) title, items = _menu_items(node, cache, pending) + menu.extra_keys = _extra_keys(cache) menu.replace_items(items, title=title) @@ -383,6 +478,25 @@ def _menu_items(node: Contact, cache: dict, pending: dict[str, str]) -> tuple[st rows.append((spec.label, _value_text(spec, cache, pending), spec.help, spec.key)) sections.append((category, rows)) + # What this node taught us itself, last and in its own section — absent entirely on a + # node that has taught us nothing, which is every node until someone asks it something. + # Its values are fitted to the lane the catalog's rows already need: the value lane is + # one width for the whole page, so an unbounded reply here would push every setting's + # description right, off the edge of a 72-column screen, and reshape a page the reader + # opened to read the catalog. + extra = _discovered_specs(cache) + if extra: + budget = max( + _EXTRA_VALUE_MIN, + max((cell_len(value.plain) for _, rows in sections for _, value, _, _ in rows)), + ) + sections.append( + ( + DISCOVERED_CATEGORY, + [(s.label, _value_text(s, cache, pending, budget), s.help, s.key) for s in extra], + ) + ) + label_w = max(cell_len(label) for _, rows in sections for label, _, _, _ in rows) value_w = max(cell_len(value.plain) for _, rows in sections for _, value, _, _ in rows) @@ -394,7 +508,13 @@ def _menu_items(node: Contact, cache: dict, pending: dict[str, str]) -> tuple[st items.append(section_heading(category)) for label, value, help_text, key in rows: items.append( - Choice(title=lane_row(label, value, help_text, label_w, value_w), value=key) + Choice( + title=lane_row(label, value, help_text, label_w, value_w), + value=key, + # Only a discovered row can be dropped: a catalog row that goes away + # would come straight back on the next paint, the catalog still naming it. + deletable=category == DISCOVERED_CATEGORY, + ) ) # ↻ and ⌨ are one cell where 📡 🕒 🔐 🔄 are two, so the column is measured once and @@ -424,7 +544,9 @@ def _menu_items(node: Contact, cache: dict, pending: dict[str, str]) -> tuple[st return title, items -def _value_text(spec: RemoteSetting, cache: dict, pending: dict[str, str]) -> Text: +def _value_text( + spec: RemoteSetting, cache: dict, pending: dict[str, str], width: int = _EXTRA_VALUE_MIN +) -> Text: """One setting's VALUE lane: what the node last said, and any staged arrow. Four states, each its own word: the value; ``empty`` for a string the node holds blank; @@ -441,10 +563,14 @@ def _value_text(spec: RemoteSetting, cache: dict, pending: dict[str, str]) -> Te value = Text("n/a", style="muted") elif cached.value == "": value = Text("empty", style="muted") + elif spec.discovered: + value = Text(_extra_value(cached.value, width)) else: value = Text(spec.display(cached.value)) if spec.key in pending: - value.append(f" → {spec.display(pending[spec.key])}", style="warn") + staged = pending[spec.key] + shown = _extra_value(staged, width) if spec.discovered else spec.display(staged) + value.append(f" → {shown}", style="warn") return value @@ -453,7 +579,7 @@ def _value_text(spec: RemoteSetting, cache: dict, pending: dict[str, str]) -> Te async def _stage_setting(ctx: AppContext, key: str, cache: dict, pending: dict[str, str]) -> None: """Prompt for one setting's new value and stage it (nothing is sent yet).""" - spec = get_setting(key) + spec = _spec_for(key, cache) if spec is None or not spec.writable: # pragma: no cover - menu offers only real keys return cached = cache.get(key) @@ -508,6 +634,33 @@ async def _stage_setting(ctx: AppContext, key: str, cache: dict, pending: dict[s pending[key] = value +async def _forget_discovered( + ctx: AppContext, node: Contact, key: str, pending: dict[str, str] +) -> None: + """Drop one discovered row from this node's page, behind a confirm. + + The row usually cleans itself up — a node that stops answering the key loses it on the + next read — but that costs a round trip and a node in reach, and neither is a given + after the board it was learned from has been replaced. So Del asks, and forgets. It is + a red confirm like any other single-record delete, though nothing on the node changes: + what goes is what MeshTerm remembers, and asking the key again brings the row back. + """ + from .tui import CANCEL + + choice = await ctx.ui.dialog( + f"Stop showing {key} on {node.name}? Nothing on the node changes — " + "ask it for the setting again and the row comes back.", + [("Cancel", CANCEL), ("Forget", "forget")], + title="Forget setting", + default=1, + destructive=True, + ) + if choice != "forget": + return + pending.pop(key, None) + ctx.remote_store.forget_setting(node, key) + + async def _stage_location(ctx: AppContext, cache: dict, pending: dict[str, str]) -> None: """Pick the node's advertised location on the map and stage both coordinates it sets. @@ -598,8 +751,11 @@ async def _apply(ctx: AppContext, device: Device, node: Contact, pending: dict[s remember_reply(ctx, node, fills, reply) writes = write_plan(pending, _known_values(ctx, node)) + # The catalog plus this node's own discovered rows: a staged key that only this node + # has is still a row with a label to name in the outcome, and a write to count. + specs = _all_specs(ctx.remote_store.settings(node)) for i, write in enumerate(writes, start=1): - staged = [s for s in REPEATER_SETTINGS if s.key in write.values and s.key in pending] + staged = [s for s in specs if s.key in write.values and s.key in pending] names = ", ".join(f"{s.label} = {s.display(write.values[s.key])}" for s in staged) if write.missing: unread = ", ".join(s.label for s in map(get_setting, write.missing) if s) @@ -668,13 +824,85 @@ def remember_reply(ctx: AppContext, node: Contact, fills: list[RemoteSetting], r for spec in fills: value = parse_reply_value(spec, reply) # None for an error reply, too if value is None: - ctx.remote_store.remember_unsupported(node, spec.key) + if spec.discovered: + # A discovered row's whole claim to exist is that this node answered the + # key. When it stops, the claim is gone and so is the row — ``n/a`` is for + # a key the *catalog* says might be there, and nothing says that here. This + # is what cleans the page up after a board swap onto the same identity. + ctx.remote_store.forget_setting(node, spec.key) + else: + ctx.remote_store.remember_unsupported(node, spec.key) else: ctx.remote_store.remember_setting(node, spec.key, value) got += 1 return got +def learn_from_cli(ctx: AppContext, node: Contact, command: str, reply: str) -> None: + """Fold what one command-line exchange proved about ``node`` into its page. + + The command line is the only place a setting outside the catalog can be reached, so it + is also the only place one can be *found*: a ``get``/``set`` this node answered is proof + the key is there, and the row it earns costs no round trip nobody asked for. The catalog + doesn't grow — a key learned here belongs to this node alone (see + :func:`~meshterm.core.remote_config.discovered_setting`), and every other node's page is + what it always was. + + Three cases, in the order they are tested: + + * A **composite's own key** (``get radio``) fills all four of its fields, exactly as a + read of any one of them does. + * A **catalog key** folds into the cache the way the sweep or an Apply would — which is + also how ``get tx`` at the command line stopped leaving the TX power row stale. + * Anything else is **discovered**, and the two verbs are not equally good evidence. + ``handleSetCmd`` matches a key with its trailing space and refuses an unknown one + outright, so an accepted write is proof. ``handleGetCmd`` matches on a bare prefix and + will answer a key it hasn't got out of a shorter key's branch, so a read is proof only + when the reply cannot be that shorter key's + (:func:`~meshterm.core.remote_config.learnable_from_get`). + + ``prv.key`` is never stored, whichever way it was asked + (:data:`~meshterm.core.remote_config.NEVER_STORED`). + """ + parsed = parse_setting_command(command) + if parsed is None or parsed.key in NEVER_STORED: + return + members = composite_members(parsed.key) + spec = _spec_for(parsed.key, ctx.remote_store.settings(node)) + + if parsed.verb == "get": + if members: + remember_reply(ctx, node, members, reply) + elif spec is not None: + remember_reply(ctx, node, [spec], reply) + elif not reply_is_error(reply) and learnable_from_get( + parsed.key, reply, _known_values(ctx, node) + ): + found = discovered_setting(parsed.key) + value = parse_reply_value(found, reply) + if value is not None: + ctx.remote_store.remember_discovered(node, found.key, value) + return + + if reply_is_error(reply): + return # a refused write says nothing about the key, and nothing about its value + if members: + for member in members: + value = parse_reply_value(member, parsed.value) + if value is not None: + ctx.remote_store.remember_setting(node, member.key, value) + return + if spec is not None and not spec.discovered: + value = parse_reply_value(spec, parsed.value) + if value is None: + return # sent in words the catalog can't read back; leave the row as it was + ctx.remote_store.remember_setting(node, spec.key, value) + for other in spec.overlaps: + ctx.remote_store.forget_setting(node, other) + return + ctx.remote_store.remember_discovered(node, parsed.key, parsed.value) + + async def read_settings( ctx: AppContext, device: Device, node: Contact, specs: Iterable[RemoteSetting] ) -> None: @@ -870,6 +1098,7 @@ async def _command_line(ctx: AppContext, device: Device, node: Contact) -> None: if reply is None: screen.failed(f"no reply within {_REPLY_TIMEOUT_S:.0f} s") else: + learn_from_cli(ctx, node, command, reply) screen.reply(reply) screen = RemoteCliScreen( @@ -877,6 +1106,7 @@ async def _command_line(ctx: AppContext, device: Device, node: Contact) -> None: history=ctx.remote_store.history(node), send=send, session=session, + extra_keys=_extra_keys(ctx.remote_store.settings(node)), ) async def animate() -> None: diff --git a/tests/test_repeater_admin.py b/tests/test_repeater_admin.py index 43fe1fe..12ea7c0 100644 --- a/tests/test_repeater_admin.py +++ b/tests/test_repeater_admin.py @@ -23,12 +23,16 @@ from meshterm.core.connection import MockDevice from meshterm.core.device_store import DeviceStore from meshterm.core.models import Contact, utcnow from meshterm.core.remote_config import ( + DISCOVERED_HELP, REPEATER_SETTINGS, + SettingCommand, composite_reads, + discovered_setting, get_setting, known_commands, normalize_value, parse_reply_value, + parse_setting_command, range_hint, read_plan, reply_is_error, @@ -780,3 +784,199 @@ def test_admin_actions_start_every_label_in_the_same_cell() -> None: starts[label] = cell_len(plain[: plain.index(label)]) assert set(starts) == set(labels) assert len(set(starts.values())) == 1, f"a label starts a column early: {starts}" + + +# --- settings discovered on the command line ------------------------------------------- + + +def test_only_get_and_set_are_read_as_settings() -> None: + """The CLI's own grammar is the filter; every top-level verb falls out of it. + + Nothing is excluded by name: ``reboot``, ``advert`` and ``powersaving`` are simply not + ``get `` or ``set ``, and ``setperm`` only starts with the letters. + """ + assert parse_setting_command("get radio.rxps") == SettingCommand("get", "radio.rxps") + assert parse_setting_command(" set reboot.interval 24 ") == SettingCommand( + "set", "reboot.interval", "24" + ) + assert parse_setting_command("set owner.info a b c") == SettingCommand( + "set", "owner.info", "a b c" + ) + for command in ( + "reboot", + "advert", + "clock sync", + "powersaving on", + "setperm ab12 rw", + "start ota", + "get", # no key + "set tx", # no value + "get ../etc", # not a key's spelling + ): + assert parse_setting_command(command) is None, command + + +def test_a_write_the_node_accepts_is_proof_the_key_is_there(tui_ctx) -> None: + """``set`` teaches a row: the firmware matches its keys exactly and refuses the rest. + + ``handleSetCmd`` compares ``"radio "`` *with* the trailing space, so a longer key can + never land in a shorter one's branch, and an unknown one answers ``unknown config:``. + """ + repeater_admin.learn_from_cli(tui_ctx, NODE, "set radio.rxps balanced", "OK") + cached = tui_ctx.remote_store.settings(NODE)["radio.rxps"] + assert (cached.value, cached.discovered) == ("balanced", True) + + repeater_admin.learn_from_cli(tui_ctx, NODE, "set nonesuch 1", "unknown config: nonesuch") + assert "nonesuch" not in tui_ctx.remote_store.settings(NODE) + + +def test_a_read_answered_out_of_a_shorter_keys_branch_is_no_discovery(tui_ctx) -> None: + """``get`` matches on a bare prefix, so a plausible reply can belong to another key. + + Stock firmware answers ``get radio.rxps`` from the ``get radio`` branch. With no typing + to reject it — a discovered key has none — the guard is that the reply is exactly what + the shadowing key last said. + """ + for key, value in (("freq", "910.525"), ("bw", "62.5"), ("sf", "7"), ("cr", "5")): + tui_ctx.remote_store.remember_setting(NODE, key, value) + + repeater_admin.learn_from_cli(tui_ctx, NODE, "get radio.rxps", "> 910.525,62.5,7,5") + assert "radio.rxps" not in tui_ctx.remote_store.settings(NODE) + + repeater_admin.learn_from_cli(tui_ctx, NODE, "get radio.rxps", "> desired=off level=2") + assert tui_ctx.remote_store.settings(NODE)["radio.rxps"].value == "desired=off level=2" + + +def test_a_read_nothing_shadows_is_taken_at_its_word(tui_ctx) -> None: + """No catalog key prefixes ``reboot.interval``, so its reply can only be its own.""" + repeater_admin.learn_from_cli(tui_ctx, NODE, "get reboot.interval", "> 24") + assert tui_ctx.remote_store.settings(NODE)["reboot.interval"].value == "24" + + repeater_admin.learn_from_cli(tui_ctx, NODE, "get nonesuch", "??: nonesuch") + assert "nonesuch" not in tui_ctx.remote_store.settings(NODE) + + +def test_an_unread_shadow_leaves_the_question_unanswerable(tui_ctx) -> None: + """A shadowed key whose shadow was never read is not discovered: we cannot tell.""" + repeater_admin.learn_from_cli(tui_ctx, NODE, "get radio.rxps", "> 910.525,62.5,7,5") + assert "radio.rxps" not in tui_ctx.remote_store.settings(NODE) + + +def test_the_private_key_is_never_written_to_disk(tui_ctx, tmp_path: Path) -> None: + """``get prv.key`` prints in the transcript and goes no further.""" + secret = "ab" * 32 + repeater_admin.learn_from_cli(tui_ctx, NODE, "get prv.key", "> " + secret) + assert "prv.key" not in tui_ctx.remote_store.settings(NODE) + written = [p for p in tmp_path.rglob("*.json") if secret in p.read_text(encoding="utf-8")] + assert written == [] + + +def test_a_catalog_key_read_on_the_command_line_refreshes_its_row(tui_ctx) -> None: + """The cache is the page, so the command line fills it exactly as a read does. + + ``get tx`` used to leave the TX power row showing what it said last week, and + ``get radio`` answered four rows that none of them heard about. + """ + repeater_admin.learn_from_cli(tui_ctx, NODE, "get tx", "> 20") + repeater_admin.learn_from_cli(tui_ctx, NODE, "get radio", "> 869.525,250,10,6") + repeater_admin.learn_from_cli(tui_ctx, NODE, "set txdelay 1.5", "OK") + cache = tui_ctx.remote_store.settings(NODE) + assert cache["tx"].value == "20" + assert (cache["freq"].value, cache["sf"].value) == ("869.525", "10") + assert cache["txdelay"].value == "1.5" + assert not any(cached.discovered for cached in cache.values()) + + +def test_a_write_stales_the_other_spelling_from_the_command_line_too(tui_ctx) -> None: + """``set dutycycle`` at the prompt drops the cached ``af``, as an Apply would.""" + tui_ctx.remote_store.remember_setting(NODE, "af", "1.00") + repeater_admin.learn_from_cli(tui_ctx, NODE, "set dutycycle 40", "OK - 40.0%") + cache = tui_ctx.remote_store.settings(NODE) + assert cache["dutycycle"].value == "40.0" and "af" not in cache + + +def test_a_discovered_row_survives_a_re_read_and_goes_when_the_key_does(tui_ctx) -> None: + """The node's answer is the row's whole claim to exist — including after a board swap. + + A refreshed value keeps the row discovered (or the page would hold a row the catalog + cannot draw); a key that stops answering loses it, rather than earning the ``n/a`` that + only a catalog key can. + """ + spec = discovered_setting("radio.rxps") + tui_ctx.remote_store.remember_discovered(NODE, "radio.rxps", "off") + + repeater_admin.remember_reply(tui_ctx, NODE, [spec], "> balanced") + cached = tui_ctx.remote_store.settings(NODE)["radio.rxps"] + assert (cached.value, cached.discovered) == ("balanced", True) + + repeater_admin.remember_reply(tui_ctx, NODE, [spec], "??: radio.rxps") + assert "radio.rxps" not in tui_ctx.remote_store.settings(NODE) + + +def test_discovered_rows_draw_last_in_their_own_section_and_are_deletable() -> None: + """A node's own settings sit under ``Extra`` — and only they answer Del.""" + from meshterm.ui.tui import Choice + + plain = {"txdelay": CachedValue("0.5", utcnow())} + catalog_rows = [ + item + for item in repeater_admin._menu_items(NODE, plain, {})[1] + if isinstance(item, Choice) and item.deletable + ] + assert catalog_rows == [], "no catalog row may be forgotten" + + cache = dict(plain, **{"radio.rxps": CachedValue("balanced", utcnow(), discovered=True)}) + items = repeater_admin._menu_items(NODE, cache, {})[1] + deletable = [i for i in items if isinstance(i, Choice) and i.deletable] + assert [i.value for i in deletable] == ["radio.rxps"] + + rows = [i for i in items if isinstance(i, Choice) and isinstance(i.value, str)] + settings_rows = [r for r in rows if repeater_admin.get_setting(r.value) or r.deletable] + assert settings_rows[-1].value == "radio.rxps", "the node's own settings come last" + assert DISCOVERED_HELP in settings_rows[-1].title.plain + + +def test_a_staged_discovered_value_is_written_with_its_own_set() -> None: + """A key the catalog hasn't got is still written the one way anything can write it.""" + writes = write_plan({"radio.rxps": "balanced", "txdelay": "1.5"}, {}) + assert [w.command for w in writes] == ["set txdelay 1.5", "set radio.rxps balanced"] + + +def test_a_long_discovered_value_cannot_widen_the_whole_page() -> None: + """The value lane is one width for every row, so a node's own setting may not set it. + + Some builds answer ``get radio.rxps`` with a whole diagnostic line. Unbounded, that one + row would push every catalog setting's description right and off a 72-column screen — + reshaping a page the reader opened to read the catalog. + """ + from rich.cells import cell_len + + from meshterm.ui.tui import Choice + + def widest(cache) -> int: + items = repeater_admin._menu_items(NODE, cache, {})[1] + return max( + cell_len(i.title.plain) + for i in items + if isinstance(i, Choice) and hasattr(i.title, "plain") + ) + + catalog = { + "tx": CachedValue("20", utcnow()), + "owner.info": CachedValue("Jean-Pierre, Montreal", utcnow()), + } + chatty = dict( + catalog, + **{ + "radio.rxps": CachedValue( + "desired=off effective=off supported=yes level=0 preamble=16 rx=0 sleep=0", + utcnow(), + discovered=True, + ) + }, + ) + assert widest(chatty) == widest(catalog) + + fitted = repeater_admin._extra_value("a value far too long for its lane", 14) + assert cell_len(fitted) == 14 + assert repeater_admin._extra_value(" balanced\n", 14) == "balanced"