Refactor cooldown management in command classes to utilize centralized tracking

- Removed per-command user cooldown tracking from individual command classes.
- Implemented a unified cooldown management system in BaseCommand for both global and per-user cooldowns.
- Updated command execution methods to call the new centralized cooldown handling, improving code clarity and reducing redundancy.
- Enhanced error handling and logging during cooldown checks to ensure robust command execution.

modules/commands/alternatives/wx_international.py

@@ -31,9 +31,6 @@ class GlobalWxCommand(BaseCommand):
         super().__init__(bot)
         self.url_timeout = 10  # seconds
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Get default state and country from config for city disambiguation
         self.default_state = self.bot.config.get('Weather', 'default_state', fallback='WA')
         self.default_country = self.bot.config.get('Weather', 'default_country', fallback='US')
@@ -74,43 +71,6 @@ class GlobalWxCommand(BaseCommand):
                 return True
         return False
     
-    def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
-        return True
-    
-    def get_remaining_cooldown(self, user_id: str) -> int:
-        """Get remaining cooldown time for a specific user"""
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            remaining = self.cooldown_seconds - elapsed
-            return max(0, int(remaining))
-        
-        return 0
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
     
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the weather command"""
@@ -154,7 +114,7 @@ class GlobalWxCommand(BaseCommand):
         
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Get weather data for the location
             weather_data = await self.get_weather_for_location(location, forecast_type, num_days)

modules/commands/aqi_command.py

@@ -34,9 +34,6 @@ class AqiCommand(BaseCommand):
         super().__init__(bot)
         self.url_timeout = 10  # seconds
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Get default state from config for city disambiguation
         self.default_state = self.bot.config.get('Weather', 'default_state', fallback='WA')
         
@@ -91,45 +88,6 @@ class AqiCommand(BaseCommand):
         # Compact explanation of all pollutants - fits within 130 chars
         return "AQI Help: PM2.5=fine particles, PM10=coarse, O3=ozone, NO2=nitrogen dioxide, CO=carbon monoxide, SO2=sulfur dioxide"
     
-    def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
-        # Check if command requires DM and message is not DM
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
-        return True
-    
-    def get_remaining_cooldown(self, user_id: str) -> int:
-        """Get remaining cooldown time for a specific user"""
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            remaining = self.cooldown_seconds - elapsed
-            return max(0, int(remaining))
-        
-        return 0
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
-    
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the AQI command"""
         content = message.content.strip()
@@ -302,7 +260,7 @@ class AqiCommand(BaseCommand):
         
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Get AQI data for the location
             aqi_data = await self.get_aqi_for_location(location, location_type)

modules/commands/base_command.py

@@ -30,6 +30,9 @@ class BaseCommand(ABC):
         self.logger = bot.logger
         self._last_execution_time = 0
         
+        # Per-user cooldown tracking (for commands that need per-user rate limiting)
+        self._user_cooldowns: Dict[str, float] = {}
+        
         # Load allowed channels from config (standardized channel override)
         self.allowed_channels = self._load_allowed_channels()
     
@@ -105,14 +108,26 @@ class BaseCommand(ABC):
         for sec in sections_to_try:
             if self.bot.config.has_section(sec):
                 try:
-                    if value_type == 'bool':
+                    if not self.bot.config.has_option(sec, key):
+                        continue
+                    
+                    raw_value = self.bot.config.get(sec, key)
+                    
+                    # Type conversion
+                    if value_type == 'str':
+                        value = raw_value
+                    elif value_type == 'bool':
                         value = self.bot.config.getboolean(sec, key, fallback=fallback)
                     elif value_type == 'int':
                         value = self.bot.config.getint(sec, key, fallback=fallback)
                     elif value_type == 'float':
                         value = self.bot.config.getfloat(sec, key, fallback=fallback)
+                    elif value_type == 'list':
+                        # Parse comma-separated list
+                        value = [item.strip() for item in raw_value.split(',') if item.strip()]
                     else:
-                        value = self.bot.config.get(sec, key, fallback=fallback)
+                        self.logger.warning(f"Unknown value_type '{value_type}' for {sec}.{key}, returning as string")
+                        value = raw_value
                     
                     # If we got a value (not fallback), return it
                     if value != fallback or self.bot.config.has_option(sec, key):
@@ -121,6 +136,9 @@ class BaseCommand(ABC):
                             self.logger.info(f"Config migration: Using old section '[{old_section}]' for '{key}'. "
                                            f"Please update to '[{new_section}]' in config.ini")
                         return value
