From 5ae67bc07124e4e96a3910811e481ea70940e82c Mon Sep 17 00:00:00 2001 From: agessaman Date: Thu, 1 Jan 2026 12:03:54 -0800 Subject: [PATCH] Refactor command handling and enhance plugin validation - Removed redundant command list in config example for clarity. - Improved plugin validation by adding checks for required attributes and types, ensuring better error handling during plugin instantiation. - Enhanced rate limiter classes to track total sends and throttled attempts, providing statistics for better monitoring. - Updated command implementations to include metadata for better organization and clarity in command handling. --- config.ini.example | 4 +- modules/commands/cmd_command.py | 119 ++++++++++++++++++++++++++++- modules/commands/hfcond_command.py | 7 +- modules/commands/sun_command.py | 7 +- modules/plugin_loader.py | 85 +++++++++++++++------ modules/rate_limiter.py | 79 +++++++++++++++++-- 6 files changed, 261 insertions(+), 40 deletions(-) diff --git a/config.ini.example b/config.ini.example index f6ed5e1..8ad5916 100644 --- a/config.ini.example +++ b/config.ini.example @@ -233,7 +233,9 @@ test = "ack @[{sender}]{phrase_part} | {connection_info} | Received at: {timesta ping = "Pong!" pong = "Ping!" help = "Bot Help: test (or t), ping, help, hello, cmd, advert, @string, wx, aqi, sun, moon, solar, hfcond, satpass, prefix, path, sports, dice, roll, stats | More: 'help '" -cmd = "Available commands: test (or t), ping, help, hello, cmd, advert, @string, wx, aqi, sun, moon, solar, hfcond, satpass, prefix, path, sports, dice, roll, stats" + +Override 'cmd' command output +# cmd = "Available commands: test (or t), ping, help, hello, cmd, advert, @string, wx, aqi, sun, moon, solar, hfcond, satpass, prefix, path, sports, dice, roll, stats" [Channels] # Channels to monitor (comma-separated) diff --git a/modules/commands/cmd_command.py b/modules/commands/cmd_command.py index f480ea9..174818c 100644 --- a/modules/commands/cmd_command.py +++ b/modules/commands/cmd_command.py @@ -11,12 +11,123 @@ from ..models import MeshMessage class CmdCommand(BaseCommand): """Handles the cmd command""" + # Plugin metadata + name = "cmd" + keywords = ['cmd', 'commands'] + description = "Lists available commands in compact format" + category = "basic" + def get_help_text(self) -> str: return "Lists commands in compact format." + def _get_commands_list(self, max_length: int = None) -> str: + """Get a compact list of available commands, prioritizing important ones + + Args: + max_length: Maximum length for the command list (None = no limit) + + Returns: + Comma-separated list of commands, truncated if necessary + """ + # Define priority order - most important/commonly used commands first + priority_commands = [ + 'test', 'ping', 'help', 'hello', 'cmd', 'advert', + 'wx', 'aqi', 'sun', 'moon', 'solar', 'hfcond', 'satpass', + 'prefix', 'path', 'sports', 'dice', 'roll', 'stats' + ] + + # Get all command names + all_commands = [] + + # Include plugin commands + for cmd_name, cmd_instance in self.bot.command_manager.commands.items(): + # Skip system commands without keywords (like greeter) + if hasattr(cmd_instance, 'keywords') and cmd_instance.keywords: + all_commands.append(cmd_name) + + # Include config keywords that aren't handled by plugins + for keyword in self.bot.command_manager.keywords.keys(): + # Check if this keyword is already handled by a plugin + is_plugin_keyword = any( + keyword.lower() in [k.lower() for k in cmd.keywords] + for cmd in self.bot.command_manager.commands.values() + ) + if not is_plugin_keyword: + all_commands.append(keyword) + + # Remove duplicates and sort + all_commands = sorted(set(all_commands)) + + # Prioritize: put priority commands first, then others + prioritized = [] + remaining = [] + + for cmd in all_commands: + if cmd in priority_commands: + prioritized.append(cmd) + else: + remaining.append(cmd) + + # Sort priority commands by their order in priority_commands list + prioritized = sorted(prioritized, key=lambda x: priority_commands.index(x) if x in priority_commands else 999) + + # Combine: priority first, then others + command_names = prioritized + sorted(remaining) + + # Build the list, respecting max_length if provided + if max_length is None: + return ', '.join(command_names) + + # Build list within length limit + result = [] + prefix = "Available commands: " + current_length = len(prefix) + + for cmd in command_names: + # Calculate length if we add this command: ", cmd" or "cmd" (first one) + if result: + test_length = current_length + len(', ') + len(cmd) + else: + test_length = current_length + len(cmd) + + if test_length <= max_length: + if result: + result.append(cmd) + current_length += len(', ') + len(cmd) + else: + result.append(cmd) + current_length += len(cmd) + else: + # Can't fit this command, add count of remaining + remaining_count = len(command_names) - len(result) + if remaining_count > 0: + suffix = f" (+{remaining_count} more)" + if current_length + len(suffix) <= max_length: + result.append(suffix.replace('+', '').replace('more', f'{remaining_count} more')) + break + + return prefix + ', '.join(result) + async def execute(self, message: MeshMessage) -> bool: """Execute the cmd command""" - # The cmd command is handled by keyword matching in the command manager - # This is just a placeholder for future functionality - self.logger.debug("Cmd command executed (handled by keyword matching)") - return True + try: + # Check if user has defined a custom cmd keyword response in config + # Use the already-loaded keywords dict (quotes are already stripped) + cmd_keyword = self.bot.command_manager.keywords.get('cmd') + if cmd_keyword: + # User has defined a custom response, use it with formatting + response = self.bot.command_manager.format_keyword_response(cmd_keyword, message) + return await self.send_response(message, response) + + # Fallback to dynamic command list if no custom keyword is defined + # Get max message length to ensure we fit within limits + max_length = self.get_max_message_length(message) + # Reserve space for "Available commands: " prefix + available_length = max_length - len("Available commands: ") + commands_list = self._get_commands_list(max_length=available_length) + response = f"Available commands: {commands_list}" + return await self.send_response(message, response) + except Exception as e: + self.logger.error(f"Error executing cmd command: {e}") + error_msg = self.translate('errors.execution_error', command='cmd', error=str(e)) + return await self.send_response(message, error_msg) diff --git a/modules/commands/hfcond_command.py b/modules/commands/hfcond_command.py index 408d8cd..6f5b3b0 100644 --- a/modules/commands/hfcond_command.py +++ b/modules/commands/hfcond_command.py @@ -11,9 +11,14 @@ from ..models import MeshMessage class HfcondCommand(BaseCommand): """Command to get HF band conditions""" + # Plugin metadata + name = "hfcond" + keywords = ['hfcond'] + description = "Get HF band conditions for ham radio" + category = "solar" + def __init__(self, bot): super().__init__(bot) - self.keywords = ['hfcond'] async def execute(self, message: MeshMessage) -> bool: """Execute the hfcond command""" diff --git a/modules/commands/sun_command.py b/modules/commands/sun_command.py index 880073f..3140b0d 100644 --- a/modules/commands/sun_command.py +++ b/modules/commands/sun_command.py @@ -11,9 +11,14 @@ from ..models import MeshMessage class SunCommand(BaseCommand): """Command to get sun information""" + # Plugin metadata + name = "sun" + keywords = ['sun'] + description = "Get sunrise/sunset times" + category = "solar" + def __init__(self, bot): super().__init__(bot) - self.keywords = ['sun'] async def execute(self, message: MeshMessage) -> bool: """Execute the sun command""" diff --git a/modules/plugin_loader.py b/modules/plugin_loader.py index 4b23d15..700a2a5 100644 --- a/modules/plugin_loader.py +++ b/modules/plugin_loader.py @@ -87,7 +87,9 @@ class PluginLoader: def _validate_plugin(self, plugin_class: Type[BaseCommand]) -> List[str]: """ - Validate a plugin class has required attributes. + Validate a plugin class has required attributes before instantiation. + All plugins should define name and keywords as class attributes (the standard convention). + We validate leniently here and re-check after instantiation to catch any edge cases. Args: plugin_class: The plugin class to validate @@ -97,28 +99,51 @@ class PluginLoader: """ errors = [] - required_attrs = ['name', 'keywords', 'description', 'execute'] - for attr in required_attrs: - if not hasattr(plugin_class, attr): - errors.append(f"Missing required attribute: {attr}") - - if hasattr(plugin_class, 'name'): - # Check if name is a class attribute (not just instance attribute) - name_value = getattr(plugin_class, 'name', None) - if not name_value: - errors.append("Plugin 'name' attribute is empty") - - if hasattr(plugin_class, 'keywords'): - keywords = getattr(plugin_class, 'keywords', None) - if not isinstance(keywords, list): - errors.append("Plugin 'keywords' must be a list") - elif not keywords: - errors.append("Plugin 'keywords' list is empty") - - if hasattr(plugin_class, 'execute'): + # Check for execute method (must exist as class method) + if not hasattr(plugin_class, 'execute'): + errors.append(f"Missing required attribute: execute") + elif hasattr(plugin_class, 'execute'): if not inspect.iscoroutinefunction(plugin_class.execute): errors.append("Plugin 'execute' method must be async") + # Check for keywords type if it exists (allow empty list for system commands) + if hasattr(plugin_class, 'keywords'): + keywords = getattr(plugin_class, 'keywords', None) + if keywords is not None and not isinstance(keywords, list): + errors.append("Plugin 'keywords' must be a list") + # Don't error on empty keywords - some commands (like greeter) intentionally have none + + # Note: We don't check name/keywords presence here since they may be set in __init__ + # or derived from class name. Post-instantiation validation will catch missing attributes. + + return errors + + def _validate_plugin_instance(self, plugin_instance: BaseCommand, plugin_name: str) -> List[str]: + """ + Validate a plugin instance after instantiation. + Ensures required attributes are present and correctly typed. + Note: Name may be derived from class name if not explicitly set. + + Args: + plugin_instance: The instantiated plugin + plugin_name: The file name of the plugin (for error messages) + + Returns: + List of validation errors (empty if valid) + """ + errors = [] + + # Check name - must be set (either as class attribute, in __init__, or derived from class name) + if not hasattr(plugin_instance, 'name') or not plugin_instance.name: + errors.append("Plugin 'name' attribute is empty or not set") + + # Check keywords - must be a list (can be empty for system commands like greeter) + if not hasattr(plugin_instance, 'keywords'): + errors.append("Plugin 'keywords' attribute is missing") + elif not isinstance(plugin_instance.keywords, list): + errors.append("Plugin 'keywords' must be a list") + # Allow empty keywords - some system commands intentionally have none + return errors def load_plugin(self, plugin_name: str, from_alternatives: bool = False) -> Optional[BaseCommand]: @@ -157,7 +182,7 @@ class PluginLoader: self._failed_plugins[plugin_name] = error_msg return None - # Validate plugin class before instantiation + # Validate plugin class before instantiation (basic checks) validation_errors = self._validate_plugin(command_class) if validation_errors: error_msg = f"Plugin validation failed: {', '.join(validation_errors)}" @@ -168,12 +193,22 @@ class PluginLoader: # Instantiate the command plugin_instance = command_class(self.bot) + # Set name from class name if not set (before validation) + if not hasattr(plugin_instance, 'name') or not plugin_instance.name: + # Use the class name as the plugin name if not specified + derived_name = command_class.__name__.lower().replace('command', '') + plugin_instance.name = derived_name + + # Validate plugin instance after instantiation (catches attributes set in __init__) + instance_validation_errors = self._validate_plugin_instance(plugin_instance, plugin_name) + if instance_validation_errors: + error_msg = f"Plugin instance validation failed: {', '.join(instance_validation_errors)}" + self.logger.error(f"Failed to load plugin '{plugin_name}': {error_msg}") + self._failed_plugins[plugin_name] = error_msg + return None + # Validate plugin metadata metadata = plugin_instance.get_metadata() - if not metadata.get('name'): - # Use the class name as the plugin name if not specified - metadata['name'] = command_class.__name__.lower().replace('command', '') - plugin_instance.name = metadata['name'] source = "alternatives" if from_alternatives else "default" self.logger.info(f"Successfully loaded plugin: {metadata['name']} from {plugin_name} ({source})") diff --git a/modules/rate_limiter.py b/modules/rate_limiter.py index f48d98a..b25c028 100644 --- a/modules/rate_limiter.py +++ b/modules/rate_limiter.py @@ -5,6 +5,8 @@ Controls how often messages can be sent to prevent spam """ import time +import asyncio +from typing import Optional class RateLimiter: @@ -13,10 +15,15 @@ class RateLimiter: def __init__(self, seconds: int): self.seconds = seconds self.last_send = 0 + self._total_sends = 0 + self._total_throttled = 0 def can_send(self) -> bool: """Check if we can send a message""" - return time.time() - self.last_send >= self.seconds + can = time.time() - self.last_send >= self.seconds + if not can: + self._total_throttled += 1 + return can def time_until_next(self) -> float: """Get time until next allowed send""" @@ -26,6 +33,17 @@ class RateLimiter: def record_send(self): """Record that we sent a message""" self.last_send = time.time() + self._total_sends += 1 + + def get_stats(self) -> dict: + """Get rate limiter statistics""" + total_attempts = self._total_sends + self._total_throttled + throttle_rate = self._total_throttled / max(1, total_attempts) + return { + 'total_sends': self._total_sends, + 'total_throttled': self._total_throttled, + 'throttle_rate': throttle_rate + } class BotTxRateLimiter: @@ -34,10 +52,15 @@ class BotTxRateLimiter: def __init__(self, seconds: float = 1.0): self.seconds = seconds self.last_tx = 0 + self._total_tx = 0 + self._total_throttled = 0 def can_tx(self) -> bool: """Check if bot can transmit a message""" - return time.time() - self.last_tx >= self.seconds + can = time.time() - self.last_tx >= self.seconds + if not can: + self._total_throttled += 1 + return can def time_until_next_tx(self) -> float: """Get time until next allowed transmission""" @@ -47,14 +70,24 @@ class BotTxRateLimiter: def record_tx(self): """Record that bot transmitted a message""" self.last_tx = time.time() + self._total_tx += 1 async def wait_for_tx(self): """Wait until bot can transmit (async)""" - import asyncio while not self.can_tx(): wait_time = self.time_until_next_tx() if wait_time > 0: await asyncio.sleep(wait_time + 0.05) # Small buffer + + def get_stats(self) -> dict: + """Get rate limiter statistics""" + total_attempts = self._total_tx + self._total_throttled + throttle_rate = self._total_throttled / max(1, total_attempts) + return { + 'total_tx': self._total_tx, + 'total_throttled': self._total_throttled, + 'throttle_rate': throttle_rate + } class NominatimRateLimiter: @@ -67,11 +100,22 @@ class NominatimRateLimiter: def __init__(self, seconds: float = 1.1): self.seconds = seconds self.last_request = 0 - self._lock = None # Will be set to asyncio.Lock if needed + self._lock: Optional[asyncio.Lock] = None + self._total_requests = 0 + self._total_throttled = 0 + + def _get_lock(self) -> asyncio.Lock: + """Lazily initialize the async lock""" + if self._lock is None: + self._lock = asyncio.Lock() + return self._lock def can_request(self) -> bool: """Check if we can make a Nominatim request""" - return time.time() - self.last_request >= self.seconds + can = time.time() - self.last_request >= self.seconds + if not can: + self._total_throttled += 1 + return can def time_until_next(self) -> float: """Get time until next allowed request""" @@ -81,19 +125,38 @@ class NominatimRateLimiter: def record_request(self): """Record that we made a Nominatim request""" self.last_request = time.time() + self._total_requests += 1 async def wait_for_request(self): """Wait until we can make a Nominatim request (async)""" - import asyncio while not self.can_request(): wait_time = self.time_until_next() if wait_time > 0: await asyncio.sleep(wait_time + 0.05) # Small buffer + async def wait_and_request(self) -> None: + """Wait until a request can be made, then mark request time (thread-safe)""" + async with self._get_lock(): + current_time = time.time() + time_since_last = current_time - self.last_request + if time_since_last < self.seconds: + await asyncio.sleep(self.seconds - time_since_last) + self.last_request = time.time() + self._total_requests += 1 + def wait_for_request_sync(self): """Wait until we can make a Nominatim request (synchronous)""" - import time as time_module while not self.can_request(): wait_time = self.time_until_next() if wait_time > 0: - time_module.sleep(wait_time + 0.05) # Small buffer + time.sleep(wait_time + 0.05) # Small buffer + + def get_stats(self) -> dict: + """Get rate limiter statistics""" + total_attempts = self._total_requests + self._total_throttled + throttle_rate = self._total_throttled / max(1, total_attempts) + return { + 'total_requests': self._total_requests, + 'total_throttled': self._total_throttled, + 'throttle_rate': throttle_rate + }