diff --git a/dbc2val/dbcfeeder.py b/dbc2val/dbcfeeder.py index 966dba7d..cedd1aa0 100755 --- a/dbc2val/dbcfeeder.py +++ b/dbc2val/dbcfeeder.py @@ -23,19 +23,20 @@ """ import argparse +import asyncio import configparser import enum import logging import os import queue import sys -import time import threading -import asyncio +import time from signal import SIGINT, SIGTERM, signal -from typing import Any -from typing import Dict +from typing import Any, Dict, Set + +import cantools.database from dbcfeederlib import canclient from dbcfeederlib import canplayer @@ -131,18 +132,18 @@ def __init__(self, client_wrapper: clientwrapper.ClientWrapper, def start( self, canport, - dbcfile, - mappingfile, - dbc_default_file, - candumpfile=None, - use_j1939=False, - use_strict_parsing=False + dbcfile: str, + mappingfile: str, + dbc_default_file: str, + candumpfile: str = None, + use_j1939: bool = False, + use_strict_parsing: bool = False ): # Read DBC file self._dbc_parser = dbcparser.DBCParser(dbcfile, use_strict_parsing) - log.info("Using mapping: {}".format(mappingfile)) + log.info("Using mapping file: %s", mappingfile) self._mapper = dbc2vssmapper.Mapper(mappingfile, self._dbc_parser, dbc_default_file) self._client_wrapper.start() @@ -201,7 +202,7 @@ def start( log.error("Subscribing to VSS signals not supported by chosen client!") self.stop() else: - log.info(f"Starting transmit thread, using {canport}") + log.info("Starting transmit thread, using %s", canport) # For now creating another bus # Maybe support different buses for downstream/upstream in the future @@ -287,10 +288,12 @@ def _run_receiver(self): vss_mapping = self._mapper.get_dbc2val_mapping(vss_observation.dbc_name, vss_observation.vss_name) value = vss_mapping.transform_value(vss_observation.raw_value) if value is None: - log.warning(f"Value ignored for dbc {vss_observation.dbc_name} to VSS {vss_observation.vss_name}," - f" from raw value {value} of type {type(value)}") + log.warning( + "Value ignored for dbc %s to VSS %s, from raw value %s of type %s", + vss_observation.dbc_name, vss_observation.vss_name, value, type(value) + ) elif not vss_mapping.change_condition_fulfilled(value): - log.debug(f"Value condition not fulfilled for VSS {vss_observation.vss_name}, value {value}") + log.debug("Value condition not fulfilled for VSS %s, value %s", vss_observation.vss_name, value) else: # get values out of the canreplay and map to desired signals target = vss_observation.vss_name @@ -301,8 +304,10 @@ def _run_receiver(self): # Give status message after 1, 2, 4, 8, 16, 32, 64, .... messages have been sent messages_sent += 1 if messages_sent >= (2 * last_sent_log_entry): - log.info(f"Number of VSS messages sent so far: {messages_sent}, " - f"queue max size: {queue_max_size}") + log.info( + "Number of VSS messages sent so far: %d, queue max size: %d", + messages_sent, queue_max_size + ) last_sent_log_entry = messages_sent except queue.Empty: pass @@ -311,30 +316,32 @@ def _run_receiver(self): async def vss_update(self, updates): log.debug("vss-Update callback!") - dbc_ids = set() + dbc_signal_names: Set[str] = set() for update in updates: if update.entry.value is not None: # This shall currently never happen as we do not subscribe to this - log.warning(f"Current value for {update.entry.path} is now: " - f"{update.entry.value.value} of type {type(update.entry.value.value)}") + log.warning( + "Current value for %s is now: %s of type %s", + update.entry.path, update.entry.value.value, type(update.entry.value.value) + ) if update.entry.actuator_target is not None: - log.debug(f"Target value for {update.entry.path} is now: {update.entry.actuator_target} " - f"of type {type(update.entry.actuator_target.value)}") - new_dbc_ids = self._mapper.handle_update(update.entry.path, update.entry.actuator_target.value) - dbc_ids.update(new_dbc_ids) - - can_ids = set() - for dbc_id in dbc_ids: - can_id = self._dbc_parser.get_canid_for_signal(dbc_id) - can_ids.add(can_id) - - for can_id in can_ids: - log.debug(f"CAN id to be sent, this is {can_id}") - sig_dict = self._mapper.get_value_dict(can_id) - message_data = self._dbc_parser.db.get_message_by_frame_id(can_id) - data = message_data.encode(sig_dict) - self._canclient.send(arbitration_id=message_data.frame_id, data=data) + log.debug( + "Target value for %s is now: %s of type %s", + update.entry.path, update.entry.actuator_target, type(update.entry.actuator_target.value) + ) + affected_signals = self._mapper.handle_update(update.entry.path, update.entry.actuator_target.value) + dbc_signal_names.update(affected_signals) + + messages_to_send: Set[cantools.database.Message] = set() + for signal_name in dbc_signal_names: + messages_to_send.update(self._dbc_parser.get_messages_for_signal(signal_name)) + + for message_definition in messages_to_send: + log.debug("sending CAN message %s with frame ID %#x", message_definition.name, message_definition.frame_id) + sig_dict = self._mapper.get_value_dict(message_definition.frame_id) + data = message_definition.encode(sig_dict) + self._canclient.send(arbitration_id=message_definition.frame_id, data=data) async def _run_subscribe(self): """ @@ -358,8 +365,8 @@ def parse_config(filename): if filename: if not os.path.exists(filename): - log.warning("Couldn't find config file {}".format(filename)) - raise Exception("Couldn't find config file {}".format(filename)) + log.warning("Couldn't find config file %s", filename) + raise FileNotFoundError(filename=filename) configfile = filename else: config_candidates = [ @@ -372,19 +379,14 @@ def parse_config(filename): configfile = candidate break - log.info("Using config: {}".format(configfile)) + log.info("Using config: %s", configfile) if configfile is None: return {} config = configparser.ConfigParser() readed = config.read(configfile) - if log.level >= logging.DEBUG: - log.debug( - "# config.read({}):\n{}".format( - readed, - {section: dict(config[section]) for section in config.sections()}, - ) - ) + if log.isEnabledFor(logging.DEBUG): + log.debug("# config.read(%s):\n%s", readed, {section: dict(config[section]) for section in config.sections()}) return config @@ -447,7 +449,7 @@ def main(argv): parser.add_argument('--no-val2dbc', action='store_true', help="Do not monitor mapped signals in KUKSA.val") - args = parser.parse_args() + args = parser.parse_args(argv) config = parse_config(args.config) @@ -485,7 +487,7 @@ def main(argv): if "root_ca_path" in config["general"]: path = config['general']['root_ca_path'] - log.info(f"Given root CA path: {path}") + log.info("Given root CA path: %s", path) client_wrapper.set_root_ca_path(path) elif client_wrapper.get_tls(): # We do not want to rely on kuksa-client default @@ -493,11 +495,11 @@ def main(argv): if "tls_server_name" in config["general"]: name = config['general']['tls_server_name'] - log.info(f"Given TLS server name: {name}") + log.info("Given TLS server name: %s", name) client_wrapper.set_tls_server_name(name) if "token" in config["general"]: - log.info(f"Given token information: {config['general']['token']}") + log.info("Given token information: %s", config['general']['token']) client_wrapper.set_token_path(config["general"]["token"]) else: log.info("Token information not given") @@ -593,7 +595,7 @@ def main(argv): # By default enabled log.info("Alt5") use_dbc2val = True - log.info(f"DBC2VAL mode is: {use_dbc2val}") + log.info("DBC2VAL mode is: %s", use_dbc2val) if args.val2dbc: use_val2dbc = True @@ -608,12 +610,12 @@ def main(argv): else: # By default disabled use_val2dbc = False - log.info(f"VAL2DBC mode is: {use_val2dbc}") + log.info("VAL2DBC mode is: %s", use_val2dbc) feeder = Feeder(client_wrapper, elmcan_config, dbc2val=use_dbc2val, val2dbc=use_val2dbc) def signal_handler(signal_received, *_): - log.info(f"Received signal {signal_received}, stopping...") + log.info("Received signal %s, stopping...", signal_received) # If we get told to shutdown a second time. Just do it. if feeder.is_stopping(): @@ -651,7 +653,7 @@ def parse_level(specified_level, default=default): "critical", ]: return specified_level.upper() - raise Exception(f"could not parse '{specified_level}' as a log level") + raise ValueError(f"could not parse '{specified_level}' as a log level") return default parsed_loglevels = {} @@ -663,7 +665,7 @@ def parse_level(specified_level, default=default): if len(spec_parts) == 1: # This is a root level spec if "root" in parsed_loglevels: - raise Exception("multiple root loglevels specified") + raise ValueError("multiple root loglevels specified") parsed_loglevels["root"] = parse_level(spec_parts[0]) if len(spec_parts) == 2: logger_name = spec_parts[0] diff --git a/dbc2val/dbcfeederlib/dbc2vssmapper.py b/dbc2val/dbcfeederlib/dbc2vssmapper.py index 5142a5e7..abefa9e9 100644 --- a/dbc2val/dbcfeederlib/dbc2vssmapper.py +++ b/dbc2val/dbcfeederlib/dbc2vssmapper.py @@ -81,14 +81,15 @@ def time_condition_fulfilled(self, time: float) -> bool: Value (on_change) condition not evaluated """ fulfilled = True - log.debug(f"Checking interval for {self.vss_name}. " - f"Time is {time}, last sent {self.last_time}") + log.debug( + "Checking interval for %s. Time is %#.3f, last sent %#.3f", + self.vss_name, time, self.last_time) # First shall always evaluate to true if (self.interval_ms > 0) and (self.last_time != 0.0): diff_ms = (time - self.last_time) * 1000.0 if diff_ms < self.interval_ms: - log.debug(f"Interval not exceeded for {self.vss_name}. Time is {time}") + log.debug("Interval not exceeded for %s. Time is %#.3f", self.vss_name, time) fulfilled = False # We must set time already now even if a value check is performed later @@ -105,8 +106,9 @@ def change_condition_fulfilled(self, vss_value: Any) -> bool: if time condition is fulfilled. """ fulfilled = False - log.debug(f"Checking change condition for {self.vss_name}. " - f"New value {vss_value}, old value {self.last_vss_value}") + log.debug( + "Checking change condition for %s. New value %s, old value %s", + self.vss_name, vss_value, self.last_vss_value) if vss_value is not None: if self.last_vss_value is None: @@ -128,7 +130,7 @@ def transform_value(self, value: Any) -> Any: """ vss_value = None if self.transform is None: - log.debug(f"No mapping to VSS {self.vss_name}, using raw value {value}") + log.debug("No mapping to VSS %s, using raw value %s", self.vss_name, value) vss_value = value else: if "mapping" in self.transform: @@ -148,25 +150,30 @@ def transform_value(self, value: Any) -> Any: # It is assumed that you may consider it ok that transformation fails sometimes, # so giving warning instead of error # This could be e.g. trying to treat a string as int - log.warning(f"Transformation failed for value {value} " - f"for VSS signal {self.vss_name}, signal ignored!", exc_info=True) + log.warning( + "Transformation failed for value %s for VSS signal %s, signal ignored!", + value, self.vss_name, + exc_info=True) else: # It is supposed that "extract_verify_transform" already have checked that # we have a valid transform, so we shall never end up here log.error("Unsupported transform") if vss_value is None: - log.info(f"No mapping to VSS {self.vss_name} found for raw value {value}," - f"returning None to indicate that it shall be ignored!") + log.info( + """No mapping to VSS %s found for raw value %s, + returning None to indicate that it shall be ignored!""", + self.vss_name, value + ) else: - log.debug(f"Transformed value {vss_value} for {self.vss_name}") + log.debug("Transformed value %s for %s", vss_value, self.vss_name) return vss_value class Mapper: """ The mapper class contain all mappings between dbc and vss. - It also contain functionality for transforming data + It also contains functionality for transforming data. """ # Where we keep mapping, key is dbc signal name @@ -176,26 +183,32 @@ class Mapper: # Same, but key is CAN id mapping val2dbc_can_id_mapping: Dict[int, List[VSSMapping]] = {} - def __init__(self, filename, dbc_parser: dbcparser.DBCParser, dbc_default_filename=""): - with open(filename, "r") as file: + def __init__(self, + filename: str, + dbc_parser: dbcparser.DBCParser, + dbc_default_filename: str = "", + fail_on_duplicate_signal_definitions: bool = False): + + with open(filename, "r", encoding="utf-8") as file: try: jsonmapping = json.load(file) - log.info(f"Reading dbc configurations from {filename}") + log.info("Reading VSS<->DBC mapping configurations from %s", filename) except Exception: - log.error(f"Failed to read json from {filename}", exc_info=True) + log.error("Failed to read JSON from %s", filename, exc_info=True) sys.exit(-1) self.dbc_default = {} if dbc_default_filename != "": - log.info(f"Reading default DBC values from {dbc_default_filename}") - with open(dbc_default_filename, "r") as file: + with open(dbc_default_filename, "r", encoding="utf-8") as file: try: self.dbc_default = json.load(file) + log.info("Read default DBC signal values from %s", dbc_default_filename) except Exception: - log.error(f"Failed to read DBC values from {dbc_default_filename}", exc_info=True) + log.error("Failed to read default DBC signal values from %s", dbc_default_filename, exc_info=True) sys.exit(-1) self.dbc_parser = dbc_parser - self.traverse_vss_node("", jsonmapping) + self._fail_on_duplicate_signal_definitions = fail_on_duplicate_signal_definitions + self._traverse_vss_node("", jsonmapping) def transform_dbc_value(self, vss_observation: VSSObservation) -> Any: """ @@ -204,9 +217,9 @@ def transform_dbc_value(self, vss_observation: VSSObservation) -> Any: vss_signal = self.get_dbc2val_mapping(vss_observation.dbc_name, vss_observation.vss_name) if vss_signal: value = vss_signal.transform_value(vss_observation.raw_value) - log.debug(f"Transformed dbc {vss_observation.dbc_name} to VSS " - f"{vss_observation.vss_name}, " - f"from raw value {vss_observation.raw_value} to {value}") + log.debug( + "Transformed dbc %s to VSS %s, from raw value %s to %s", + vss_observation.dbc_name, vss_observation.vss_name, vss_observation.raw_value, value) else: log.error("No mapping found, that is not expected!") value = None @@ -217,7 +230,7 @@ def extract_verify_transform(self, expanded_name: str, node: dict): Extracts transformation and checks it seems to be correct """ if "transform" not in node: - log.debug(f"No transformation found for {expanded_name}") + log.debug("No transformation found for %s", expanded_name) # For now assumed that None is Ok return None transform = node["transform"] @@ -225,32 +238,32 @@ def extract_verify_transform(self, expanded_name: str, node: dict): has_mapping = False if not isinstance(transform, dict): - log.error(f"Transform not dict for {expanded_name}") + log.error("Transform not dict for %s", expanded_name) sys.exit(-1) if "mapping" in transform: tmp = transform["mapping"] if not isinstance(tmp, list): - log.error(f"Transform mapping not list for {expanded_name}") + log.error("Transform mapping not list for %s", expanded_name) sys.exit(-1) for item in tmp: if not (("from" in item) and ("to" in item)): - log.error(f"Mapping missing to and from in {item} for {expanded_name}") + log.error("Mapping missing to and from in %s for %s", item, expanded_name) sys.exit(-1) has_mapping = True if "math" in transform: if has_mapping: - log.error(f"Can not have both mapping and math for {expanded_name}") + log.error("Can not have both mapping and math for %s", expanded_name) sys.exit(-1) if not isinstance(transform["math"], str): - log.error(f"Math must be str for {expanded_name}") + log.error("Math must be str for %s", expanded_name) sys.exit(-1) elif not has_mapping: - log.error(f"Unsupported transform for {expanded_name}") + log.error("Unsupported transform for %s", expanded_name) sys.exit(-1) return transform - def analyze_dbc2val(self, expanded_name, node: dict, dbc2vss: dict): + def _analyze_dbc2val(self, expanded_name, node: dict, dbc2vss: dict): """ Analyze a dbc2val entry (from CAN to KUKSA) """ @@ -258,7 +271,7 @@ def analyze_dbc2val(self, expanded_name, node: dict, dbc2vss: dict): transform = self.extract_verify_transform(expanded_name, dbc2vss) dbc_name = dbc2vss.get("signal", "") if dbc_name == "": - log.error(f"No dbc signal found for {expanded_name}") + log.error("No dbc signal found for %s", expanded_name) sys.exit(-1) on_change: bool = False if "on_change" in dbc2vss: @@ -266,20 +279,19 @@ def analyze_dbc2val(self, expanded_name, node: dict, dbc2vss: dict): if isinstance(tmp, bool): on_change = tmp else: - log.error(f"Value for on_change ({tmp}) is not bool") + log.error("Value for on_change (%s) is not bool", tmp) sys.exit(-1) if "interval_ms" in dbc2vss: interval = dbc2vss["interval_ms"] if not isinstance(interval, int): - log.error(f"Faulty interval for {expanded_name}") + log.error("Faulty interval for %s", expanded_name) sys.exit(-1) else: if on_change: - log.info(f"Using default interval 0 ms for {expanded_name} " - f"as it has on_change condition") + log.info("Using default interval 0 ms for %s as it has on_change condition", expanded_name) interval = 0 else: - log.info(f"Using default interval 1000 ms for {expanded_name}") + log.info("Using default interval 1000 ms for %s", expanded_name) interval = 1000 mapping_entry = VSSMapping(expanded_name, dbc_name, transform, interval, on_change, node["datatype"], node["description"]) @@ -287,59 +299,84 @@ def analyze_dbc2val(self, expanded_name, node: dict, dbc2vss: dict): self.dbc2val_mapping[dbc_name] = [] self.dbc2val_mapping[dbc_name].append(mapping_entry) - def analyze_val2dbc(self, expanded_name, node: dict, vss2dbc: dict): + def _analyze_val2dbc(self, expanded_name, node: dict, vss2dbc: dict): """ - Analyze a dbc2val entry (from CAN to KUKSA) + Analyze a val2dbc entry (mapping a KUKSA VSS datapoint to a CAN message signal) """ - transform = self.extract_verify_transform(expanded_name, vss2dbc) - dbc_name = vss2dbc.get("signal", "") - if dbc_name == "": - log.error(f"No dbc signal found for {expanded_name}") + dbc_signal_name = vss2dbc.get("signal", "") + if dbc_signal_name == "": + log.error("\"vss2dbc\" mapping for %s does not contain mandatory \"signal\" property", expanded_name) sys.exit(-1) + + dbc_message_defs = self.dbc_parser.get_messages_for_signal(dbc_signal_name) + if len(dbc_message_defs) == 0: + log.error( + "VSS datapoint %s is mapped to CAN signal %s which is not used in any message definition", + expanded_name, dbc_signal_name + ) + return + + if len(dbc_message_defs) > 1 and log.isEnabledFor(logging.WARNING): + message_names = ', '.join([msg_def.name for msg_def in dbc_message_defs]) + if self._fail_on_duplicate_signal_definitions: + log.error( + """Mapping of VSS datapoint %s to CAN signal %s is ambiguous because signal is used by multiple + CAN messages (%s)""", + expanded_name, dbc_signal_name, message_names) + sys.exit(-1) + else: + log.warning( + """Mapping of VSS datapoint %s to CAN signal %s is ambiguous because signal is used by multiple + CAN messages (%s). Make sure that signal %s has the same semantics in all messages in order to + prevent unexpected messages being sent on the CAN bus when the VSS datapoint's target value + is being set.""", + expanded_name, dbc_signal_name, message_names, dbc_signal_name) + + transform = self.extract_verify_transform(expanded_name, vss2dbc) # For now we only support on_change, and we actually do not use the values on_change: bool = True interval = 0 if "on_change" in vss2dbc: - log.warning(f"on_change attribute ignored for {expanded_name}") + log.warning("on_change attribute ignored for %s", expanded_name) if "interval_ms" in vss2dbc: - log.warning(f"interval_ms attribute ignored for {expanded_name}") + log.warning("interval_ms attribute ignored for %s", expanded_name) - mapping_entry = VSSMapping(expanded_name, dbc_name, transform, interval, on_change, + mapping_entry = VSSMapping(expanded_name, dbc_signal_name, transform, interval, on_change, node["datatype"], node["description"]) - if dbc_name not in self.val2dbc_mapping: + if dbc_signal_name not in self.val2dbc_mapping: self.val2dbc_mapping[expanded_name] = [] self.val2dbc_mapping[expanded_name].append(mapping_entry) # Also add CAN-id - dbc_can_id = self.dbc_parser.get_canid_for_signal(dbc_name) - if not dbc_can_id: - log.error(f"Could not find {dbc_name}") - return - if dbc_can_id not in self.val2dbc_can_id_mapping: - self.val2dbc_can_id_mapping[dbc_can_id] = [] - self.val2dbc_can_id_mapping[dbc_can_id].append(mapping_entry) + for message_def in dbc_message_defs: + if message_def.frame_id not in self.val2dbc_can_id_mapping: + self.val2dbc_can_id_mapping[message_def.frame_id] = [] + self.val2dbc_can_id_mapping[message_def.frame_id].append(mapping_entry) - def analyze_signal(self, expanded_name, node): + def _analyze_signal(self, expanded_name, node): """ Analyzes a signal and add mapping entry if correct mapping found """ dbc2vss_def = None if "dbc" in node: - log.debug(f"Signal {expanded_name} has dbc!") + log.debug("Found \"dbc\" mapping definition for VSS datapoint %s", expanded_name) dbc2vss_def = node["dbc"] if "dbc2vss" in node: - log.error(f"Node {expanded_name} has both dbc and dbc2vss") + log.error( + "VSS datapoint %s may have either \"dbc\" or \"dbc2vss\" mapping defined, but not both", + expanded_name + ) sys.exit(-1) elif "dbc2vss" in node: - log.debug(f"Signal {expanded_name} has dbc2vss!") + log.debug("Found \"dbc2vss\" mapping definition for VSS datapoint %s", expanded_name) dbc2vss_def = node["dbc2vss"] if dbc2vss_def is not None: - self.analyze_dbc2val(expanded_name, node, dbc2vss_def) + self._analyze_dbc2val(expanded_name, node, dbc2vss_def) if "vss2dbc" in node: - self.analyze_val2dbc(expanded_name, node, node["vss2dbc"]) + self._analyze_val2dbc(expanded_name, node, node["vss2dbc"]) - def traverse_vss_node(self, name, node, prefix=""): + def _traverse_vss_node(self, name, node, prefix=""): """ Traverse a vss node/tree and order all found VSS signals to be analyzed so that mapping can be extracted @@ -358,13 +395,13 @@ def traverse_vss_node(self, name, node, prefix=""): # Assuming it to be a dict if is_branch: for item in node["children"].items(): - self.traverse_vss_node(item[0], item[1], prefix) + self._traverse_vss_node(item[0], item[1], prefix) elif is_signal: expanded_name = prefix + name - self.analyze_signal(expanded_name, node) + self._analyze_signal(expanded_name, node) elif isinstance(node, dict): for item in node.items(): - self.traverse_vss_node(item[0], item[1], prefix) + self._traverse_vss_node(item[0], item[1], prefix) def get_dbc2val_mapping(self, dbc_name: str, vss_name: str) -> Optional[VSSMapping]: """ @@ -390,7 +427,7 @@ def get_vss_names(self) -> Set[str]: for entry in self.dbc2val_mapping.values(): for vss_mapping in entry: vss_names.add(vss_mapping.vss_name) - for key_entry in self.val2dbc_mapping.keys(): + for key_entry in self.val2dbc_mapping: vss_names.add(key_entry) return vss_names @@ -424,25 +461,25 @@ def handle_update(self, vss_name, value: Any) -> Set[str]: def get_default_values(self, can_id) -> Dict[str, Any]: res = {} - for signal in self.dbc_parser.get_signals_for_canid(can_id): - if signal in self.dbc_default: - res[signal] = self.dbc_default[signal] + for signal in self.dbc_parser.get_signals_by_frame_id(can_id): + if signal.name in self.dbc_default: + res[signal.name] = self.dbc_default[signal.name] else: - log.error(f"No default value for {signal} in CAN id {can_id}") + log.error("No default value for signal %s of message with CAN id %#x defined", signal, can_id) return res def get_value_dict(self, can_id): - log.debug(f"Using stored information to create CAN-frame for {can_id}") + log.debug("Using stored information to create CAN-frame for %#x", can_id) res = self.get_default_values(can_id) for can_mapping in self.val2dbc_can_id_mapping[can_id]: - log.debug(f"Using DBC id {can_mapping.dbc_name} with value {can_mapping.last_dbc_value}") + log.debug("Using DBC id %s with value %s", can_mapping.dbc_name, can_mapping.last_dbc_value) if can_mapping.last_dbc_value is not None: res[can_mapping.dbc_name] = can_mapping.last_dbc_value return res def __contains__(self, key): - return key in self.dbc2val_mapping.keys() + return key in self.dbc2val_mapping def __getitem__(self, item): return self.dbc2val_mapping[item] diff --git a/dbc2val/dbcfeederlib/dbcparser.py b/dbc2val/dbcfeederlib/dbcparser.py index c9405af0..81686d26 100644 --- a/dbc2val/dbcfeederlib/dbcparser.py +++ b/dbc2val/dbcfeederlib/dbcparser.py @@ -21,7 +21,7 @@ import logging import sys import os -from typing import Set, Optional, Dict, cast, Tuple +from typing import cast, Dict, List, Tuple import cantools.database log = logging.getLogger(__name__) @@ -29,32 +29,57 @@ class DBCParser: def __init__(self, dbcfile: str, use_strict_parsing: bool = True): + """Create a parser from definitions in a DBC file.""" first = True found_names = set() for name in dbcfile.split(","): filename = name.strip() if filename in found_names: - log.warning("The DBC file {} has already been read, ignoring it!".format(filename)) + log.warning("The DBC file %s has already been read, ignoring it!", filename) continue found_names.add(filename) if first: - log.info("Reading DBC file {} as first file".format(filename)) - db = cantools.database.load_file(filename, strict=use_strict_parsing) + log.info("Reading definitions from DBC file %s", filename) + database = cantools.database.load_file(filename, strict=use_strict_parsing) # load_file can return multiple types of databases, make sure we have CAN database - if isinstance(db, cantools.database.can.database.Database): - self.db = cast(cantools.database.can.database.Database, db) + if isinstance(database, cantools.database.can.database.Database): + self._db = cast(cantools.database.can.database.Database, database) first = False else: - log.error("File is not a CAN database, likely a diagnostics database") + log.error("File %s is not a CAN database, likely a diagnostics database", filename) sys.exit(-1) else: - log.info("Adding definitions from {}".format(filename)) + log.info("Adding definitions from DBC file %s", filename) self._add_db_file(filename) # Init some dictionaries to speed up search - self.signal_to_canid: Dict[str, Optional[int]] = {} - self.canid_to_signals: Dict[int, Set[str]] = {} + self._signal_to_message_definitions = self._populate_signal_to_message_map() + + def _populate_signal_to_message_map(self) -> Dict[str, List[cantools.database.Message]]: + + signal_to_message_defs: Dict[str, List[cantools.database.Message]] = {} + + for msg_definition in self._db.messages: + for signal in msg_definition.signals: + if signal.name in signal_to_message_defs: + signal_to_message_defs[signal.name].append(msg_definition) + else: + signal_to_message_defs[signal.name] = list({msg_definition}) + + if log.isEnabledFor(logging.WARNING): + for (sig_name, messages) in signal_to_message_defs.items(): + if len(messages) > 1: + log.warning( + "Signal name %s is being used in multiple messages (%s).", + sig_name, ', '.join([msg_def.name for msg_def in messages]) + ) + log.warning( + """Make sure that signals have the same semantics in all messages where they are used + to prevent unexpected behaviour when mapping VSS datapoints to these signals.""" + ) + + return signal_to_message_defs def _determine_db_format_and_encoding(self, filename) -> Tuple[str, str]: db_format = os.path.splitext(filename)[1][1:].lower() @@ -72,47 +97,34 @@ def _determine_db_format_and_encoding(self, filename) -> Tuple[str, str]: def _add_db_file(self, filename: str): db_format, encoding = self._determine_db_format_and_encoding(filename) if db_format == "arxml": - self.db.add_arxml_file(filename, encoding) + self._db.add_arxml_file(filename, encoding) elif db_format == "dbc": - self.db.add_dbc_file(filename, encoding) + self._db.add_dbc_file(filename, encoding) elif db_format == "kcd": - self.db.add_kcd_file(filename, encoding) + self._db.add_kcd_file(filename, encoding) elif db_format == "sym": - self.db.add_sym_file(filename, encoding) + self._db.add_sym_file(filename, encoding) else: log.warning("Cannot read CAN message definitions from file using unsupported format: %s", db_format) - def get_canid_for_signal(self, sig_to_find: str) -> Optional[int]: - if sig_to_find in self.signal_to_canid: - return self.signal_to_canid[sig_to_find] - - for msg in self.db.messages: - for signal in msg.signals: - if signal.name == sig_to_find: - frame_id = msg.frame_id - log.info( - "Found signal in DBC file {} in CAN frame id 0x{:02x}".format( - signal.name, frame_id - ) - ) - self.signal_to_canid[sig_to_find] = frame_id - return frame_id - log.warning("Signal {} not found in DBC file".format(sig_to_find)) - self.signal_to_canid[sig_to_find] = None - return None - - def get_signals_for_canid(self, canid: int) -> Set[str]: - - if canid in self.canid_to_signals: - return self.canid_to_signals[canid] - - for msg in self.db.messages: - if canid == msg.frame_id: - names = set() - for signal in msg.signals: - names.add(signal.name) - self.canid_to_signals[canid] = names - return names - log.warning(f"CAN id {canid} not found in DBC file") - self.canid_to_signals[canid] = set() - return set() + def get_messages_for_signal(self, sig_to_find: str) -> List[cantools.database.Message]: + """Get all CAN message definitions that use a given CAN signal name.""" + if sig_to_find in self._signal_to_message_definitions: + return self._signal_to_message_definitions[sig_to_find] + + log.warning("Signal %s not found in CAN message database", sig_to_find) + empty_list = [] + self._signal_to_message_definitions[sig_to_find] = empty_list + return empty_list + + def get_message_by_frame_id(self, frame_id: int) -> cantools.database.Message: + """Get the CAN message definition for a given CAN frame ID.""" + return self._db.get_message_by_frame_id(frame_id) + + def get_signals_by_frame_id(self, frame_id: int) -> List[cantools.database.Signal]: + """Get the signals of the CAN message definition for a given CAN frame ID.""" + try: + return self.get_message_by_frame_id(frame_id).signals + except Exception: + log.warning("CAN id %s not found in CAN message database", frame_id) + return [] diff --git a/dbc2val/test/test_dbc/dbcparser_test.py b/dbc2val/test/test_dbc/dbcparser_test.py new file mode 100644 index 00000000..a469e0cb --- /dev/null +++ b/dbc2val/test/test_dbc/dbcparser_test.py @@ -0,0 +1,133 @@ +#!/usr/bin/python3 + +######################################################################## +# Copyright (c) 2023 Robert Bosch GmbH +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# SPDX-License-Identifier: Apache-2.0 +######################################################################## + +from dbcfeederlib import dbcparser +import os + +# read config only once +test_path = os.path.dirname(os.path.abspath(__file__)) +def_dbc = test_path + "/../../Model3CAN.dbc" + + +def test_default_dbc(): + + parser = dbcparser.DBCParser(def_dbc) + msg_list = parser.get_messages_for_signal('SteeringAngle129') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 297 + msg_list = parser.get_messages_for_signal('DI_uiSpeed') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 599 + + signals = [signal.name for signal in parser.get_signals_by_frame_id(599)] + assert "DI_speedChecksum" in signals + assert "DI_speedCounter" in signals + assert "DI_uiSpeed" in signals + assert "DI_uiSpeedHighSpeed" in signals + assert "DI_uiSpeedUnits" in signals + assert "DI_vehicleSpeed" in signals + + +def test_split_dbc(): + """ + This verifies that we can read a DBC database split across multiple files. + Difference compared to default is that 'SteeringAngle129' has been moved to test1_2.dbc + """ + parser = dbcparser.DBCParser(test_path + "/test1_1.dbc," + test_path + "/test1_2.dbc") + msg_list = parser.get_messages_for_signal('SteeringAngle129') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 297 + msg_list = parser.get_messages_for_signal('DI_uiSpeed') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 599 + + +def test_duplicated_dbc(): + """ + Load original DBC multiple times + """ + parser = dbcparser.DBCParser(def_dbc + ',' + def_dbc) + msg_list = parser.get_messages_for_signal('SteeringAngle129') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 297 + msg_list = parser.get_messages_for_signal('DI_uiSpeed') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 599 + + +def test_single_kcd(): + + parser = dbcparser.DBCParser(test_path + "/test1_1.kcd") + msg_list = parser.get_messages_for_signal('DI_bmsRequestInterfaceVersion') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 0x16 + + signals = [signal.name for signal in parser.get_signals_by_frame_id(0x16)] + assert "DI_bmsOpenContactorsRequest" in signals + assert "DI_bmsRequestInterfaceVersion" in signals + + +def test_split_kcd(): + """ + This verifies that we can read multiple KCD files. + """ + parser = dbcparser.DBCParser(test_path + "/test1_1.kcd," + test_path + "/test1_2.kcd") + msg_list = parser.get_messages_for_signal('DI_bmsRequestInterfaceVersion') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 0x16 + msg_list = parser.get_messages_for_signal('SteeringAngle129') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 0x129 + + +def test_mixed_file_types(): + """ + This verifies that we can read files of different type. + """ + parser = dbcparser.DBCParser(test_path + "/test1_1.kcd," + test_path + "/test1_2.dbc") + msg_list = parser.get_messages_for_signal('DI_bmsRequestInterfaceVersion') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 0x16 + msg_list = parser.get_messages_for_signal('SteeringAngle129') + assert len(msg_list) == 1 + assert msg_list[0].frame_id == 0x129 + + +def test_duplicate_signal_name_dbc(): + # GIVEN a database with two messages defining a signal of name SignalOne + parser = dbcparser.DBCParser(test_path + "/duplicate_signal_name.kcd") + + # WHEN looing up the two messages by name + msg1 = parser._db.get_message_by_name('First') + msg2 = parser._db.get_message_by_name('Second') + + # THEN they have different frame IDs + assert msg1.frame_id != msg2.frame_id + + # AND they both have a signal of name SignalOne + assert msg1.get_signal_by_name('SignalOne') is not None + assert msg2.get_signal_by_name('SignalOne') is not None + + # BUT WHEN looking up the CAN id for SignalOne + # THEN the IDs of both messages defining the signal are found + msg_list = parser.get_messages_for_signal('SignalOne') + assert len(msg_list) == 2 + assert msg1 in msg_list + assert msg2 in msg_list diff --git a/dbc2val/test/test_dbc/duplicate_signal_name.kcd b/dbc2val/test/test_dbc/duplicate_signal_name.kcd new file mode 100644 index 00000000..2770d9cb --- /dev/null +++ b/dbc2val/test/test_dbc/duplicate_signal_name.kcd @@ -0,0 +1,19 @@ + + + + + + + + + + + + + + + + + + + diff --git a/dbc2val/test/test_dbc/test_dbc.py b/dbc2val/test/test_dbc/test_dbc.py deleted file mode 100644 index 12028b0c..00000000 --- a/dbc2val/test/test_dbc/test_dbc.py +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/python3 - -######################################################################## -# Copyright (c) 2023 Robert Bosch GmbH -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# SPDX-License-Identifier: Apache-2.0 -######################################################################## - -from dbcfeederlib import dbcparser -import os - -# read config only once -test_path = os.path.dirname(os.path.abspath(__file__)) -def_dbc = test_path + "/../../Model3CAN.dbc" - - -def test_default_dbc(): - - parser3 = dbcparser.DBCParser(def_dbc) - assert parser3.get_canid_for_signal('SteeringAngle129') == 297 - assert parser3.get_canid_for_signal('DI_uiSpeed') == 599 - - -def test_splitted_dbc(): - """ - This verifies that we can read a splitted DBC File. - Difference compared to default is that 'SteeringAngle129' has been moved to test1_2.dbc - """ - parser3 = dbcparser.DBCParser(test_path + "/test1_1.dbc," + test_path + "/test1_2.dbc") - assert parser3.get_canid_for_signal('SteeringAngle129') == 297 - assert parser3.get_canid_for_signal('DI_uiSpeed') == 599 - - -def test_duplicated_dbc(): - """ - Load original DBC multiple times - """ - parser3 = dbcparser.DBCParser(def_dbc + ',' + def_dbc) - assert parser3.get_canid_for_signal('SteeringAngle129') == 297 - assert parser3.get_canid_for_signal('DI_uiSpeed') == 599 diff --git a/dbc2val/test/test_dbc/test_kcd.py b/dbc2val/test/test_dbc/test_kcd.py deleted file mode 100644 index 73594769..00000000 --- a/dbc2val/test/test_dbc/test_kcd.py +++ /dev/null @@ -1,57 +0,0 @@ -#!/usr/bin/python3 - -######################################################################## -# Copyright (c) 2023 Robert Bosch GmbH -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# SPDX-License-Identifier: Apache-2.0 -######################################################################## - -from dbcfeederlib import dbcparser -import os - -# read config only once -test_path = os.path.dirname(os.path.abspath(__file__)) - - -def test_single_kcd(): - - parser = dbcparser.DBCParser(test_path + "/test1_1.kcd") - assert parser.get_canid_for_signal('DI_bmsRequestInterfaceVersion') == 0x16 - - -def test_split_kcd(): - """ - This verifies that we can read multiple KCD files. - """ - parser = dbcparser.DBCParser(test_path + "/test1_1.kcd," + test_path + "/test1_2.kcd") - assert parser.get_canid_for_signal('DI_bmsRequestInterfaceVersion') == 0x16 - assert parser.get_canid_for_signal('SteeringAngle129') == 0x129 - - -def test_mixed_file_types(): - """ - This verifies that we can read files of different type. - """ - parser = dbcparser.DBCParser(test_path + "/test1_1.kcd," + test_path + "/test1_2.dbc") - assert parser.get_canid_for_signal('DI_bmsRequestInterfaceVersion') == 0x16 - assert parser.get_canid_for_signal('SteeringAngle129') == 0x129 - - -def test_duplicated_dbc(): - """ - Load original KCD multiple times - """ - parser = dbcparser.DBCParser(test_path + "/test1_1.kcd," + test_path + "/test1_1.kcd") - assert parser.get_canid_for_signal('DI_bmsRequestInterfaceVersion') == 0x16 diff --git a/dbc2val/test/test_mapping_error/mapping_for_ambiguous_signal.json b/dbc2val/test/test_mapping_error/mapping_for_ambiguous_signal.json new file mode 100644 index 00000000..5079b350 --- /dev/null +++ b/dbc2val/test/test_mapping_error/mapping_for_ambiguous_signal.json @@ -0,0 +1,17 @@ +{ + "A": { + "children": { + "B": { + "datatype": "uint8", + "description": "...", + "type": "sensor", + "unit": "km", + "vss2dbc": { + "signal": "SignalOne" + } + } + }, + "description": "Branch A.", + "type": "branch" + } +} \ No newline at end of file diff --git a/dbc2val/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json b/dbc2val/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json new file mode 100644 index 00000000..0f17f1ea --- /dev/null +++ b/dbc2val/test/test_mapping_error/mapping_for_unused_ambiguous_signal.json @@ -0,0 +1,17 @@ +{ + "A": { + "children": { + "B": { + "datatype": "uint8", + "description": "...", + "type": "sensor", + "unit": "km", + "vss2dbc": { + "signal": "SignalTwo" + } + } + }, + "description": "Branch A.", + "type": "branch" + } +} \ No newline at end of file diff --git a/dbc2val/test/test_mapping_error/test_mapping_error.py b/dbc2val/test/test_mapping_error/test_mapping_error.py index deff444d..7a4a2c06 100644 --- a/dbc2val/test/test_mapping_error/test_mapping_error.py +++ b/dbc2val/test/test_mapping_error/test_mapping_error.py @@ -24,15 +24,35 @@ import pytest import logging +test_path = os.path.dirname(os.path.abspath(__file__)) -def test_unknown_transform(caplog, capsys): - test_path = os.path.dirname(os.path.abspath(__file__)) +def test_unknown_transform(caplog: pytest.LogCaptureFixture): + mapping_path = test_path + "/test_unknown_transform.json" parser = dbcparser.DBCParser(test_path + "/../../Model3CAN.dbc") with pytest.raises(SystemExit) as excinfo: dbc2vssmapper.Mapper(mapping_path, parser) - out, err = capsys.readouterr() assert excinfo.value.code == -1 - assert caplog.record_tuples == [("dbcfeederlib.dbc2vssmapper", logging.ERROR, "Unsupported transform for A.B")] + assert ("dbcfeederlib.dbc2vssmapper", logging.ERROR, "Unsupported transform for A.B") in caplog.record_tuples + + +def test_mapper_fails_for_duplicate_signal_definition(): + + mapping_path = test_path + "/mapping_for_ambiguous_signal.json" + parser = dbcparser.DBCParser(test_path + "/../test_dbc/duplicate_signal_name.kcd") + + with pytest.raises(SystemExit) as excinfo: + dbc2vssmapper.Mapper(mapping_path, parser, fail_on_duplicate_signal_definitions=True) + assert excinfo.value.code == -1 + + +def test_mapper_ignores_unused_duplicate_signal_definition(): + + mapping_path = test_path + "/mapping_for_unused_ambiguous_signal.json" + parser = dbcparser.DBCParser(test_path + "/../test_dbc/duplicate_signal_name.kcd") + mapper = dbc2vssmapper.Mapper(mapping_path, parser, fail_on_duplicate_signal_definitions=True) + affected_signal_names = mapper.handle_update("A.B", 15) + assert len(affected_signal_names) == 1 + assert "SignalTwo" in affected_signal_names