From c6e6ee8a60caa5af35f955c72f9d2009fa85a28f Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Tue, 25 Aug 2026 17:43:24 -0400 Subject: [PATCH] Chat: a contact the device forgot is offered back, not a wire error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sending to a contact the firmware no longer holds came back as `send failed: failed to send message to 'bob_caribou': {'error_code': 2, 'code_string': 'ERR_CODE_NOT_FOUND'}` — a wire constant in a status line, and no hint of what to do about it. It is reachable for a contact MeshTerm itself listed. The list a screen sees is the union of the device's live table and the contacts MeshTerm remembers for that device (core/contact_store), which exists so a RAM-only radio bridge still shows the nodes you know. The store claims those stay messageable without writing anything back — true for the bridge, false for a firmware companion, which addresses a direct message by looking the recipient up in its *own* table by a key prefix and refuses when nothing matches. So a contact the firmware has dropped still lists, still opens a chat, and only the send finds out it is gone. Now the rejection is its own class carrying the contact, and the chat offers the one write that fixes it: "bob_caribou isn't in this device's contacts, so the radio can't address it. Add it back and send?" — Cancel or Add & send, then the same send runs again. Declining reports the refusal in words. The write is offered rather than silent because it changes what the device stores, and a full contact table refuses it, which is worth seeing. Retry (^R) takes the same path. Adding a contact is new to the Device contract (the inverse of remove_contact, ADD_UPDATE_CONTACT on the wire): the record carries the key, name, type and last advertised position we already hold, with no learned route, so the first message floods exactly as it would for a freshly-heard node. The name is clipped to the 32 *bytes* the firmware field holds, not 32 characters. Every other rejection now reads as a sentence too — reject_reason maps the six error codes to plain words, so a send, an admin command, or any _ok()'d command says "the device's table is full" rather than printing its payload dict. The simulator refuses a recipient it doesn't hold, exactly as hardware does, so the whole offer is walkable under --mock. Co-Authored-By: Claude Opus 5 (1M context) --- meshterm/core/connection.py | 194 ++++++++++++++++++++++++++++++++++-- meshterm/ui/chat.py | 77 +++++++++++++- tests/test_chat.py | 165 ++++++++++++++++++++++++++++++ 3 files changed, 427 insertions(+), 9 deletions(-) diff --git a/meshterm/core/connection.py b/meshterm/core/connection.py index 240310c..3362559 100644 --- a/meshterm/core/connection.py +++ b/meshterm/core/connection.py @@ -18,6 +18,7 @@ import logging import random import sys from abc import ABC, abstractmethod +from dataclasses import replace from datetime import datetime, timedelta, timezone from typing import TYPE_CHECKING, Callable, Optional @@ -158,6 +159,37 @@ class DeviceCommandError(RuntimeError): """ +class ContactNotOnDeviceError(DeviceCommandError): + """The companion has no contact matching the recipient, so it cannot address it. + + Firmware addresses a direct message by looking the recipient up in *its own* contact + table (by a prefix of the public key) and answers ``ERR_CODE_NOT_FOUND`` when nothing + matches — the one send rejection with an obvious fix: put the contact back on the + device. A distinct :class:`DeviceCommandError` subclass so the chat screen can offer + exactly that (see :func:`~meshterm.ui.chat.open_chat`) while every other caller keeps + treating it as an ordinary command failure. + + This is reachable for a contact MeshTerm itself listed, because the contact list a + screen sees is the union of the device's live table and the ones MeshTerm remembers + for it (see :mod:`meshterm.core.contact_store`): a contact the firmware has since + dropped still lists, and only the send finds out it is gone. + + Attributes: + contact: The recipient the device could not find. + """ + + def __init__(self, contact: Contact) -> None: + """Explain the rejection in terms of the contact the device could not find. + + Args: + contact: The recipient the companion has no entry for. + """ + super().__init__( + f"{contact.name} isn't in this device's contacts — add it back to send." + ) + self.contact = contact + + class DeviceAuthenticationError(DeviceCommandError): """A Bluetooth companion refused the connection because it needs a pairing PIN/bond. @@ -169,6 +201,84 @@ class DeviceAuthenticationError(DeviceCommandError): """ +#: ``ERR_CODE_NOT_FOUND`` — the companion has no entry matching what a command addressed. +_ERR_NOT_FOUND = 2 + +#: What each companion error code means, in a sentence that finishes "the device …". +#: The wire carries only the number and the library's ``ERR_CODE_*`` spelling (see +#: ``meshcore.events.ErrorMessages``), which is diagnostic text, not something to put in +#: front of a user — :func:`reject_reason` turns it into the sentence below. +_ERROR_REASONS = { + 1: "the firmware doesn't support that command", + 2: "the device has no contact with that key", + 3: "the device's table is full", + 4: "the device isn't in a state to do that", + 5: "the device hit a storage error", + 6: "the device rejected the request as malformed", +} + + +def _clip_utf8(text: str, limit: int) -> str: + """Trim ``text`` to at most ``limit`` UTF-8 bytes, never splitting a character. + + Firmware fields are byte-sized, not character-sized, so a name with an accent in it can + overrun a field it looks short enough for. + + Args: + text: The value to fit. + limit: The field's size in bytes. + + Returns: + ``text`` itself when it fits, else its longest whole-character prefix that does. + """ + encoded = text.encode("utf-8") + if len(encoded) <= limit: + return text + return encoded[:limit].decode("utf-8", "ignore") + + +def error_code(result) -> Optional[int]: # noqa: ANN001 + """Return the companion error code carried by a rejected command's event, if any. + + Args: + result: The :class:`meshcore.events.Event` a command returned (or ``None``). + + Returns: + The ``error_code`` from the event's payload, or ``None`` when the rejection carried + no code (an absent reply, or a payload shaped some other way). + """ + payload = getattr(result, "payload", {}) or {} + if not isinstance(payload, dict): + return None + try: + return int(payload["error_code"]) + except (KeyError, TypeError, ValueError): + return None + + +def reject_reason(result) -> str: # noqa: ANN001 + """Explain a rejected command in one clause, for the end of a user-facing sentence. + + A rejection reaches us as ``{'error_code': 2, 'code_string': 'ERR_CODE_NOT_FOUND'}``; + pasting that dict into a status line (which is what every raise site used to do) shows + the user a wire constant and leaves them no wiser. This maps the code to plain words, + falling back to the raw payload only for a rejection with no code we know. + + Args: + result: The :class:`meshcore.events.Event` a command returned (or ``None``). + + Returns: + A lowercase clause naming the cause, e.g. ``"the device's table is full"``. + """ + code = error_code(result) + if code in _ERROR_REASONS: + return _ERROR_REASONS[code] + if result is None: + return "the device didn't answer" + payload = getattr(result, "payload", {}) or {} + return f"the device rejected it ({payload})" + + #: Exception class names that signal the link to the companion has dropped — the device was #: unplugged, powered off, moved out of range, or its port/transport otherwise vanished — as #: opposed to an ordinary command-level failure. Matched by name in :func:`is_connection_lost` @@ -392,6 +502,26 @@ class Device(ABC): The list of :class:`Contact` records currently stored on the device. """ + @abstractmethod + async def add_contact(self, node: Contact) -> None: + """Add (or update) a contact in the device's contact table. + + The inverse of :meth:`remove_contact`, and the fix for a node MeshTerm knows but the + firmware has forgotten: a direct message is addressed by the *device's* own contact + entry, so a contact missing from its table can't be messaged at all + (:class:`ContactNotOnDeviceError`) until it is written back. Everything the entry + needs travels on the contact — its public key, name, type and last advertised + position — and it is added with no learned route, so the first message floods + exactly as it would for a freshly-heard node. + + Args: + node: The contact to write; must carry a full public key. + + Raises: + DeviceCommandError: If the contact carries no public key to address it by, or + the device rejected the write (a full contact table, most often). + """ + @abstractmethod async def remove_contact(self, node: Contact) -> None: """Delete a contact from the device's contact table. @@ -1576,6 +1706,39 @@ class MeshCoreDevice(Device): ) return contacts + #: How many bytes of a name the firmware's contact record holds (a 32-byte field). + _CONTACT_NAME_BYTES = 32 + + async def add_contact(self, node: Contact) -> None: # noqa: D102 - inherited docstring + mc = self._require() + pub = self._node_pubkey(node) # raises DeviceCommandError if it has no key + if len(pub) != 64: + raise DeviceCommandError( + f"{node.name} is known only by a key prefix, so it can't be added to the " + "device — receive an advert from it first." + ) + # The library writes contacts through one add-or-update frame, from a record shaped + # exactly like the one a contacts read yields; a flood route (``out_path_len`` -1) is + # what a freshly-heard contact carries, and the device relearns a path from received + # traffic as usual. + record = { + "public_key": pub, + "type": int(node.node_type if node.node_type is not None else NODE_TYPE_CHAT), + "flags": 0, + "out_path": "", + "out_path_len": -1, + "out_path_hash_mode": 0, + "adv_name": _clip_utf8(node.name, self._CONTACT_NAME_BYTES), + "last_advert": int(node.last_seen.timestamp()) if node.last_seen else 0, + "adv_lat": float(node.lat or 0.0), + "adv_lon": float(node.lon or 0.0), + } + result = await mc.commands.add_contact(record) + if result is None or getattr(result, "is_error", lambda: False)(): + raise DeviceCommandError( + f"couldn't add {node.name} to the device: {reject_reason(result)}" + ) + async def remove_contact(self, node: Contact) -> None: # noqa: D102 - inherited docstring mc = self._require() pub = self._node_pubkey(node) # raises DeviceCommandError if it has no key @@ -1784,8 +1947,8 @@ class MeshCoreDevice(Device): sent = await mc.commands.send_cmd(pub, cmd) if sent is not None and getattr(sent, "is_error", lambda: False)(): raise DeviceCommandError( - f"failed to send admin command {cmd!r} to {node.name!r}: " - f"{getattr(sent, 'payload', {})}" + f"couldn't send admin command {cmd!r} to {node.name}: " + f"{reject_reason(sent)}" ) reply = await mc.wait_for_event(EventType.CONTACT_MSG_RECV, timeout=timeout) if reply is None: @@ -2190,9 +2353,13 @@ class MeshCoreDevice(Device): pub = self._node_pubkey(contact) result = await mc.commands.send_msg(pub, text) if result is None or getattr(result, "is_error", lambda: False)(): + # A recipient the firmware has no entry for is the one rejection with an obvious + # fix, so it gets its own class for the chat screen to offer that fix on; see + # :class:`ContactNotOnDeviceError` for how a listed contact can be missing here. + if error_code(result) == _ERR_NOT_FOUND: + raise ContactNotOnDeviceError(contact) raise DeviceCommandError( - f"failed to send message to {contact.name!r}: " - f"{getattr(result, 'payload', {})}" + f"couldn't send to {contact.name}: {reject_reason(result)}" ) # The companion acknowledges the send immediately with an ``expected_ack`` code and # a suggested wait; the recipient's delivery ACK arrives later carrying that code. @@ -2227,7 +2394,7 @@ class MeshCoreDevice(Device): RuntimeError: If the device reported an error. """ if event is not None and getattr(event, "is_error", lambda: False)(): - raise RuntimeError(f"device rejected command: {getattr(event, 'payload', {})}") + raise RuntimeError(f"device rejected the command: {reject_reason(event)}") return event async def get_tuning(self) -> dict: # noqa: D102 - inherited docstring @@ -2558,6 +2725,14 @@ class MockDevice(Device): async def get_contacts(self) -> list[Contact]: # noqa: D102 - inherited docstring return list(self._contacts) + async def add_contact(self, node: Contact) -> None: # noqa: D102 - inherited docstring + await asyncio.sleep(0) + key = self._mock_key(node) + self._contacts = [c for c in self._contacts if self._mock_key(c) != key] + # Added with no learned route, exactly as the firmware stores a contact it was + # handed rather than heard from. + self._contacts.append(replace(node, route_hops=None)) + async def remove_contact(self, node: Contact) -> None: # noqa: D102 - inherited docstring await asyncio.sleep(0) key = self._mock_key(node) @@ -2573,8 +2748,13 @@ class MockDevice(Device): self, contact: Contact, text: str ) -> Optional[Ack]: await asyncio.sleep(0) - # The simulator "delivers" instantly and always acknowledges, so outbound direct - # messages show as acked without a radio. + # Firmware can only address a contact it holds, so a recipient this simulated device + # doesn't have is refused exactly as hardware refuses one — which is what makes the + # chat screen's "add it back and send" offer walkable on the simulator. + if self._mock_key(contact) not in {self._mock_key(c) for c in self._contacts}: + raise ContactNotOnDeviceError(contact) + # Otherwise the simulator "delivers" instantly and always acknowledges, so outbound + # direct messages show as acked without a radio. return Ack(code="mock") async def send_channel_message( # noqa: D102 - inherited docstring diff --git a/meshterm/ui/chat.py b/meshterm/ui/chat.py index d7a2059..797c9b4 100644 --- a/meshterm/ui/chat.py +++ b/meshterm/ui/chat.py @@ -23,6 +23,7 @@ from rich.console import Group, RenderableType from rich.text import Text from ..core.channels import split_channel_sender +from ..core.connection import ContactNotOnDeviceError from ..core.events import EventKind, MeshEvent from ..core.models import ChatMessage, Contact, Conversation, Message, utcnow from .theme import name_style, snr_style @@ -1002,14 +1003,18 @@ async def open_chat(ctx: "AppContext", conversation: Conversation) -> int: conversation.channel_idx, text, label=conversation.label ) assert conversation.contact is not None - return await ctx.chat.send_direct(conversation.contact, text) + return await _with_restore( + ctx, lambda: ctx.chat.send_direct(conversation.contact, text) + ) resend: Optional[Callable[[ChatMessage], Awaitable[ChatMessage]]] = None if not conversation.is_channel: async def resend(message: ChatMessage) -> ChatMessage: assert conversation.contact is not None - return await ctx.chat.resend_direct(conversation.contact, message) + return await _with_restore( + ctx, lambda: ctx.chat.resend_direct(conversation.contact, message) + ) paths = await _make_paths_presenter(ctx, conversation, device) screen = ChatScreen( @@ -1043,6 +1048,74 @@ async def open_chat(ctx: "AppContext", conversation: Conversation) -> int: return len(screen._messages) +async def _with_restore(ctx: "AppContext", attempt: Callable[[], Awaitable[Any]]) -> Any: + """Run a direct send, offering to restore a contact the device has forgotten, then retry. + + The one rejection a send can recover from: the firmware has no contact for the recipient + (see :class:`~meshterm.core.connection.ContactNotOnDeviceError`), which one write puts + right. The user is asked first (:func:`_restore_contact`); declining re-raises, so the + chat reports the refusal exactly as it reports any other failed send. + + Args: + ctx: The shared application context. + attempt: The send to run — called a second time, unchanged, once the contact is back. + + Returns: + Whatever ``attempt`` returns. + + Raises: + Exception: Anything ``attempt`` raises; a forgotten-contact rejection is re-raised + only when the offer to add it back was declined. + """ + try: + return await attempt() + except ContactNotOnDeviceError as missing: + if not await _restore_contact(ctx, missing): + raise + return await attempt() + + +async def _restore_contact(ctx: "AppContext", missing: ContactNotOnDeviceError) -> bool: + """Offer to write a forgotten contact back to the device; ``True`` if it now holds it. + + A direct message is addressed by the *device's* own contact entry, so a contact the + firmware has dropped can't be messaged even though MeshTerm still lists it (the list a + screen sees is the union of the device's table and the ones MeshTerm remembers for it — + see :mod:`meshterm.core.contact_store`). Everything the entry needs is on the contact we + already hold, so the fix is one write; the send that hit the rejection retries after it. + + The write is offered rather than done silently: it changes what the device stores, and a + full contact table refuses it (which is worth seeing, not swallowing). + + Args: + ctx: The shared application context. + missing: The rejection, carrying the contact the device couldn't find. + + Returns: + ``True`` once the contact is on the device (retry the send), ``False`` if the user + declined — in which case the caller re-raises, so the chat reports the refusal. + + Raises: + DeviceCommandError: If the device refused the write (a full table, most often). + """ + contact = missing.contact + add = await ctx.ui.dialog( + f"{contact.name} isn't in this device's contacts, so the radio can't address it. " + "Add it back and send?", + [("Cancel", False), ("Add & send", True)], + title="Contact not on device", + default=1, + ) + if not add: + return False + device = await ctx.device() + await device.add_contact(contact) + # The device's table just changed under the session cache; the next read re-fetches it + # (with the route the firmware learns for the contact from here on). + ctx.devstate.invalidate_contacts() + return True + + def _contact_names(contacts: list[Contact]) -> dict[str, str]: """Build a key-prefix → name map for labeling inbound direct messages.""" names: dict[str, str] = {} diff --git a/tests/test_chat.py b/tests/test_chat.py index 7392cdc..705c1ea 100644 --- a/tests/test_chat.py +++ b/tests/test_chat.py @@ -111,6 +111,171 @@ async def test_mock_send_direct_returns_ack() -> None: await device.disconnect() +async def test_sending_to_a_contact_the_device_forgot_is_refused_by_name() -> None: + """A recipient the radio has no entry for is refused in words, naming the contact. + + MeshTerm lists the union of the device's contacts and the ones it remembers for that + device, so a contact the firmware dropped still appears and only the send finds out. + The rejection has to carry the contact, so the chat can offer to write it back. + """ + from meshterm.core.connection import ContactNotOnDeviceError + + device = MockDevice() + await device.connect() + stranger = Contact(name="bob_caribou", public_key="a3" * 32, key_prefix="a3a3a3a3") + with pytest.raises(ContactNotOnDeviceError) as caught: + await device.send_direct_message(stranger, "hello") + assert caught.value.contact is stranger + assert "bob_caribou" in str(caught.value) + assert "error_code" not in str(caught.value) # never the wire payload + await device.disconnect() + + +async def test_adding_a_forgotten_contact_back_makes_it_messageable() -> None: + """Writing the contact to the device is the whole fix — the same send then lands.""" + device = MockDevice() + await device.connect() + stranger = Contact(name="bob_caribou", public_key="a3" * 32, key_prefix="a3a3a3a3") + await device.add_contact(stranger) + assert "bob_caribou" in {c.name for c in await device.get_contacts()} + assert await device.send_direct_message(stranger, "hello") is not None + await device.disconnect() + + +async def test_restore_contact_writes_only_when_the_dialog_is_accepted() -> None: + """The device write is offered, not silent — declining leaves the table untouched.""" + from meshterm.core.connection import ContactNotOnDeviceError + from meshterm.ui.chat import _restore_contact + + class _Ui: + def __init__(self, answer: bool) -> None: + self.answer = answer + self.prompt = "" + + async def dialog(self, prompt, buttons, **kwargs): # noqa: ANN001, ANN003 + self.prompt = prompt + return self.answer + + class _Devstate: + def __init__(self) -> None: + self.invalidated = 0 + + def invalidate_contacts(self) -> None: + self.invalidated += 1 + + class _Ctx: + def __init__(self, device: MockDevice, answer: bool) -> None: + self._device = device + self.ui = _Ui(answer) + self.devstate = _Devstate() + + async def device(self) -> MockDevice: + return self._device + + stranger = Contact(name="bob_caribou", public_key="a3" * 32, key_prefix="a3a3a3a3") + missing = ContactNotOnDeviceError(stranger) + + device = MockDevice() + await device.connect() + before = len(await device.get_contacts()) + + declined = _Ctx(device, answer=False) + assert await _restore_contact(declined, missing) is False + assert len(await device.get_contacts()) == before + assert declined.devstate.invalidated == 0 + assert "bob_caribou" in declined.ui.prompt + + accepted = _Ctx(device, answer=True) + assert await _restore_contact(accepted, missing) is True + assert len(await device.get_contacts()) == before + 1 + assert accepted.devstate.invalidated == 1 + await device.disconnect() + + +async def test_a_declined_restore_lets_the_rejection_stand() -> None: + """The send retries once the contact is back, and reports the refusal when it isn't.""" + from meshterm.core.connection import ContactNotOnDeviceError + from meshterm.ui.chat import _with_restore + + stranger = Contact(name="bob_caribou", public_key="a3" * 32) + tries = 0 + + async def attempt() -> str: + nonlocal tries + tries += 1 + if tries == 1: + raise ContactNotOnDeviceError(stranger) + return "sent" + + class _Ctx: + def __init__(self, restored: bool) -> None: + self.restored = restored + + async def restore(ctx, missing) -> bool: # noqa: ANN001 + return ctx.restored + + import meshterm.ui.chat as chat_module + + original = chat_module._restore_contact + chat_module._restore_contact = restore + try: + with pytest.raises(ContactNotOnDeviceError): + await _with_restore(_Ctx(restored=False), attempt) + assert tries == 1 # declined: no second transmission + tries = 0 + assert await _with_restore(_Ctx(restored=True), attempt) == "sent" + assert tries == 2 # the same send, run again once the contact was written back + finally: + chat_module._restore_contact = original + + +def test_a_rejected_command_is_explained_in_words_not_wire_codes() -> None: + """Error codes become sentences; only an unrecognised rejection shows its payload.""" + from meshterm.core.connection import reject_reason + + class _Ev: + def __init__(self, payload: dict) -> None: + self.payload = payload + + assert reject_reason(_Ev({"error_code": 2})) == "the device has no contact with that key" + assert reject_reason(_Ev({"error_code": 3})) == "the device's table is full" + assert reject_reason(None) == "the device didn't answer" + assert "97" in reject_reason(_Ev({"error_code": 97})) + + +async def test_add_contact_writes_a_flood_record_with_the_name_the_field_fits() -> None: + """The record the firmware gets carries no learned route and a byte-clipped name.""" + from meshterm.core.connection import MeshCoreDevice + + class _Ok: + payload: dict = {} + + def is_error(self) -> bool: + return False + + class _FakeContacts: + def __init__(self) -> None: + self.record = None + + async def add_contact(self, record): # noqa: ANN001 + self.record = record + return _Ok() + + class _FakeMc: + def __init__(self) -> None: + self.commands = _FakeContacts() + + device = MeshCoreDevice.__new__(MeshCoreDevice) + mc = _FakeMc() + device._require = lambda: mc # type: ignore[method-assign] + long_name = "é" * 40 # 80 bytes: twice what the firmware's name field holds + await device.add_contact(Contact(name=long_name, public_key="a3" * 32, node_type=1)) + record = mc.commands.record + assert record["out_path_len"] == -1 and record["out_path"] == "" + assert len(record["adv_name"].encode("utf-8")) == 32 + assert record["public_key"] == "a3" * 32 + + async def test_message_pump_drains_until_empty() -> None: """The RX pump pulls get_msg() until the queue is empty (the pull model).