diff --git a/src/meshcore/commands/base.py b/src/meshcore/commands/base.py index 9e0f00e..dcd4673 100644 --- a/src/meshcore/commands/base.py +++ b/src/meshcore/commands/base.py @@ -58,7 +58,7 @@ def _validate_destination(dst: DestinationType, prefix_length: int = 6) -> bytes class CommandHandlerBase: - DEFAULT_TIMEOUT = 5.0 + DEFAULT_TIMEOUT = 15.0 def __init__(self, default_timeout: Optional[float] = None): self._sender_func: Optional[Callable[[bytes], Coroutine[Any, Any, None]]] = None @@ -243,18 +243,31 @@ class CommandHandlerBase: logger.debug(f"Binary request to {dst_bytes.hex()}") data = b"\x32" + dst_bytes + request_type.value.to_bytes(1, "little", signed=False) + (data if data else b"") - result = await self.send(data, [EventType.MSG_SENT, EventType.ERROR]) - - # Register the request with the reader if we have both reader and request_type - if (result.type == EventType.MSG_SENT and - self._reader is not None and - request_type is not None): - - exp_tag = result.payload["expected_ack"].hex() - # Use provided timeout or fallback to suggested timeout (with 5s default) - actual_timeout = timeout if timeout is not None and timeout > 0 else result.payload.get("suggested_timeout", 4000) / 800.0 + # Pre-register a placeholder binary request before send() to close the race + # window where a BINARY_RESPONSE could arrive between send() returning and + # registration. The placeholder tag is patched to the real tag once MSG_SENT + # returns. If send() fails, the placeholder is cleaned up. + placeholder_tag = None + if self._reader is not None and request_type is not None: + placeholder_tag = f"_pending_{id(data)}" + actual_timeout = timeout if timeout is not None and timeout > 0 else self.default_timeout actual_timeout = min_timeout if actual_timeout < min_timeout else actual_timeout - self._reader.register_binary_request(pubkey_prefix.hex(), exp_tag, request_type, actual_timeout, context=context) + self._reader.register_binary_request(pubkey_prefix.hex(), placeholder_tag, request_type, actual_timeout, context=context) + + result = await self.send(data, [EventType.MSG_SENT, EventType.ERROR]) + + # Patch the placeholder tag with the real tag from MSG_SENT, or clean up on failure + if placeholder_tag is not None and self._reader is not None: + # Remove the placeholder entry + self._reader.pending_binary_requests.pop(placeholder_tag, None) + if (result.type == EventType.MSG_SENT and + request_type is not None): + exp_tag = result.payload["expected_ack"].hex() + # Use suggested_timeout from the result if available + actual_timeout = timeout if timeout is not None and timeout > 0 else result.payload.get("suggested_timeout", 4000) / 800.0 + actual_timeout = min_timeout if actual_timeout < min_timeout else actual_timeout + # Register with the real tag + self._reader.register_binary_request(pubkey_prefix.hex(), exp_tag, request_type, actual_timeout, context=context) return result diff --git a/src/meshcore/commands/binary.py b/src/meshcore/commands/binary.py index f1578d0..5720a26 100644 --- a/src/meshcore/commands/binary.py +++ b/src/meshcore/commands/binary.py @@ -73,10 +73,6 @@ class BinaryCommandHandler(CommandHandlerBase): return telem_event.payload["lpp"] if telem_event else None - async def req_mma(self, contact, timeout=0, min_timeout=0): - logger.error("*** please consider using req_mma_sync instead of req_mma") - return await self.req_mma_sync(contact, start, end, timeout,min_timeout) - async def req_mma_sync(self, contact, start, end, timeout=0,min_timeout=0): async with self._mesh_request_lock: req = ( diff --git a/src/meshcore/commands/contact.py b/src/meshcore/commands/contact.py index ac723ea..436a066 100644 --- a/src/meshcore/commands/contact.py +++ b/src/meshcore/commands/contact.py @@ -43,13 +43,17 @@ class ContactCommands(CommandHandlerBase): logger.debug("Timeout while getting contacts") for future in pending: # cancel all futures future.cancel() - return None + return Event(EventType.ERROR, {"reason": "timeout waiting for contacts"}) for future in done: event = await future - if event is None or event.type != EventType.NEXT_CONTACT: - for future in pending: - future.cancel() + if event is None: + for f in pending: + f.cancel() + return Event(EventType.ERROR, {"reason": "no event received during contacts retrieval"}) + if event.type != EventType.NEXT_CONTACT: + for f in pending: + f.cancel() return event futures = [] @@ -64,7 +68,7 @@ class ContactCommands(CommandHandlerBase): except asyncio.TimeoutError: logger.debug(f"Timeout receiving contacts") - return None + return Event(EventType.ERROR, {"reason": "asyncio timeout receiving contacts"}) except Exception as e: logger.debug(f"Command error: {e}") return Event(EventType.ERROR, {"error": str(e)}) @@ -116,7 +120,9 @@ class ContactCommands(CommandHandlerBase): path_hash_mode = int(path.split(":")[1]) path = path.split(":")[0].replace(":","") else: # use device one by default - path_hash_mode = contact["out_path_len"] >> 6 # would fallback to previous val + # out_path_len is pre-masked (& 0x3F) in reader.py, so high bits are always 0; + # the actual path_hash_mode is fetched from the device query below. + path_hash_mode = 0 res = await self.send_device_query() if not res is None and res.type != EventType.ERROR: if "path_hash_mode" in res.payload: diff --git a/src/meshcore/commands/messaging.py b/src/meshcore/commands/messaging.py index b266ae0..0c15479 100644 --- a/src/meshcore/commands/messaging.py +++ b/src/meshcore/commands/messaging.py @@ -313,6 +313,8 @@ class MessagingCommands(CommandHandlerBase): elif isinstance (scope, bytes): # scope has been sent directly as byte logger.debug(f"Directly setting scope to {scope}") scope_key = scope + else: + raise TypeError(f"set_flood_scope: unsupported scope type {type(scope).__name__}") logger.debug(f"Setting scope to {scope_key.hex()}") diff --git a/src/meshcore/meshcore.py b/src/meshcore/meshcore.py index bdd0db3..24a1550 100644 --- a/src/meshcore/meshcore.py +++ b/src/meshcore/meshcore.py @@ -1,6 +1,6 @@ import asyncio import logging -from typing import Any, Callable, Coroutine, Dict, Optional, Union +from typing import Any, Callable, Dict, Optional, Union from .events import Event, EventDispatcher, EventType, Subscription from .reader import MessageReader @@ -206,7 +206,7 @@ class MeshCore: def subscribe( self, event_type: Union[EventType, None], - callback: Callable[[Event], Coroutine[Any, Any, None]], + callback: Callable[[Event], Union[None, asyncio.Future]], attribute_filters: Optional[Dict[str, Any]] = None, ) -> Subscription: """ diff --git a/tests/unit/test_standalone_fixes.py b/tests/unit/test_standalone_fixes.py new file mode 100644 index 0000000..9b3be35 --- /dev/null +++ b/tests/unit/test_standalone_fixes.py @@ -0,0 +1,227 @@ +""" +Verification tests for standalone bug fixes and cleanup. +""" + +import pytest +import asyncio +import inspect +from unittest.mock import AsyncMock, MagicMock, patch + +from meshcore.events import Event, EventDispatcher, EventType +from meshcore.commands.base import CommandHandlerBase +from meshcore.commands.binary import BinaryCommandHandler +from meshcore.commands.messaging import MessagingCommands +from meshcore.commands.contact import ContactCommands +from meshcore.meshcore import MeshCore + +pytestmark = pytest.mark.asyncio + + +# ── req_mma removed ────────────────────────────────────────────────────── + +def test_req_mma_removed(): + """The broken req_mma method should no longer exist on BinaryCommandHandler.""" + assert not hasattr(BinaryCommandHandler, "req_mma"), \ + "req_mma should be removed — it had NameError on undefined start/end" + + +def test_req_mma_sync_still_exists(): + """req_mma_sync should still be present and functional.""" + assert hasattr(BinaryCommandHandler, "req_mma_sync"), \ + "req_mma_sync should still exist after removing req_mma" + + +# ── DEFAULT_TIMEOUT bumped ─────────────────────────────────────────────── + +def test_default_timeout_bumped(): + """DEFAULT_TIMEOUT should be 15.0, not the old 5.0.""" + assert CommandHandlerBase.DEFAULT_TIMEOUT == 15.0, \ + f"DEFAULT_TIMEOUT is {CommandHandlerBase.DEFAULT_TIMEOUT}, expected 15.0" + + +def test_instance_default_timeout(): + """Instance default_timeout should inherit the new 15.0 value.""" + handler = CommandHandlerBase() + assert handler.default_timeout == 15.0 + + +def test_custom_timeout_still_works(): + """Passing a custom timeout should still override the default.""" + handler = CommandHandlerBase(default_timeout=30.0) + assert handler.default_timeout == 30.0 + + +# ── set_flood_scope TypeError guard ────────────────────────────────────── + +async def test_set_flood_scope_bad_type_raises(): + """Passing an unsupported type (e.g., int) should raise TypeError.""" + handler = MessagingCommands() + with pytest.raises(TypeError, match="unsupported scope type"): + await handler.set_flood_scope(42) + + +async def test_set_flood_scope_bad_type_bytearray(): + """bytearray is not bytes — should raise TypeError.""" + handler = MessagingCommands() + with pytest.raises(TypeError, match="unsupported scope type"): + await handler.set_flood_scope(bytearray(b"\x00" * 16)) + + +async def test_set_flood_scope_none_still_works(): + """None scope should reach send() without TypeError — verifies the None branch still binds scope_key.""" + handler = MessagingCommands() + handler._sender_func = AsyncMock() + handler.dispatcher = EventDispatcher() + await handler.dispatcher.start() + try: + # Dispatch an OK event so send() resolves + async def _dispatch_ok(): + await asyncio.sleep(0.05) + await handler.dispatcher.dispatch(Event(EventType.OK, {})) + asyncio.ensure_future(_dispatch_ok()) + result = await handler.set_flood_scope(None) + assert result.type == EventType.OK + finally: + handler.dispatcher.running = False + + +async def test_set_flood_scope_str_still_works(): + """String scope should reach send() without TypeError.""" + handler = MessagingCommands() + handler._sender_func = AsyncMock() + handler.dispatcher = EventDispatcher() + await handler.dispatcher.start() + try: + async def _dispatch_ok(): + await asyncio.sleep(0.05) + await handler.dispatcher.dispatch(Event(EventType.OK, {})) + asyncio.ensure_future(_dispatch_ok()) + result = await handler.set_flood_scope("#test") + assert result.type == EventType.OK + finally: + handler.dispatcher.running = False + + +async def test_set_flood_scope_bytes_still_works(): + """Bytes scope should reach send() without TypeError.""" + handler = MessagingCommands() + handler._sender_func = AsyncMock() + handler.dispatcher = EventDispatcher() + await handler.dispatcher.start() + try: + async def _dispatch_ok(): + await asyncio.sleep(0.05) + await handler.dispatcher.dispatch(Event(EventType.OK, {})) + asyncio.ensure_future(_dispatch_ok()) + result = await handler.set_flood_scope(b"\x01" * 16) + assert result.type == EventType.OK + finally: + handler.dispatcher.running = False + + +# ── dead path_hash_mode shift removed ──────────────────────────────────── + +def test_no_shift_in_update_contact(): + """The dead `>> 6` shift on out_path_len should not appear in contact.py.""" + import meshcore.commands.contact as contact_mod + source = inspect.getsource(contact_mod.ContactCommands.update_contact) + assert ">> 6" not in source, \ + "Dead path_hash_mode = out_path_len >> 6 shift should be removed" + + +# ── get_contacts returns Event, never None ─────────────────────────────── + +async def test_get_contacts_timeout_returns_error_event(): + """On timeout (no futures complete), get_contacts should return an Error Event, not None.""" + handler = ContactCommands() + handler._sender_func = AsyncMock() + handler._reader = MagicMock() + handler.dispatcher = MagicMock() + # Make wait_for_event always timeout by never returning + handler.dispatcher.wait_for_event = AsyncMock(side_effect=asyncio.TimeoutError) + + result = await handler.get_contacts(timeout=0.1) + assert result is not None, "get_contacts should never return None" + assert isinstance(result, Event) + assert result.type == EventType.ERROR + + +# ── binary request pre-registration ────────────────────────────────────── + +async def test_placeholder_registered_before_send(): + """A placeholder binary request should be registered before send() is called.""" + from meshcore.packets import BinaryReqType + + handler = CommandHandlerBase() + handler._sender_func = AsyncMock() + + # Track registration calls + mock_reader = MagicMock() + mock_reader.pending_binary_requests = {} + original_register = MagicMock() + + registration_order = [] + send_called = False + + async def mock_send(data): + nonlocal send_called + # At the point send() is called, a placeholder should already exist + registration_order.append(("send", len(mock_reader.pending_binary_requests))) + send_called = True + + handler._sender_func = mock_send + handler._reader = mock_reader + handler.dispatcher = MagicMock() + handler.dispatcher.wait_for_event = AsyncMock( + return_value=Event(EventType.MSG_SENT, {"expected_ack": b"\x01\x02\x03\x04"}) + ) + + # Resolve subscribed events immediately so send() doesn't block. + # Use MSG_SENT with expected_ack because send_binary_req reads that key. + def resolving_subscribe(event_type, cb, attribute_filters=None): + sub = MagicMock() + sub.unsubscribe = MagicMock() + payload = {"expected_ack": b"\x01\x02\x03\x04"} if event_type == EventType.MSG_SENT else {} + asyncio.get_event_loop().call_soon( + cb, Event(event_type, payload) + ) + return sub + handler.dispatcher.subscribe = MagicMock(side_effect=resolving_subscribe) + + # Call send_binary_req + dst = "aa" * 32 # 32-byte hex pubkey + await handler.send_binary_req(dst, BinaryReqType.MMA) + + # Verify register_binary_request was called (at least the placeholder) + assert mock_reader.register_binary_request.call_count >= 1, \ + "register_binary_request should be called at least once for the placeholder" + + +# ── MeshCore.subscribe annotation matches EventDispatcher ──────────────── + +def test_subscribe_annotation_matches_dispatcher(): + """MeshCore.subscribe callback annotation should match EventDispatcher.subscribe.""" + mc_hints = MeshCore.subscribe.__annotations__ + ed_hints = EventDispatcher.subscribe.__annotations__ + + # Both should have 'callback' in their annotations + assert "callback" in mc_hints, "MeshCore.subscribe missing callback annotation" + assert "callback" in ed_hints, "EventDispatcher.subscribe missing callback annotation" + + # The callback annotations should be identical + assert mc_hints["callback"] == ed_hints["callback"], ( + f"MeshCore.subscribe callback annotation {mc_hints['callback']} " + f"does not match EventDispatcher.subscribe {ed_hints['callback']}" + ) + + +def test_no_coroutine_import_in_meshcore(): + """After widening the annotation, Coroutine should no longer be imported in meshcore.py.""" + import meshcore.meshcore as mc_mod + source = inspect.getsource(mc_mod) + # Check the import line specifically — Coroutine should not be in the typing imports + for line in source.splitlines(): + if line.startswith("from typing import"): + assert "Coroutine" not in line, \ + "Coroutine should be removed from typing imports in meshcore.py" + break