Choose correct socketcan interface. Closes hardbyte#120

* Improve handling of configuration.
* If interface is set to socketcan in can.rc['interface'] choose which implementation to use.
* Extract helper function for checking the can.rc global config.
* Slightly rearrange where to find set of valid interfaces.

Author: hardbyte

can/__init__.py

@@ -4,7 +4,7 @@
 import logging
 log = logging.getLogger('can')
 
-rc = dict(channel=0)
+rc = dict()
 
 
 class CanError(IOError):
@@ -24,4 +24,5 @@ class CanError(IOError):
 from can.bus import BusABC
 from can.notifier import Notifier
 from can.broadcastmanager import send_periodic, CyclicSendTaskABC, MultiRateCyclicSendTaskABC
-from can.interfaces import interface
+from can.interfaces import VALID_INTERFACES
+from . import interface

can/interfaces/interface.py renamed to can/interface.py

@@ -1,10 +1,6 @@
 import can
 from can.broadcastmanager import CyclicSendTaskABC, MultiRateCyclicSendTaskABC
-from can.util import load_config, choose_socketcan_implementation
-
-VALID_INTERFACES = set(['kvaser', 'serial', 'pcan', 'socketcan_native',
-                        'socketcan_ctypes', 'socketcan', 'usb2can', 'ixxat',
-                        'nican', 'remote', 'virtual', 'neovi'])
+from can.util import load_config
 
 
 class Bus(object):
@@ -21,30 +17,25 @@ def __new__(cls, other, channel=None, *args, **kwargs):
         :param kwargs:
             Should contain a bustype key with a valid interface name.
 