+                except (ValueError, TypeError) as e:
+                    self.logger.debug(f"Config conversion error for {sec}.{key}: {e}")
+                    continue
                 except Exception as e:
                     self.logger.debug(f"Error reading config {sec}.{key}: {e}")
                     continue
@@ -220,11 +238,10 @@ class BaseCommand(ABC):
         if self.requires_dm and not message.is_dm:
             return False
         
-        # Check cooldown
+        # Check cooldown (per-user if message has sender_id, otherwise global)
         if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            if (current_time - self._last_execution_time) < self.cooldown_seconds:
+            can_execute, _ = self.check_cooldown(message.sender_id if message.sender_id else None)
+            if not can_execute:
                 return False
         
         # Check admin ACL if this command requires admin access
@@ -305,20 +322,84 @@ class BaseCommand(ABC):
         # Minimum of 130 characters to ensure some functionality
         return max(130, max_length)
     
-    def _record_execution(self):
-        """Record the execution time for cooldown tracking"""
-        import time
-        self._last_execution_time = time.time()
-    
-    def get_remaining_cooldown(self) -> int:
-        """Get remaining cooldown time in seconds"""
+    def check_cooldown(self, user_id: Optional[str] = None) -> Tuple[bool, float]:
+        """
+        Check if user is on cooldown.
+        
+        Args:
+            user_id: User ID to check cooldown for. If None, checks global cooldown.
+        
+        Returns:
+            Tuple of (can_execute: bool, remaining_seconds: float)
+        """
         if self.cooldown_seconds <= 0:
-            return 0
+            return True, 0.0
         
         import time
+        
+        if user_id:
+            # Per-user cooldown
+            last_exec = self._user_cooldowns.get(user_id, 0)
+            elapsed = time.time() - last_exec
+            remaining = self.cooldown_seconds - elapsed
+            
+            if remaining > 0:
+                return False, remaining
+            return True, 0.0
+        else:
+            # Global cooldown (backward compatibility)
+            elapsed = time.time() - self._last_execution_time
+            remaining = self.cooldown_seconds - elapsed
+            
+            if remaining > 0:
+                return False, remaining
+            return True, 0.0
+    
+    def record_execution(self, user_id: Optional[str] = None) -> None:
+        """
+        Record command execution for cooldown tracking.
+        
+        Args:
+            user_id: User ID to record execution for. If None, records global execution.
+        """
+        import time
         current_time = time.time()
-        elapsed = current_time - self._last_execution_time
-        remaining = self.cooldown_seconds - elapsed
+        
+        if user_id:
+            # Per-user cooldown
+            self._user_cooldowns[user_id] = current_time
+            
+            # Clean up old entries periodically to prevent memory growth
+            if len(self._user_cooldowns) > 1000:
+                cutoff = current_time - (self.cooldown_seconds * 2)
+                self._user_cooldowns = {
+                    k: v for k, v in self._user_cooldowns.items() 
+                    if v > cutoff
+                }
+        else:
+            # Global cooldown (backward compatibility)
+            self._last_execution_time = current_time
+    
+    def _record_execution(self, user_id: Optional[str] = None):
+        """
+        Record the execution time for cooldown tracking (backward compatibility).
+        
+        Args:
+            user_id: User ID to record execution for. If None, records global execution.
+        """
+        self.record_execution(user_id)
+    
+    def get_remaining_cooldown(self, user_id: Optional[str] = None) -> int:
+        """
+        Get remaining cooldown time in seconds.
+        
+        Args:
+            user_id: User ID to check cooldown for. If None, checks global cooldown.
+        
+        Returns:
+            Remaining cooldown time in seconds (as integer)
+        """
+        _, remaining = self.check_cooldown(user_id)
         return max(0, int(remaining))
     
     def _load_translated_keywords(self):

modules/commands/catfact_command.py

@@ -19,9 +19,6 @@ class CatfactCommand(BaseCommand):
     category = "hidden"  # Hidden category so it won't appear in help
     cooldown_seconds = 3  # 3 second cooldown per user
     
-    # Per-user cooldown tracking
-    user_cooldowns = {}  # user_id -> last_execution_time
-    
     def __init__(self, bot):
         super().__init__(bot)
         
@@ -108,50 +105,11 @@ class CatfactCommand(BaseCommand):
         # Return empty string so it doesn't appear in help
         return ""
     
