From aa0f3c4fa608cacf4c7bb37750a9db7172f91d65 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Wed, 16 May 2018 19:09:47 +0200 Subject: [PATCH 1/6] cleanups,fixes & docs in Bus.__new__() and util.load_config() --- can/interface.py | 43 +++++++++++++++++------------------ can/util.py | 58 +++++++++++++++++++++++++++++++++--------------- 2 files changed, 62 insertions(+), 39 deletions(-) diff --git a/can/interface.py b/can/interface.py index f6ba2bc56..ba966567b 100644 --- a/can/interface.py +++ b/can/interface.py @@ -100,35 +100,36 @@ class Bus(BusABC): """ @classmethod - def __new__(cls, other, channel=None, *args, **kwargs): + 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) + # 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 index 6ca57b5ea..20502c363 100644 --- a/can/util.py +++ b/can/util.py @@ -121,10 +121,17 @@ 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':: @@ -132,46 +139,61 @@ def load_config(path=None, config=None): { '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 + if 'bustype' in cfg: + if 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(): From c6873bb035aad822973bcd045cf11ed0f2687cfb Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Wed, 16 May 2018 19:17:44 +0200 Subject: [PATCH 2/6] fix wrong method call --- can/interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interface.py b/can/interface.py index ba966567b..8c16a3c96 100644 --- a/can/interface.py +++ b/can/interface.py @@ -116,7 +116,7 @@ def __new__(cls, *args, **config): """ # figure out the rest of the configuration; this might raise an error - config = load_config(**config) + config = load_config(config=config) # resolve the bus class to use for that interface cls = _get_class_for_interface(config['interface']) From 40f70aa57b213b914de61bb7b5c080c448dc4482 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Wed, 16 May 2018 19:27:28 +0200 Subject: [PATCH 3/6] fix case where interface was not present but bustype was --- can/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/can/util.py b/can/util.py index 20502c363..a947e8979 100644 --- a/can/util.py +++ b/can/util.py @@ -167,9 +167,9 @@ def load_config(path=None, config=None): for cfg in config_sources: if callable(cfg): cfg = cfg() - # remove legacy operator + # remove legacy operator (and copy to interface if not already present) if 'bustype' in cfg: - if not cfg['interface']: + if 'interface' not in cfg or not cfg['interface']: cfg['interface'] = cfg['bustype'] del cfg['bustype'] # copy all new parameters From 20c3637574edb1faef5be4e94a1692eb7807ce04 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Fri, 18 May 2018 14:39:10 +0200 Subject: [PATCH 4/6] debugging commit --- can/interface.py | 4 +++- can/interfaces/virtual.py | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/can/interface.py b/can/interface.py index 8c16a3c96..82e008ca9 100644 --- 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 @@ -129,6 +129,8 @@ def __new__(cls, *args, **config): raise ValueError("channel argument missing") # the channel attribute should be present in **config + print("DEBUGGING: ", args) + print("DEBUGGING: ", config) return cls(*args, **config) diff --git a/can/interfaces/virtual.py b/can/interfaces/virtual.py index c364cda59..ec7a6426f 100644 --- a/can/interfaces/virtual.py +++ b/can/interfaces/virtual.py @@ -33,7 +33,7 @@ class VirtualBus(BusABC): A virtual CAN bus using an internal message queue. It can be used for example for testing. - In this interface, a channel is an arbitarty object used as + In this interface, a channel is an arbitrary object used as an identifier for connected buses. Implements :meth:`can.BusABC._detect_available_configs`; see @@ -78,7 +78,7 @@ def shutdown(self): with channels_lock: self.channel.remove(self.queue) - # remove if emtpy + # remove if empty if not self.channel: del channels[self.channel_id] From e642db648dc795b8262e523d7b902503d180372c Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Sun, 20 May 2018 10:58:52 +0200 Subject: [PATCH 5/6] attempt to fix constructor --- can/interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can/interface.py b/can/interface.py index 82e008ca9..6d4abb82b 100644 --- a/can/interface.py +++ b/can/interface.py @@ -99,7 +99,7 @@ class Bus(BusABC): configuration file from default locations. """ - @classmethod + @staticmethod def __new__(cls, *args, **config): """ Takes the same arguments as :class:`can.BusABC.__init__` with the addition of: From ea72864b7414936fdacf8f8fff31f4f1f4130f15 Mon Sep 17 00:00:00 2001 From: Felix Divo Date: Mon, 28 May 2018 00:56:14 +0200 Subject: [PATCH 6/6] removed left over print statements --- can/interface.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/can/interface.py b/can/interface.py index 6d4abb82b..0fc3a41e9 100644 --- a/can/interface.py +++ b/can/interface.py @@ -129,8 +129,6 @@ def __new__(cls, *args, **config): raise ValueError("channel argument missing") # the channel attribute should be present in **config - print("DEBUGGING: ", args) - print("DEBUGGING: ", config) return cls(*args, **config)