-        :raises: NotImplementedError if the bustype isn't recognized
+        :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.
+
         """
+        config = load_config(config={
+            'interface': kwargs.get('bustype'),
+            'channel': channel
+        })
+
         if 'bustype' in kwargs:
-            can.rc['interface'] = kwargs['bustype']
+            # remove the bustype so it doesn't get passed to the backend
             del kwargs['bustype']
-
-            if can.rc['interface'] == 'socketcan':
-                can.rc['interface'] = choose_socketcan_implementation()
-
-        # Update can.rc from kwargs
-        for kw in ('interface', 'bitrate'):
-            if kw in kwargs:
-                can.rc[kw] = kwargs[kw]
-
-        if 'interface' not in can.rc or 'channel' not in can.rc or can.rc['interface'] is None:
-            can.log.debug("Loading default configuration")
-            # Load defaults
-            can.rc = load_config()
-
-        if can.rc['interface'] not in VALID_INTERFACES:
-            raise NotImplementedError('Invalid CAN Bus Type - {}'.format(can.rc['interface']))
+        interface = config['interface']
+        channel = config['channel']
 
         # Import the correct Bus backend
-        interface = can.rc['interface']
         if interface == 'kvaser':
             from can.interfaces.kvaser import KvaserBus
             cls = KvaserBus
@@ -81,8 +72,6 @@ def __new__(cls, other, channel=None, *args, **kwargs):
         else:
             raise NotImplementedError("CAN interface '{}' not supported".format(interface))
 
-        if channel is None:
-            channel = can.rc['channel']
         return cls(channel, **kwargs)
 
 
@@ -91,20 +80,13 @@ class CyclicSendTask(CyclicSendTaskABC):
     @classmethod
     def __new__(cls, other, channel, *args, **kwargs):
 
-        # If can.rc doesn't look valid: load default
-        if 'interface' not in can.rc or 'channel' not in can.rc:
-            can.log.debug("Loading default configuration")
-            can.rc = load_config()
-
-        print(can.rc)
-        if can.rc['interface'] not in VALID_INTERFACES:
-            raise NotImplementedError('Invalid CAN Bus Type - {}'.format(can.rc['interface']))
+        config = load_config(config={'channel': channel})
 
         # Import the correct implementation of CyclicSendTask
-        if can.rc['interface'] == 'socketcan_ctypes':
+        if config['interface'] == 'socketcan_ctypes':
             from can.interfaces.socketcan.socketcan_ctypes import CyclicSendTask as _ctypesCyclicSendTask
             cls = _ctypesCyclicSendTask
-        elif can.rc['interface'] == 'socketcan_native':
+        elif config['interface'] == 'socketcan_native':
             from can.interfaces.socketcan.socketcan_native import CyclicSendTask as _nativeCyclicSendTask
             cls = _nativeCyclicSendTask
         # CyclicSendTask has not been fully implemented on remote interface yet.
@@ -114,33 +96,26 @@ def __new__(cls, other, channel, *args, **kwargs):
         #    from can.interfaces.remote import CyclicSendTask as _remoteCyclicSendTask
         #    cls = _remoteCyclicSendTask
         else:
-            can.log.info("Current CAN interface doesn't support CyclicSendTask")
+            raise can.CanError("Current CAN interface doesn't support CyclicSendTask")
 
-        return cls(channel, *args, **kwargs)
+        return cls(config['channel'], *args, **kwargs)
 
 
 class MultiRateCyclicSendTask(MultiRateCyclicSendTaskABC):
 
     @classmethod
     def __new__(cls, other, channel, *args, **kwargs):
 
-        # If can.rc doesn't look valid: load default
-        if 'interface' not in can.rc or 'channel' not in can.rc:
-            can.log.debug("Loading default configuration")
-            can.rc = load_config()
-
-        print(can.rc)
-        if can.rc['interface'] not in VALID_INTERFACES:
-            raise NotImplementedError('Invalid CAN Bus Type - {}'.format(can.rc['interface']))
+        config = load_config(config={'channel': channel})
 
         # Import the correct implementation of CyclicSendTask
-        if can.rc['interface'] == 'socketcan_ctypes':
+        if config['interface'] == 'socketcan_ctypes':
             from can.interfaces.socketcan.socketcan_ctypes import MultiRateCyclicSendTask as _ctypesMultiRateCyclicSendTask
             cls = _ctypesMultiRateCyclicSendTask
-        elif can.rc['interface'] == 'socketcan_native':
+        elif config['interface'] == 'socketcan_native':
             from can.interfaces.socketcan.socketcan_native import MultiRateCyclicSendTask as _nativeMultiRateCyclicSendTask
             cls = _nativeMultiRateCyclicSendTask
         else:
             can.log.info("Current CAN interface doesn't support CyclicSendTask")
 
-        return cls(channel, *args, **kwargs)
+        return cls(config['channel'], *args, **kwargs)

can/interfaces/__init__.py

@@ -2,3 +2,7 @@
 """
 Interfaces contain low level implementations that interact with CAN hardware.
 """
+
+VALID_INTERFACES = set(['kvaser', 'serial', 'pcan', 'socketcan_native',
+                        'socketcan_ctypes', 'socketcan', 'usb2can', 'ixxat',
+                        'nican', 'remote', 'virtual', 'neovi'])

can/interfaces/remote/__main__.py

@@ -1,10 +1,11 @@
 #!/usr/bin/env python
 import logging
-logging.basicConfig(format='%(asctime)-15s %(message)s', level=logging.INFO)
 import argparse
 import can
 from can.interfaces import remote
 
+logging.basicConfig(format='%(asctime)-15s %(message)s', level=logging.INFO)
+
 
 def main():
     parser = argparse.ArgumentParser(description="Remote CAN server")
@@ -22,7 +23,7 @@ def main():
     parser.add_argument('-i', '--interface',
                         help='''Specify the backend CAN interface to use. If left blank,
                         fall back to reading from configuration files.''',
-                        choices=can.interface.VALID_INTERFACES)
+                        choices=can.VALID_INTERFACES)
 
     parser.add_argument('-b', '--bitrate', type=int,
                         help='''Force to use a specific bitrate.

can/interfaces/socketcan/socketcan_ctypes.py

@@ -39,7 +39,7 @@ class SocketcanCtypes_Bus(BusABC):
     channel_info = "ctypes socketcan channel"
 
     def __init__(self,
-                 channel=can.rc['channel'],
+                 channel=0,
                  receive_own_messages=False,
                  *args, **kwargs):
         """

can/io/logger.py

@@ -79,7 +79,7 @@ def main():
     parser.add_argument('-i', '--interface', dest="interface",
                         help='''Specify the backend CAN interface to use. If left blank,
                         fall back to reading from configuration files.''',
-                        choices=can.interface.VALID_INTERFACES)
+                        choices=can.VALID_INTERFACES)
 
     parser.add_argument('--filter', help='''Comma separated filters can be specified for the given CAN interface:
         <can_id>:<can_mask> (matches when <received_can_id> & mask == can_id & mask)

can/util.py

@@ -4,6 +4,10 @@
 """
 
 import time
+
+import can
+from can.interfaces import VALID_INTERFACES
+
 try:
     from configparser import ConfigParser
 except ImportError:
@@ -88,37 +92,74 @@ def load_environment_config():
     )
 
 
-def load_config(path=None):
+def load_config(path=None, config=None):
     """
     Returns a dict with configuration details which is loaded from (in this order):
 
-    * Environment variables CAN_INTERFACE, CAN_CHANNEL
-    * Config files ``/etc/can.conf`` or ``~/.can`` or ``~/.canrc``
+    - config
+    - can.rc
+    - Environment variables CAN_INTERFACE, CAN_CHANNEL
+    - Config files ``/etc/can.conf`` or ``~/.can`` or ``~/.canrc``
       where the latter may add or replace values of the former.
 
-    Interface can be kvaser, socketcan, socketcan_ctypes, socketcan_native, serial
+    Interface can be any of the strings from ``can.VALID_INTERFACES`` for example:
+    kvaser, socketcan, pcan, usb2can, ixxat, nican, remote, virtual.
+
+    .. note::
+
+        If you pass ``"socketcan"`` this automatically selects between the
+        native and ctypes version.
+
+    :param path:
+        Optional path to config file.
+    :param config:
+        A dict which may set the 'interface', and/or the 'channel', or neither.
 
-    The returned dictionary may look like this::
+    :return:
+        A config dictionary that should contain 'interface' & 'channel'::
 
-        {
-            'interface': 'python-can backend interface to use',
-            'channel': 'default channel to use',
-        }
+            {
+                'interface': 'python-can backend interface to use',
+                'channel': 'default channel to use',
+            }
 
-    :param path: Optional path to config file.
+        Note ``None`` will be used if all the options are exhausted without
+        finding a value.
     """
-    config = load_file_config(path)
-    config.update(load_environment_config())
+
+
+    system_config = {}
+    configs = [
+        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:
+        if callable(cfg):
+            cfg = cfg()
+        for key in REQUIRED_KEYS:
+            if key not in system_config and key in cfg and cfg[key] is not None:
+                system_config[key] = cfg[key]
+
+        if all(k in system_config for k in REQUIRED_KEYS):
+            break
 
     # substitute None for all values not found
     for key in REQUIRED_KEYS:
-        if key not in config:
-            config[key] = None
+        if key not in system_config:
+            system_config[key] = None
+
+    if system_config['interface'] == 'socketcan':
+        system_config['interface'] = choose_socketcan_implementation()
 
-    if config['interface'] == 'socketcan':
-        config['interface'] = choose_socketcan_implementation()
+    if system_config['interface'] not in VALID_INTERFACES:
+        raise NotImplementedError('Invalid CAN Bus Type - {}'.format(can.rc['interface']))
 
-    return config
+    can.log.debug("can config: {}".format(system_config))
+    return system_config
 
 
 def choose_socketcan_implementation():

doc/bus.rst

@@ -22,7 +22,7 @@ API
     :members:
     :special-members: __iter__
 
-.. autoclass:: can.interfaces.interface.Bus
+.. autoclass:: can.interface.Bus
     :members:
     :special-members: __iter__