-    def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
-        # Check if command requires DM and message is not DM
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
-        return True
-    
-    def get_remaining_cooldown(self, user_id: str) -> int:
-        """Get remaining cooldown time for a specific user"""
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            remaining = self.cooldown_seconds - elapsed
-            return max(0, int(remaining))
-        
-        return 0
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
-    
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the cat fact command"""
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Get cat facts from translations or fallback
             cat_facts = self.get_cat_facts()

modules/commands/dadjoke_command.py

@@ -31,9 +31,6 @@ class DadJokeCommand(BaseCommand):
     def __init__(self, bot):
         super().__init__(bot)
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Load configuration
         self.dadjoke_enabled = bot.config.getboolean('Jokes', 'dadjoke_enabled', fallback=True)
         self.long_jokes = bot.config.getboolean('Jokes', 'long_jokes', fallback=False)
@@ -54,58 +51,22 @@ class DadJokeCommand(BaseCommand):
         return False
     
     def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
-        # Check channel access (standardized channel override)
-        if not self.is_channel_allowed(message):
+        """Override to add custom check (dadjoke_enabled) while using base class cooldown"""
+        # Use base class for channel access, DM requirements, and cooldown
+        if not super().can_execute(message):
             return False
         
         # Check if dadjoke command is enabled
         if not self.dadjoke_enabled:
             return False
         
-        # Check if command requires DM and message is not DM
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
         return True
     
-    def get_remaining_cooldown(self, user_id: str) -> int:
-        """Get remaining cooldown time for a specific user"""
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            if elapsed < self.cooldown_seconds:
-                remaining = self.cooldown_seconds - elapsed
-                return max(0, int(remaining))
-        
-        return 0
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
-    
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the dad joke command"""
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Get dad joke from API with length handling
             joke_data = await self.get_dad_joke_with_length_handling()

modules/commands/joke_command.py

@@ -43,9 +43,6 @@ class JokeCommand(BaseCommand):
     def __init__(self, bot):
         super().__init__(bot)
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Load configuration
         self.joke_enabled = bot.config.getboolean('Jokes', 'joke_enabled', fallback=True)
         self.seasonal_jokes = bot.config.getboolean('Jokes', 'seasonal_jokes', fallback=True)
@@ -76,9 +73,9 @@ class JokeCommand(BaseCommand):
         return False
     
     def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
-        # Check channel access (standardized channel override)
-        if not self.is_channel_allowed(message):
+        """Override to add custom checks (joke_enabled, dark joke) while using base class cooldown"""
+        # Use base class for channel access, DM requirements, and cooldown
+        if not super().can_execute(message):
             return False
         
         # Check if joke command is enabled
@@ -89,38 +86,8 @@ class JokeCommand(BaseCommand):
         if self.is_dark_joke_request(message) and not message.is_dm:
             return False
         
-        # Check if command requires DM and message is not DM
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
         return True
     
-    def get_remaining_cooldown(self, user_id: str) -> int:
-        """Get remaining cooldown time for a specific user"""
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            remaining = self.cooldown_seconds - elapsed
-            return max(0, int(remaining))
-        
-        return 0
-    
     def is_dark_joke_request(self, message: MeshMessage) -> bool:
         """Check if the message is requesting a dark joke"""
         content = message.content.strip()
@@ -135,11 +102,6 @@ class JokeCommand(BaseCommand):
         
         return False
     
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
-    
     def get_seasonal_default(self) -> str:
         """Get the seasonal default category based on current month"""
         if not self.seasonal_jokes:
@@ -182,7 +144,7 @@ class JokeCommand(BaseCommand):
         
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Get joke from API with length handling
             joke_data = await self.get_joke_with_length_handling(category)

modules/commands/solarforecast_command.py

@@ -43,9 +43,6 @@ class SolarforecastCommand(BaseCommand):
         super().__init__(bot)
         self.url_timeout = 15  # seconds
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}
-        
         # Forecast cache: {cache_key: {'data': dict, 'timestamp': float}}
         self.forecast_cache = {}
         
@@ -72,28 +69,6 @@ class SolarforecastCommand(BaseCommand):
         # Fallback: return original if no translation found
         return day_abbr
     
-    def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user"""
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
-        return True
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
-    
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the solar forecast command"""
         content = message.content.strip()
@@ -105,7 +80,8 @@ class SolarforecastCommand(BaseCommand):
             return True
         
         try:
-            self._record_execution(message.sender_id)
+            # Record execution for this user (handles cooldown)
+            self.record_execution(message.sender_id)
             
             # Parse arguments - location might be multiple words (e.g., "Hillcrest Repeater v2")
             # Try to find where location ends and numeric parameters begin

modules/commands/sports_command.py

