Refactor configuration handling for joke commands

- Standardized the configuration keys for joke commands by replacing `joke_enabled` and `dadjoke_enabled` with a unified `enabled` key in the configuration files.
- Updated the validation logic to warn users when both legacy `[Jokes]` and new `[Joke_Command]`/`[DadJoke_Command]` sections are present, suggesting the removal of the legacy section.
- Enhanced the command classes to support fallback mechanisms for legacy configuration keys, ensuring backward compatibility.
- Added tests to verify the correct behavior of the new configuration handling and legacy support.

config.ini.example

@@ -232,7 +232,7 @@ companion_min_inactive_days = 30
 # Enable or disable the joke command
 # true: Joke command is available (default)
 # false: Joke command is disabled
-joke_enabled = true
+enabled = true
 
 # Enable seasonal joke defaults (October: spooky, December: Christmas)
 # true: Seasonal defaults are applied (default)
@@ -245,13 +245,13 @@ seasonal_jokes = true
 long_jokes = false
 
 # Channels where joke command is allowed (omit to use global monitor_channels)
-# channels = general,#bot,#jokes
+# channels = #bot,#jokes
 
 [DadJoke_Command]
 # Enable or disable the dad joke command
 # true: Dad joke command is available (default)
 # false: Dad joke command is disabled
-dadjoke_enabled = true
+enabled = true
 
 # Handle long jokes (over 130 characters)
 # false: Fetch new jokes until we get a short one (default)
@@ -259,7 +259,7 @@ dadjoke_enabled = true
 long_jokes = false
 
 # Channels where dadjoke command is allowed (omit to use global monitor_channels)
-# channels = general,#bot,#jokes
+# channels = #bot,#jokes
 
 [Keywords]
 # Available placeholders (message-based):
@@ -549,95 +549,6 @@ precipitation_unit = inch
 # Note: Only one 'default' source can be set per provider type.
 # Future providers (e.g., custom.pwaweather.location) will follow the same pattern.
 
-# Command stubs (enable/channels only; omit to use global monitor_channels)
-[Wx_Command]
-enabled = true
-# channels =
-
-[WebViewer_Command]
-enabled = true
-# channels =
-
-[Sun_Command]
-enabled = true
-# channels =
-
-[Solar_Command]
-enabled = true
-# channels =
-
-[Solarforecast_Command]
-enabled = true
-# channels =
-
-[Satpass_Command]
-enabled = true
-# channels =
-
-[Roll_Command]
-enabled = true
-# channels =
-
-[Repeater_Command]
-enabled = true
-# channels =
-
-[Ping_Command]
-enabled = true
-# channels =
-
-[Moon_Command]
-enabled = true
-# channels =
-
-[Magic8_Command]
-enabled = true
-# channels =
-
-[Hfcond_Command]
-enabled = true
-# channels =
-
-[Help_Command]
-enabled = true
-# channels =
-
-[Hello_Command]
-enabled = true
-# channels =
-
-[Dice_Command]
-enabled = true
-# channels =
-
-[Cmd_Command]
-enabled = true
-# channels =
-
-[Channels_Command]
-enabled = true
-# channels =
-
-[Catfact_Command]
-enabled = true
-# channels =
-
-[Aqi_Command]
-enabled = true
-# channels =
-
-[Advert_Command]
-enabled = true
-# channels =
-
-[Test_Command]
-enabled = true
-# channels =
-
-[Feed_Command]
-enabled = true
-# channels =
-
 [Solar_Config]
 # URL timeout for external API calls (seconds)
 url_timeout = 10
@@ -1098,6 +1009,104 @@ max_results = 10
 # API request timeout in seconds
 url_timeout = 10
 
+# Command stubs (enable/channels only; omit to use global monitor_channels)
+[Wx_Command]
+enabled = true
+# channels =
+
+[WebViewer_Command]
+enabled = true
+# channels =
+
+[Sun_Command]
+enabled = true
+# channels =
+
+[Solar_Command]
+enabled = true
+# channels =
+
+[Solarforecast_Command]
+enabled = true
+# channels =
+
+[Satpass_Command]
+enabled = true
+# channels =
+
+[Roll_Command]
+enabled = true
+# channels =
+
+[Repeater_Command]
+enabled = true
+# channels =
+
+[Ping_Command]
+enabled = true
+# channels =
+
+[Moon_Command]
+enabled = true
+# channels =
+
+[Magic8_Command]
+enabled = true
+# channels =
+
+[Hfcond_Command]
+enabled = true
+# channels =
+
+[Help_Command]
+enabled = true
+# channels =
+
+[Hello_Command]
+enabled = true
+# channels =
+
+[Dice_Command]
+enabled = true
+# channels =
+
+[Cmd_Command]
+enabled = true
+# channels =
+
+[Channels_Command]
+enabled = true
+# channels =
+
+[Catfact_Command]
+enabled = true
+# channels =
+
+[Aqi_Command]
+enabled = true
+# channels =
+
+[Advert_Command]
+enabled = true
+# channels =
+
+[Test_Command]
+enabled = true
+# channels =
+
+[Feed_Command]
+enabled = false
+# channels =
+
+####################################################################################################
+#                                                                                                  #
+#                                       Web Viewer Configuration                                   #
+#                                                                                                  #
+#                             Settings for web administration backend                              #
+#                                                                                                  #
+####################################################################################################
+
+
 [Web_Viewer]
 # Enable or disable the web data viewer
 # SECURITY NOTE: Web viewer has NO AUTHENTICATION built-in

