cleanups, fixes & docs in Bus.__new__() and util.load_config() (hardbyte#309)

felixdivo · web-flow · commit 6ebe61064752 · 2018-05-28T07:46:04.000+02:00
diff --git a/can/interface.py b/can/interface.py
@@ -7,7 +7,7 @@
 CyclicSendTasks.
 """
 
-from __future__ import absolute_import
+from __future__ import absolute_import, print_function
 
 import sys
 import importlib
@@ -98,36 +98,37 @@ class Bus(BusABC):
     configuration file from default locations.
     """
 
-    @classmethod
-    def __new__(cls, other, channel=None, *args, **kwargs):
+    @staticmethod
+    def __new__(cls, *args, **config):
         """
-        Takes the same arguments as :class:`can.BusABC` with the addition of:
+        Takes the same arguments as :class:`can.BusABC.__init__` with the addition of:
 
-        :param kwargs:
-            Should contain a bustype key with a valid interface name.
+        :param dict config:
+            Should contain an ``interface`` key with a valid interface name. If not,
+            it is completed using :meth:`can.util.load_config`.
 
-        :raises:
-            NotImplementedError if the bustype isn't recognized
-        :raises:
-            ValueError if the bustype or channel isn't either passed as an argument
-            or set in the can.rc config.
+        :raises: NotImplementedError
+            if the ``interface`` isn't recognized
 
+        :raises: ValueError
+            if the ``channel`` could not be determined
         """
 
-        # Figure out the configuration
-        config = load_config(config={
-            'interface': kwargs.get('bustype', kwargs.get('interface')),
-            'channel': channel
-        })
-
-        # remove the bustype & interface so it doesn't get passed to the backend
-        if 'bustype' in kwargs:
-            del kwargs['bustype']
-        if 'interface' in kwargs:
-            del kwargs['interface']
+        # figure out the rest of the configuration; this might raise an error
+        config = load_config(config=config)
 
+        # resolve the bus class to use for that interface
         cls = _get_class_for_interface(config['interface'])
-        return cls(channel=config['channel'], *args, **kwargs)
+
+        # remove the 'interface' key so it doesn't get passed to the backend
+        del config['interface']
+
+        # make sure the bus can handle this config
+        if 'channel' not in config:
+            raise ValueError("channel argument missing")
+
+        # the channel attribute should be present in **config
+        return cls(*args, **config)
 
 
 def detect_available_configs(interfaces=None):
diff --git a/can/util.py b/can/util.py
@@ -121,57 +121,79 @@ def load_config(path=None, config=None):
         If you pass ``"socketcan"`` this automatically selects between the
         native and ctypes version.
 
+    .. note::
+ 
+            The key ``bustype`` is copied to ``interface`` if that one is missing
+            and does never appear in the result.
+
     :param path:
         Optional path to config file.
+
     :param config:
         A dict which may set the 'interface', and/or the 'channel', or neither.
+        It may set other values that are passed through.
 
     :return:
         A config dictionary that should contain 'interface' & 'channel'::
 
             {
                 'interface': 'python-can backend interface to use',
                 'channel': 'default channel to use',
+                # possibly more
             }
 
         Note ``None`` will be used if all the options are exhausted without
         finding a value.
+
+        All unused values are passed from ``config`` over to this.
+
+    :raises:
+        NotImplementedError if the ``interface`` isn't recognized
     """
-    if config is None:
-        config = {}
 
-    system_config = {}
-    configs = [
-        config,
+    # start with an empty dict to apply filtering to all sources
+    given_config = config
+    config = {}
+
+    # use the given dict for default values
+    config_sources = [
+        given_config,
         can.rc,
         load_environment_config,
         lambda: load_file_config(path)
     ]
 
     # Slightly complex here to only search for the file config if required
-    for cfg in configs:
+    for cfg in config_sources:
         if callable(cfg):
             cfg = cfg()
+        # remove legacy operator (and copy to interface if not already present)
+        if 'bustype' in cfg:
+            if 'interface' not in cfg or not cfg['interface']:
+                cfg['interface'] = cfg['bustype']
+            del cfg['bustype']
+        # copy all new parameters
         for key in cfg:
-            if key not in system_config and cfg[key] is not None:
-                system_config[key] = cfg[key]
+            if key not in config:
+                config[key] = cfg[key]
 
     # substitute None for all values not found
     for key in REQUIRED_KEYS:
-        if key not in system_config:
-            system_config[key] = None
+        if key not in config:
+            config[key] = None
 
-    if system_config['interface'] == 'socketcan':
-        system_config['interface'] = choose_socketcan_implementation()
+    # this is done later too but better safe than sorry
+    if config['interface'] == 'socketcan':
+        config['interface'] = choose_socketcan_implementation()
 
-    if system_config['interface'] not in VALID_INTERFACES:
-        raise NotImplementedError('Invalid CAN Bus Type - {}'.format(system_config['interface']))
+    if config['interface'] not in VALID_INTERFACES:
+        raise NotImplementedError('Invalid CAN Bus Type - {}'.format(config['interface']))
 
-    if 'bitrate' in system_config:
-        system_config['bitrate'] = int(system_config['bitrate'])
+    if 'bitrate' in config:
+        config['bitrate'] = int(config['bitrate'])
 
-    can.log.debug("can config: {}".format(system_config))
-    return system_config
+    can.log.debug("loaded can config: {}".format(config))
+    return config
 
 
 def choose_socketcan_implementation():
