mirror of
https://github.com/agessaman/meshcore-bot.git
synced 2026-06-05 15:21:24 +00:00
fix: reduce channel message budget by 10 bytes for regional flood scope
Match firmware TC_FLOOD scope overhead in get_max_message_length via MeshMessage.effective_outgoing_flood_scope; keep CommandManager and BaseCommand in sync with tests. Made-with: Cursor
This commit is contained in:
@@ -20,7 +20,7 @@ from .config_validation import (
|
||||
_channel_name_is_public,
|
||||
strip_optional_quotes,
|
||||
)
|
||||
from .models import MeshMessage
|
||||
from .models import CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD, MeshMessage
|
||||
from .plugin_loader import PluginLoader
|
||||
from .utils import check_internet_connectivity_async, decode_escape_sequences, format_keyword_response_with_placeholders
|
||||
|
||||
@@ -576,6 +576,9 @@ class CommandManager:
|
||||
def get_max_message_length(self, message: MeshMessage) -> int:
|
||||
"""Return max message body size in UTF-8 bytes (DM=158, channel per firmware budget).
|
||||
|
||||
Regional (non-global) flood scope reduces the channel body budget by
|
||||
``CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD`` bytes.
|
||||
|
||||
Mirrors ``BaseCommand.get_max_message_length`` but works on the manager level so it
|
||||
can be called outside of a specific command instance.
|
||||
"""
|
||||
@@ -594,8 +597,10 @@ class CommandManager:
|
||||
pass
|
||||
if not username:
|
||||
username = self.bot.config.get('Bot', 'bot_name', fallback='Bot')
|
||||
max_length = 160 - len(str(username).encode('utf-8')) - 2
|
||||
return max(130, max_length)
|
||||
max_length = max(130, 160 - len(str(username).encode('utf-8')) - 2)
|
||||
if not MeshMessage.is_global_flood_scope(message.effective_outgoing_flood_scope(self.bot)):
|
||||
max_length -= CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD
|
||||
return max_length
|
||||
|
||||
def check_keywords(self, message: MeshMessage) -> list[tuple]:
|
||||
"""Check message content for keywords and return matching responses.
|
||||
|
||||
@@ -9,7 +9,7 @@ from abc import ABC, abstractmethod
|
||||
from datetime import datetime
|
||||
from typing import Any, Optional
|
||||
|
||||
from ..models import MeshMessage
|
||||
from ..models import CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD, MeshMessage
|
||||
from ..security_utils import validate_pubkey_format
|
||||
from ..utils import format_elapsed_display, get_config_timezone
|
||||
|
||||
@@ -549,6 +549,7 @@ class BaseCommand(ABC):
|
||||
|
||||
Channel messages are formatted as "<username>: <message>", so the body budget is:
|
||||
160 - utf8_byte_len(username) - 2 (for ": "), matching firmware cipher block limits.
|
||||
Regional (non-global) flood scope subtracts CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD bytes.
|
||||
|
||||
DM (contact) messages have no username prefix; max safe payload is 158 bytes.
|
||||
|
||||
@@ -584,11 +585,10 @@ class BaseCommand(ABC):
|
||||
|
||||
# 160 bytes are available for channel messages
|
||||
# Calculate max length: 160 - username_length - 2 (for ": ")
|
||||
max_length = 160 - len(str(username).encode('utf-8')) - 2
|
||||
|
||||
# Ensure we don't return a negative or unreasonably small value
|
||||
# Minimum of 130 bytes to ensure some functionality
|
||||
return max(130, max_length)
|
||||
max_length = max(130, 160 - len(str(username).encode('utf-8')) - 2)
|
||||
if not MeshMessage.is_global_flood_scope(message.effective_outgoing_flood_scope(self.bot)):
|
||||
max_length -= CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD
|
||||
return max_length
|
||||
|
||||
def check_cooldown(self, user_id: Optional[str] = None) -> tuple[bool, float]:
|
||||
"""Check if user is on cooldown.
|
||||
|
||||
@@ -7,6 +7,9 @@ Contains shared data structures used across modules
|
||||
from dataclasses import dataclass
|
||||
from typing import Any, Optional
|
||||
|
||||
# Firmware reserves extra bytes for regional (non-global) TC_FLOOD scope on channel text.
|
||||
CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD = 10
|
||||
|
||||
|
||||
@dataclass
|
||||
class MeshMessage:
|
||||
@@ -26,3 +29,25 @@ class MeshMessage:
|
||||
routing_info: Optional[dict[str, Any]] = None
|
||||
# Matched flood scope for the reply (e.g. "#west"), None means global flood
|
||||
reply_scope: Optional[str] = None
|
||||
|
||||
def effective_outgoing_flood_scope(self, bot: Any) -> str:
|
||||
"""Resolve outbound flood scope the same way as ``CommandManager.send_channel_message``.
|
||||
|
||||
For channel replies: ``reply_scope`` when set, else ``[Channels] outgoing_flood_scope_override``.
|
||||
Empty string means global flood. DMs return ``""`` (not applicable).
|
||||
"""
|
||||
if self.is_dm:
|
||||
return ""
|
||||
if self.reply_scope is not None:
|
||||
return (self.reply_scope or "").strip()
|
||||
scope_cfg = ""
|
||||
if bot.config.has_section("Channels") and bot.config.has_option(
|
||||
"Channels", "outgoing_flood_scope_override"
|
||||
):
|
||||
scope_cfg = (bot.config.get("Channels", "outgoing_flood_scope_override") or "").strip()
|
||||
return scope_cfg
|
||||
|
||||
@staticmethod
|
||||
def is_global_flood_scope(scope: str) -> bool:
|
||||
"""Match ``send_channel_message`` global markers (before ``_normalize_scope_name``)."""
|
||||
return scope in ("", "*", "0", "None")
|
||||
|
||||
@@ -10,7 +10,7 @@ from modules.commands.joke_command import JokeCommand
|
||||
from modules.commands.ping_command import PingCommand
|
||||
from modules.commands.sports_command import SportsCommand
|
||||
from modules.commands.stats_command import StatsCommand
|
||||
from modules.models import MeshMessage
|
||||
from modules.models import CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD, MeshMessage
|
||||
from tests.conftest import mock_message
|
||||
|
||||
|
||||
@@ -225,6 +225,13 @@ class TestGetMaxMessageLength:
|
||||
msg = mock_message(content="x", channel="general", is_dm=False)
|
||||
assert cmd.get_max_message_length(msg) == 130 # max(130, 160 - 40 - 2)
|
||||
|
||||
def test_channel_regional_reply_scope_reduces_budget_by_10_bytes(self, command_mock_bot):
|
||||
command_mock_bot.meshcore = None
|
||||
command_mock_bot.config.set("Bot", "bot_name", "LongBotName")
|
||||
cmd = _TestCommand(command_mock_bot)
|
||||
msg = mock_message(content="x", channel="general", is_dm=False, reply_scope="#west")
|
||||
assert cmd.get_max_message_length(msg) == 147 - CHANNEL_REGIONAL_FLOOD_SCOPE_BODY_OVERHEAD
|
||||
|
||||
|
||||
class TestCanExecute:
|
||||
"""Tests for can_execute()."""
|
||||
|
||||
@@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, MagicMock, Mock, patch
|
||||
import pytest
|
||||
|
||||
from modules.command_manager import CommandManager, InternetStatusCache
|
||||
from modules.models import MeshMessage
|
||||
from tests.conftest import mock_message
|
||||
|
||||
|
||||
@@ -728,39 +729,52 @@ class TestGetMaxMessageLength:
|
||||
|
||||
def test_dm_returns_158_bytes(self):
|
||||
mgr = self._make_manager()
|
||||
msg = Mock()
|
||||
msg.is_dm = True
|
||||
msg = MeshMessage(content="x", is_dm=True)
|
||||
assert mgr.get_max_message_length(msg) == 158
|
||||
|
||||
def test_channel_uses_bot_name_utf8_bytes(self):
|
||||
mgr = self._make_manager(bot_name="LongBotName")
|
||||
msg = Mock()
|
||||
msg.is_dm = False
|
||||
msg = MeshMessage(content="x", channel="general", is_dm=False)
|
||||
# 160 - utf8("LongBotName") - 2 = 160 - 11 - 2 = 147
|
||||
assert mgr.get_max_message_length(msg) == 147
|
||||
|
||||
def test_channel_uses_meshcore_username_utf8_bytes(self):
|
||||
mgr = self._make_manager(bot_name="fallback", username="Radio")
|
||||
msg = Mock()
|
||||
msg.is_dm = False
|
||||
msg = MeshMessage(content="x", channel="general", is_dm=False)
|
||||
# 160 - utf8("Radio") - 2 = 160 - 5 - 2 = 153
|
||||
assert mgr.get_max_message_length(msg) == 153
|
||||
|
||||
def test_channel_regional_reply_scope_reduces_budget_by_10_bytes(self):
|
||||
mgr = self._make_manager(bot_name="LongBotName")
|
||||
msg = MeshMessage(content="x", channel="general", is_dm=False, reply_scope="#west")
|
||||
assert mgr.get_max_message_length(msg) == 137 # 147 - 10
|
||||
|
||||
def test_channel_outgoing_flood_scope_override_reduces_budget_by_10_bytes(self):
|
||||
mgr = self._make_manager(bot_name="LongBotName")
|
||||
mgr.bot.config.set("Channels", "outgoing_flood_scope_override", "#west")
|
||||
msg = MeshMessage(content="x", channel="general", is_dm=False)
|
||||
assert mgr.get_max_message_length(msg) == 137
|
||||
|
||||
def test_parity_with_base_command_get_max_message_length(self):
|
||||
"""CommandManager must mirror BaseCommand byte budgets (PR #128)."""
|
||||
from tests.commands.test_base_command import _TestCommand
|
||||
|
||||
cases: list[tuple[str, str | None, bool]] = [
|
||||
("LongBotName", None, False),
|
||||
("Bot", None, True),
|
||||
("fallback", "Radio", False),
|
||||
("x", "😀😀", False),
|
||||
cases: list[tuple[str, str | None, bool, str | None]] = [
|
||||
("LongBotName", None, False, None),
|
||||
("Bot", None, True, None),
|
||||
("fallback", "Radio", False, None),
|
||||
("x", "😀😀", False, None),
|
||||
("LongBotName", None, False, "#west"),
|
||||
]
|
||||
for bot_name, username, is_dm in cases:
|
||||
for bot_name, username, is_dm, reply_scope in cases:
|
||||
mgr = self._make_manager(bot_name=bot_name, username=username)
|
||||
cmd = _TestCommand(mgr.bot)
|
||||
msg = Mock()
|
||||
msg.is_dm = is_dm
|
||||
msg = MeshMessage(
|
||||
content="x",
|
||||
channel=None if is_dm else "general",
|
||||
is_dm=is_dm,
|
||||
reply_scope=reply_scope,
|
||||
)
|
||||
m_len = mgr.get_max_message_length(msg)
|
||||
b_len = cmd.get_max_message_length(msg)
|
||||
assert m_len == b_len, (bot_name, username, is_dm, m_len, b_len)
|
||||
assert m_len == b_len, (bot_name, username, is_dm, reply_scope, m_len, b_len)
|
||||
|
||||
Reference in New Issue
Block a user