config.ini.minimal-example

@@ -373,10 +373,11 @@ enabled = false
 # Enable or disable the cmd command
 enabled = false
 
-[Jokes]
-# Enable or disable joke commands
-joke_enabled = false
-dadjoke_enabled = false
+[Joke_Command]
+enabled = false
+
+[DadJoke_Command]
+enabled = false
 
 [Sports_Command]
 # Enable or disable the sports command

modules/commands/base_command.py

@@ -105,6 +105,11 @@ class BaseCommand(ABC):
             'Weather': 'Wx_Command',  # wx command reads from [Wx_Command]; [Weather] is legacy
         }
         # Legacy [Jokes] -> [Joke_Command] / [DadJoke_Command]: (requested_section, key) -> legacy section
+        # For 'enabled', also try legacy key in [Jokes]: (section, key) -> (legacy_section, legacy_key)
+        legacy_key_alias = {
+            ('Joke_Command', 'enabled'): ('Jokes', 'joke_enabled'),
+            ('DadJoke_Command', 'enabled'): ('Jokes', 'dadjoke_enabled'),
+        }
         legacy_section_fallback = {
             ('Joke_Command', 'joke_enabled'): 'Jokes',
             ('Joke_Command', 'seasonal_jokes'): 'Jokes',
@@ -170,6 +175,27 @@ class BaseCommand(ABC):
                     self.logger.debug(f"Error reading config {sec}.{key}: {e}")
                     continue
         
+        # Try legacy key alias (e.g. [Jokes] joke_enabled when requesting Joke_Command enabled)
+        alias = legacy_key_alias.get((section, key))
+        if alias:
+            legacy_sec, legacy_key = alias
+            if self.bot.config.has_section(legacy_sec) and self.bot.config.has_option(legacy_sec, legacy_key):
+                try:
+                    if value_type == 'bool':
+                        value = self.bot.config.getboolean(legacy_sec, legacy_key, fallback=fallback)
+                    elif value_type == 'int':
+                        value = self.bot.config.getint(legacy_sec, legacy_key, fallback=fallback)
+                    elif value_type == 'float':
+                        value = self.bot.config.getfloat(legacy_sec, legacy_key, fallback=fallback)
+                    elif value_type == 'list':
+                        raw = self.bot.config.get(legacy_sec, legacy_key)
+                        value = [item.strip() for item in raw.split(',') if item.strip()]
+                    else:
+                        value = self.bot.config.get(legacy_sec, legacy_key)
+                    return value
+                except (ValueError, TypeError) as e:
+                    self.logger.debug(f"Config conversion error for {legacy_sec}.{legacy_key}: {e}")
+        
         return fallback
     
     @abstractmethod

modules/commands/dadjoke_command.py

@@ -41,8 +41,10 @@ class DadJokeCommand(BaseCommand):
         """
         super().__init__(bot)
         
-        # Load configuration (DadJoke_Command; legacy [Jokes] supported via get_config_value)
-        self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'dadjoke_enabled', fallback=True, value_type='bool')
+        # Load configuration (enabled standard; dadjoke_enabled legacy from [DadJoke_Command] or [Jokes])
+        self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'enabled', fallback=None, value_type='bool')
+        if self.dadjoke_enabled is None:
+            self.dadjoke_enabled = self.get_config_value('DadJoke_Command', 'dadjoke_enabled', fallback=True, value_type='bool')
         self.long_jokes = self.get_config_value('DadJoke_Command', 'long_jokes', fallback=False, value_type='bool')
     
     def get_help_text(self) -> str:

modules/commands/joke_command.py

@@ -56,8 +56,10 @@ class JokeCommand(BaseCommand):
         """
         super().__init__(bot)
         
-        # Load configuration (Joke_Command; legacy [Jokes] supported via get_config_value)
-        self.joke_enabled = self.get_config_value('Joke_Command', 'joke_enabled', fallback=True, value_type='bool')
+        # Load configuration (enabled standard; joke_enabled legacy from [Joke_Command] or [Jokes])
+        self.joke_enabled = self.get_config_value('Joke_Command', 'enabled', fallback=None, value_type='bool')
+        if self.joke_enabled is None:
+            self.joke_enabled = self.get_config_value('Joke_Command', 'joke_enabled', fallback=True, value_type='bool')
         self.seasonal_jokes = self.get_config_value('Joke_Command', 'seasonal_jokes', fallback=True, value_type='bool')
         self.long_jokes = self.get_config_value('Joke_Command', 'long_jokes', fallback=False, value_type='bool')
     

modules/config_validation.py

@@ -213,10 +213,20 @@ def validate_config(config_path: str) -> List[Tuple[str, str]]:
         # Check typo map for known non-standard names
         if section_stripped in SECTION_TYPO_MAP:
             suggestion = SECTION_TYPO_MAP[section_stripped]
-            results.append((
-                SEVERITY_WARNING,
-                f"Non-standard section [{section_stripped}]; did you mean [{suggestion}]?",
-            ))
+            # Special case: [Jokes] + [Joke_Command]/[DadJoke_Command] overlap
+            if section_stripped == "Jokes" and (
+                "Joke_Command" in sections_present or "DadJoke_Command" in sections_present
+            ):
+                results.append((
+                    SEVERITY_WARNING,
+                    "Both [Jokes] and [Joke_Command]/[DadJoke_Command] are present; "
+                    "the *_Command sections take precedence. Consider removing [Jokes] to avoid confusion.",
+                ))
+            else:
+                results.append((
+                    SEVERITY_WARNING,
+                    f"Non-standard section [{section_stripped}]; did you mean [{suggestion}]?",
+                ))
         else:
             # Check if section looks like a command name (e.g. [Stats] -> [Stats_Command])
             if prefix_to_section is None:

tests/commands/test_base_command.py

@@ -6,6 +6,7 @@ from unittest.mock import MagicMock
 from modules.commands.base_command import BaseCommand
 from modules.commands.ping_command import PingCommand
 from modules.commands.dadjoke_command import DadJokeCommand
+from modules.commands.joke_command import JokeCommand
 from modules.models import MeshMessage
 from tests.conftest import mock_message
 
@@ -88,6 +89,41 @@ class TestGetConfigValue:
         val = cmd.get_config_value("Hacker_Command", "enabled", fallback=False, value_type="bool")
         assert val is True
 
+    def test_joke_command_enabled_standard(self, command_mock_bot):
+        """Joke_Command uses enabled (standard) when present."""
+        command_mock_bot.config.add_section("Joke_Command")
+        command_mock_bot.config.set("Joke_Command", "enabled", "false")
+        cmd = JokeCommand(command_mock_bot)
+        assert cmd.joke_enabled is False
+
+    def test_joke_command_joke_enabled_legacy(self, command_mock_bot):
+        """Joke_Command falls back to joke_enabled when enabled absent."""
+        command_mock_bot.config.add_section("Joke_Command")
+        command_mock_bot.config.set("Joke_Command", "joke_enabled", "false")
+        cmd = JokeCommand(command_mock_bot)
+        assert cmd.joke_enabled is False
+
+    def test_joke_command_legacy_jokes_section(self, command_mock_bot):
+        """Joke_Command reads joke_enabled from legacy [Jokes] when enabled absent."""
+        command_mock_bot.config.add_section("Jokes")
+        command_mock_bot.config.set("Jokes", "joke_enabled", "false")
+        cmd = JokeCommand(command_mock_bot)
+        assert cmd.joke_enabled is False
+
+    def test_dadjoke_command_enabled_standard(self, command_mock_bot):
+        """DadJoke_Command uses enabled (standard) when present."""
+        command_mock_bot.config.add_section("DadJoke_Command")
+        command_mock_bot.config.set("DadJoke_Command", "enabled", "false")
+        cmd = DadJokeCommand(command_mock_bot)
+        assert cmd.dadjoke_enabled is False
+
+    def test_dadjoke_command_dadjoke_enabled_legacy(self, command_mock_bot):
+        """DadJoke_Command falls back to dadjoke_enabled when enabled absent."""
+        command_mock_bot.config.add_section("DadJoke_Command")
+        command_mock_bot.config.set("DadJoke_Command", "dadjoke_enabled", "false")
+        cmd = DadJokeCommand(command_mock_bot)
+        assert cmd.dadjoke_enabled is False
+
 
 class TestCanExecute:
     """Tests for can_execute()."""

tests/test_config_validation.py

@@ -119,6 +119,37 @@ enabled = true
         infos = [r for r in results if r[0] == SEVERITY_INFO]
         assert any("Stats" in r[1] and "Stats_Command" in r[1] for r in infos)
 
+    def test_jokes_overlap_suggests_removal(self, tmp_path):
+        """When both [Jokes] and [Joke_Command]/[DadJoke_Command] exist, suggest removing [Jokes]."""
+        config = tmp_path / "config.ini"
+        config.write_text("""[Connection]
+connection_type = serial
+serial_port = /dev/ttyUSB0
+
+[Bot]
+bot_name = TestBot
+db_path = {db_path}
+
+[Channels]
+monitor_channels = general
+respond_to_dms = true
+
+[Jokes]
+joke_enabled = true
+
+[Joke_Command]
+enabled = true
+
+[Keywords]
+test = ack
+""".format(db_path=str(tmp_path / "meshcore_bot.db")))
+        results = validate_config(str(config))
+        warnings = [r for r in results if r[0] == SEVERITY_WARNING]
+        assert any(
+            "Both [Jokes]" in r[1] and "Consider removing [Jokes]" in r[1]
+            for r in warnings
+        )
+
 
 class TestPathValidation:
     """Tests for path writability validation."""