@@ -799,9 +799,6 @@ class SportsCommand(BaseCommand):
         super().__init__(bot)
         self.url_timeout = 10  # seconds
         
-        # Per-user cooldown tracking
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Initialize TheSportsDB client
         self.thesportsdb_client = TheSportsDBClient(logger=self.logger)
         
@@ -950,25 +947,9 @@ class SportsCommand(BaseCommand):
         if not sports_enabled:
             return False
         
-        # Channel access is now handled by BaseCommand.is_channel_allowed()
-        # Call parent can_execute() which includes channel checking
-        if not super().can_execute(message):
-            return False
-        
-        # Check per-user cooldown (don't set it here, just check)
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id or "unknown"
-            
-            if user_id in self.user_cooldowns:
-                time_since_last = current_time - self.user_cooldowns[user_id]
-                if time_since_last < self.cooldown_seconds:
-                    remaining = self.cooldown_seconds - time_since_last
-                    self.logger.info(f"Sports command cooldown active for user {user_id}, {remaining:.1f}s remaining")
-                    return False
-        
-        return True
+        # Channel access and cooldown are now handled by BaseCommand.can_execute()
+        # Call parent can_execute() which includes channel checking and cooldown
+        return super().can_execute(message)
     
     def get_help_text(self) -> str:
         return self.translate('commands.sports.help')
@@ -976,12 +957,8 @@ class SportsCommand(BaseCommand):
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the sports command"""
         try:
-            # Set cooldown for this user
-            if self.cooldown_seconds > 0:
-                import time
-                current_time = time.time()
-                user_id = message.sender_id or "unknown"
-                self.user_cooldowns[user_id] = current_time
+            # Record execution for this user (handles cooldown)
+            self.record_execution(message.sender_id)
             
             # Parse the command
             content = message.content.strip()

modules/commands/wx_command.py

@@ -45,10 +45,6 @@ class WxCommand(BaseCommand):
     def __init__(self, bot):
         super().__init__(bot)
         
-        # Always initialize user_cooldowns to prevent AttributeError
-        # (even when delegating, some methods might still reference it)
-        self.user_cooldowns = {}  # user_id -> last_execution_time
-        
         # Check weather provider setting - delegate to international command if using Open-Meteo
         weather_provider = bot.config.get('Weather', 'weather_provider', fallback='noaa').lower()
         if weather_provider == 'openmeteo' and WX_INTERNATIONAL_AVAILABLE:
@@ -132,49 +128,20 @@ class WxCommand(BaseCommand):
         return False
     
     def can_execute(self, message: MeshMessage) -> bool:
-        """Override cooldown check to be per-user instead of per-command-instance"""
+        """Override to delegate or use base class cooldown"""
         if self.delegate_command:
             return self.delegate_command.can_execute(message)
         
-        # Check if command requires DM and message is not DM
-        if self.requires_dm and not message.is_dm:
-            return False
-        
-        # Check per-user cooldown
-        if self.cooldown_seconds > 0:
-            import time
-            current_time = time.time()
-            user_id = message.sender_id
-            
-            if user_id in self.user_cooldowns:
-                last_execution = self.user_cooldowns[user_id]
-                if (current_time - last_execution) < self.cooldown_seconds:
-                    return False
-        
-        return True
+        # Use base class for cooldown and other checks
+        return super().can_execute(message)
     
     def get_remaining_cooldown(self, user_id: str) -> int:
         """Get remaining cooldown time for a specific user"""
         if self.delegate_command:
             return self.delegate_command.get_remaining_cooldown(user_id)
         
-        if self.cooldown_seconds <= 0:
-            return 0
-        
-        import time
-        current_time = time.time()
-        if user_id in self.user_cooldowns:
-            last_execution = self.user_cooldowns[user_id]
-            elapsed = current_time - last_execution
-            remaining = self.cooldown_seconds - elapsed
-            return max(0, int(remaining))
-        
-        return 0
-    
-    def _record_execution(self, user_id: str):
-        """Record the execution time for a specific user"""
-        import time
-        self.user_cooldowns[user_id] = time.time()
+        # Use base class method
+        return super().get_remaining_cooldown(user_id)
     
     async def execute(self, message: MeshMessage) -> bool:
         """Execute the weather command"""
@@ -242,7 +209,7 @@ class WxCommand(BaseCommand):
         
         try:
             # Record execution for this user
-            self._record_execution(message.sender_id)
+            self.record_execution(message.sender_id)
             
             # Special handling for "alerts" command
             if show_full_alerts: