Refactor to avoid the use of a special global object.

The global object formerly used is now replaced by direct use
of the namespace opf the globals module. This eliminates the
redundant getters and setters and simplifies the code for
future maintainers.

Note that the globals module name conflicts (harmlessly at
present) with a Python built-in function. A future commit
should rename it `config` to remove this clash and better
represent its intended purpose.

Author: holdenweb

meshtastic/__main__.py

@@ -16,16 +16,14 @@
 
 import meshtastic.test
 import meshtastic.util
+from meshtastic import globals
 from meshtastic import channel_pb2, config_pb2, portnums_pb2, remote_hardware, BROADCAST_ADDR
 from meshtastic.version import get_active_version
 from meshtastic.ble_interface import BLEInterface
-from meshtastic.globals import Globals
-
 
 def onReceive(packet, interface):
     """Callback invoked when a packet arrives"""
-    our_globals = Globals.getInstance()
-    args = our_globals.get_args()
+    args = globals.args
     try:
         d = packet.get("decoded")
         logging.debug(f"in onReceive() d:{d}")
@@ -69,7 +67,7 @@ def getPref(node, comp_name):
     # Note: protobufs has the keys in snake_case, so snake internally
     snake_name = meshtastic.util.camel_to_snake(name[1])
     logging.debug(f"snake_name:{snake_name} camel_name:{camel_name}")
-    logging.debug(f"use camel:{Globals.getInstance().get_camel_case()}")
+    logging.debug(f"use camel:{globals.camel_case}")
 
     # First validate the input
     localConfig = node.localConfig
@@ -86,7 +84,7 @@ def getPref(node, comp_name):
                 break
 
     if not found:
-        if Globals.getInstance().get_camel_case():
+        if globals.camel_case:
             print(
                 f"{localConfig.__class__.__name__} and {moduleConfig.__class__.__name__} do not have an attribute {snake_name}."
             )
@@ -105,7 +103,7 @@ def getPref(node, comp_name):
         config_values = getattr(config, config_type.name)
         if not wholeField:
             pref_value = getattr(config_values, pref.name)
-            if Globals.getInstance().get_camel_case():
+            if globals.camel_case:
                 print(f"{str(config_type.name)}.{camel_name}: {str(pref_value)}")
                 logging.debug(
                     f"{str(config_type.name)}.{camel_name}: {str(pref_value)}"
@@ -171,7 +169,7 @@ def setPref(config, comp_name, valStr) -> bool:
         if e:
             val = e.number
         else:
-            if Globals.getInstance().get_camel_case():
+            if globals.camel_case:
                 print(
                     f"{name[0]}.{camel_name} does not have an enum called {val}, so you can not set it."
                 )
@@ -210,7 +208,7 @@ def setPref(config, comp_name, valStr) -> bool:
             config_type.message_type.ignore_incoming.extend([val])
 
     prefix = f"{name[0]}." if config_type.message_type is not None else ""
-    if Globals.getInstance().get_camel_case():
+    if globals.camel_case:
         print(f"Set {prefix}{camel_name} to {valStr}")
     else:
         print(f"Set {prefix}{snake_name} to {valStr}")
@@ -225,8 +223,7 @@ def onConnected(interface):
         False  # Should we wait for an acknowledgment if we send to a remote node?
     )
     try:
-        our_globals = Globals.getInstance()
-        args = our_globals.get_args()
+        args = globals.args
 
         # do not print this line if we are exporting the config
         if not args.export_config:
@@ -477,7 +474,7 @@ def onConnected(interface):
                 print("Writing modified preferences to device")
                 node.writeConfig(field)
             else:
-                if Globals.getInstance().get_camel_case():
+                if globals.camel_case:
                     print(
                         f"{node.localConfig.__class__.__name__} and {node.moduleConfig.__class__.__name__} do not have an attribute {pref[0]}."
                     )
@@ -590,7 +587,7 @@ def onConnected(interface):
         # handle changing channels
 
         if args.ch_add:
-            channelIndex = our_globals.get_channel_index()
+            channelIndex = globals.channel_index
             if channelIndex is not None:
                 # Since we set the channel index after adding a channel, don't allow --ch-index
                 meshtastic.util.our_exit(
@@ -621,12 +618,12 @@ def onConnected(interface):
                 n.writeChannel(ch.index)
                 if channelIndex is None:
                     print(f"Setting newly-added channel's {ch.index} as '--ch-index' for further modifications")
-                    our_globals.set_channel_index(ch.index)
+                    globals.channel_index = ch.index
 
         if args.ch_del:
             closeNow = True
 
-            channelIndex = our_globals.get_channel_index()
+            channelIndex = globals.channel_index
             if channelIndex is None:
                 meshtastic.util.our_exit(
                     "Warning: Need to specify '--ch-index' for '--ch-del'.", 1
@@ -642,7 +639,7 @@ def onConnected(interface):
 
         def setSimpleConfig(modem_preset):
             """Set one of the simple modem_config"""
-            channelIndex = our_globals.get_channel_index()
+            channelIndex = globals.channel_index
             if channelIndex is not None and channelIndex > 0:
                 meshtastic.util.our_exit(
                     "Warning: Cannot set modem preset for non-primary channel", 1
@@ -677,7 +674,7 @@ def setSimpleConfig(modem_preset):
         if args.ch_set or args.ch_enable or args.ch_disable:
             closeNow = True
 
-            channelIndex = our_globals.get_channel_index()
+            channelIndex = globals.channel_index
             if channelIndex is None:
                 meshtastic.util.our_exit("Warning: Need to specify '--ch-index'.", 1)
             ch = interface.getNode(args.dest).channels[channelIndex]
@@ -832,7 +829,7 @@ def printConfig(config):
             names = []
             for field in config.message_type.fields:
                 tmp_name = f"{config_section.name}.{field.name}"
-                if Globals.getInstance().get_camel_case():
+                if globals.camel_case:
                     tmp_name = meshtastic.util.snake_to_camel(tmp_name)
                 names.append(tmp_name)
             for temp_name in sorted(names):
@@ -877,7 +874,7 @@ def export_config(interface):
     if owner_short:
         configObj["owner_short"] = owner_short
     if channel_url:
-        if Globals.getInstance().get_camel_case():
+        if globals.camel_case:
             configObj["channelUrl"] = channel_url
         else:
             configObj["channel_url"] = channel_url
@@ -889,11 +886,11 @@ def export_config(interface):
         # Convert inner keys to correct snake/camelCase
         prefs = {}
         for pref in config:
-            if Globals.getInstance().get_camel_case():
+            if globals.camel_case:
                 prefs[meshtastic.util.snake_to_camel(pref)] = config[pref]
             else:
                 prefs[pref] = config[pref]
-        if Globals.getInstance().get_camel_case():
+        if globals.camel_case:
             configObj["config"] = config
         else:
             configObj["config"] = config
@@ -905,7 +902,7 @@ def export_config(interface):
         for pref in module_config:
             if len(module_config[pref]) > 0:
                 prefs[pref] = module_config[pref]
-        if Globals.getInstance().get_camel_case():
+        if globals.camel_case:
             configObj["module_config"] = prefs
         else:
             configObj["module_config"] = prefs
@@ -919,9 +916,8 @@ def export_config(interface):
 def common():
     """Shared code for all of our command line wrappers"""
     logfile = None
-    our_globals = Globals.getInstance()
-    args = our_globals.get_args()
-    parser = our_globals.get_parser()
+    args = globals.args
+    parser = globals.parser
     logging.basicConfig(
         level=logging.DEBUG if (args.debug or args.listen) else logging.INFO,
         format="%(levelname)s file:%(filename)s %(funcName)s line:%(lineno)s %(message)s",
@@ -937,7 +933,7 @@ def common():
 
         if args.ch_index is not None:
             channelIndex = int(args.ch_index)
-            our_globals.set_channel_index(channelIndex)
+            globals.channel_index = channelIndex
 
         if not args.dest:
             args.dest = BROADCAST_ADDR
@@ -972,7 +968,7 @@ def common():
                 # Note: using "line buffering"
                 # pylint: disable=R1732
                 logfile = open(args.seriallog, "w+", buffering=1, encoding="utf8")
-                our_globals.set_logfile(logfile)
+                globals.logfile = logfile
 
             subscribe()
             if args.ble_scan:
@@ -1063,9 +1059,8 @@ def addConnectionArgs(parser: argparse.ArgumentParser) -> argparse.ArgumentParse
 
 def initParser():
     """Initialize the command line argument parsing."""
-    our_globals = Globals.getInstance()
-    parser = our_globals.get_parser()
-    args = our_globals.get_args()
+    parser = globals.parser
+    args = globals.args
 
     # The "Help" group includes the help option and other informational stuff about the CLI itself
     outerHelpGroup = parser.add_argument_group('Help')
@@ -1286,7 +1281,7 @@ def initParser():
     )
 
     group.add_argument(
-        "--request-telemetry", 
+        "--request-telemetry",
         help="Request telemetry from a node. "
         "You need pass the destination ID as argument with '--dest'. "
         "For repeaters, the nodeNum is required.",
@@ -1431,34 +1426,32 @@ def initParser():
 
 
     args = parser.parse_args()
-    our_globals.set_args(args)
-    our_globals.set_parser(parser)
+    globals.args = args
+    globals.parser = parser
 
 
 def main():
     """Perform command line meshtastic operations"""
-    our_globals = Globals.getInstance()
     parser = argparse.ArgumentParser(
         add_help=False,
         epilog="If no connection arguments are specified, we search for a compatible serial device, "
                "and if none is found, then attempt a TCP connection to localhost.")
-    our_globals.set_parser(parser)
+    globals.parser = parser
     initParser()
     common()
-    logfile = our_globals.get_logfile()
+    logfile = globals.logfile
     if logfile:
         logfile.close()
 
 
 def tunnelMain():
     """Run a meshtastic IP tunnel"""
-    our_globals = Globals.getInstance()
     parser = argparse.ArgumentParser(add_help=False)
-    our_globals.set_parser(parser)
+    globals.parser = parser
     initParser()
-    args = our_globals.get_args()
+    args = globals.args
     args.tunnel = True
-    our_globals.set_args(args)
+    globals.args = args
     common()
 
 

meshtastic/globals.py

@@ -1,96 +1,28 @@
-"""Globals singleton class.
-
-   Instead of using a global, stuff your variables in this "trash can".
-   This is not much better than using python's globals, but it allows
-   us to better test meshtastic. Plus, there are some weird python
-   global issues/gotcha that we can hopefully avoid by using this
-   class instead.
-
 """
+Globals singleton class.
 
+The Global object is gone, as are all its setters and getters. Instead the
+module itself is the singleton namespace, which can be imported into
+whichever module is used. The associated tests have also been removed,
+since we now rely on built in Python mechanisms.
 
-class Globals:
-    """Globals class is a Singleton."""
-
-    __instance = None
-
-    @staticmethod
-    def getInstance():
-        """Get an instance of the Globals class."""
-        if Globals.__instance is None:
-            Globals()
-        return Globals.__instance
-
-    def __init__(self):
-        """Constructor for the Globals CLass"""
-        if Globals.__instance is not None:
-            raise Exception("This class is a singleton") # pylint: disable=W0719
-        else:
-            Globals.__instance = self
-        self.args = None
-        self.parser = None
-        self.channel_index = None
-        self.logfile = None
-        self.tunnelInstance = None
-        # TODO: to migrate to camel_case for v1.3 change this value to True
-        self.camel_case = False
-
-    def reset(self):
-        """Reset all of our globals. If you add a member, add it to this method, too."""
-        self.args = None
-        self.parser = None
-        self.channel_index = None
-        self.logfile = None
-        self.tunnelInstance = None
-        # TODO: to migrate to camel_case for v1.3 change this value to True
-        self.camel_case = False
-
-    # setters
-    def set_args(self, args):
-        """Set the args"""
-        self.args = args
-
-    def set_parser(self, parser):
-        """Set the parser"""
-        self.parser = parser
-
-    def set_channel_index(self, channel_index):
-        """Set the channel_index"""
-        self.channel_index = channel_index
+This is intended to make the Python read more naturally, and to make the
+intention of the code clearer and more compact. It is merely a sticking
+plaster over the use of shared globals, but the coupling issues wil be dealt
+with rather more easily once the code is simplified by this change.
 
-    def set_logfile(self, logfile):
-        """Set the logfile"""
-        self.logfile = logfile
-
-    def set_tunnelInstance(self, tunnelInstance):
-        """Set the tunnelInstance"""
-        self.tunnelInstance = tunnelInstance
-
-    def set_camel_case(self):
-        """Force using camelCase for things like prefs/set/set"""
-        self.camel_case = True
-
-    # getters
-    def get_args(self):
-        """Get args"""
-        return self.args
-
-    def get_parser(self):
-        """Get parser"""
-        return self.parser
-
-    def get_channel_index(self):
-        """Get channel_index"""
-        return self.channel_index
-
-    def get_logfile(self):
-        """Get logfile"""
-        return self.logfile
-
-    def get_tunnelInstance(self):
-        """Get tunnelInstance"""
-        return self.tunnelInstance
+"""
 
-    def get_camel_case(self):
-        """Get whether or not to use camelCase"""
-        return self.camel_case
+def reset():
+    """
+    """
+    global args, parser, channel_index, logfile, tunnelInstance, camel_case
+    args = None
+    parser = None
+    channel_index = None
+    logfile = None
+    tunnelInstance = None
+    # TODO: to migrate to camel_case for v1.3 change this value to True
+    camel_case = False
+
+reset()

meshtastic/tests/conftest.py

@@ -5,7 +5,7 @@
 
 import pytest
 
-from meshtastic.__main__ import Globals
+from meshtastic import globals
 
 from ..mesh_interface import MeshInterface
 
@@ -15,8 +15,8 @@ def reset_globals():
     """Fixture to reset globals."""
     parser = None
     parser = argparse.ArgumentParser(add_help=False)
-    Globals.getInstance().reset()
-    Globals.getInstance().set_parser(parser)
+    globals.reset()
+    globals.parser = parser
 
 
 @pytest.fixture