diff --git a/can/common.h b/can/common.h index 0031d787f0..03b99e5598 100644 --- a/can/common.h +++ b/can/common.h @@ -76,8 +76,7 @@ class CANParser { uint64_t can_invalid_cnt = CAN_INVALID_CNT; CANParser(int abus, const std::string& dbc_name, - const std::vector &options, - const std::vector &sigoptions); + const std::vector> &messages); CANParser(int abus, const std::string& dbc_name, bool ignore_checksum, bool ignore_counter); #ifndef DYNAMIC_CAPNP void update_string(const std::string &data, bool sendcan); diff --git a/can/common.pxd b/can/common.pxd index b62bef1a26..4f62ba2a15 100644 --- a/can/common.pxd +++ b/can/common.pxd @@ -3,6 +3,7 @@ from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t from libcpp cimport bool +from libcpp.pair cimport pair from libcpp.string cimport string from libcpp.vector cimport vector @@ -48,14 +49,6 @@ cdef extern from "common_dbc.h": vector[Msg] msgs vector[Val] vals - cdef struct SignalParseOptions: - uint32_t address - string name - - cdef struct MessageParseOptions: - uint32_t address - int check_frequency - cdef struct SignalValue: uint32_t address uint64_t ts_nanos @@ -74,7 +67,7 @@ cdef extern from "common.h": cdef cppclass CANParser: bool can_valid bool bus_timeout - CANParser(int, string, vector[MessageParseOptions], vector[SignalParseOptions]) + CANParser(int, string, vector[pair[uint32_t, int]]) void update_strings(vector[string]&, vector[SignalValue]&, bool) except + cdef cppclass CANPacker: diff --git a/can/common_dbc.h b/can/common_dbc.h index 1e0f034520..e8c27278c5 100644 --- a/can/common_dbc.h +++ b/can/common_dbc.h @@ -12,16 +12,6 @@ struct SignalPackValue { double value; }; -struct SignalParseOptions { - uint32_t address; - std::string name; -}; - -struct MessageParseOptions { - uint32_t address; - int check_frequency; -}; - struct SignalValue { uint32_t address; uint64_t ts_nanos; diff --git a/can/parser.cc b/can/parser.cc index db6d4e65e3..224ecfec1e 100644 --- a/can/parser.cc +++ b/can/parser.cc @@ -89,9 +89,7 @@ bool MessageState::update_counter_generic(int64_t v, int cnt_size) { } -CANParser::CANParser(int abus, const std::string& dbc_name, - const std::vector &options, - const std::vector &sigoptions) +CANParser::CANParser(int abus, const std::string& dbc_name, const std::vector> &messages) : bus(abus), aligned_buf(kj::heapArray(1024)) { dbc = dbc_lookup(dbc_name); assert(dbc); @@ -99,14 +97,14 @@ CANParser::CANParser(int abus, const std::string& dbc_name, bus_timeout_threshold = std::numeric_limits::max(); - for (const auto& op : options) { - MessageState &state = message_states[op.address]; - state.address = op.address; + for (const auto& [address, frequency] : messages) { + MessageState &state = message_states[address]; + state.address = address; // state.check_frequency = op.check_frequency, // msg is not valid if a message isn't received for 10 consecutive steps - if (op.check_frequency > 0) { - state.check_threshold = (1000000000ULL / op.check_frequency) * 10; + if (frequency > 0) { + state.check_threshold = (1000000000ULL / frequency) * 10; // bus timeout threshold should be 10x the fastest msg bus_timeout_threshold = std::min(bus_timeout_threshold, state.check_threshold); @@ -114,13 +112,13 @@ CANParser::CANParser(int abus, const std::string& dbc_name, const Msg* msg = NULL; for (const auto& m : dbc->msgs) { - if (m.address == op.address) { + if (m.address == address) { msg = &m; break; } } if (!msg) { - fprintf(stderr, "CANParser: could not find message 0x%X in DBC %s\n", op.address, dbc_name.c_str()); + fprintf(stderr, "CANParser: could not find message 0x%X in DBC %s\n", address, dbc_name.c_str()); assert(false); } @@ -128,28 +126,10 @@ CANParser::CANParser(int abus, const std::string& dbc_name, state.size = msg->size; assert(state.size <= 64); // max signal size is 64 bytes - // track checksums and counters for this message - for (const auto& sig : msg->sigs) { - if (sig.type != SignalType::DEFAULT) { - state.parse_sigs.push_back(sig); - state.vals.push_back(0); - state.all_vals.push_back({}); - } - } - - // track requested signals for this message - for (const auto& sigop : sigoptions) { - if (sigop.address != op.address) continue; - - for (const auto& sig : msg->sigs) { - if (sig.name == sigop.name && sig.type == SignalType::DEFAULT) { - state.parse_sigs.push_back(sig); - state.vals.push_back(0); - state.all_vals.push_back({}); - break; - } - } - } + // track all signals for this message + state.parse_sigs = msg->sigs; + state.vals.resize(msg->sigs.size()); + state.all_vals.resize(msg->sigs.size()); } } diff --git a/can/parser_pyx.pyx b/can/parser_pyx.pyx index f9cf87e04c..81d3e06ff4 100644 --- a/can/parser_pyx.pyx +++ b/can/parser_pyx.pyx @@ -2,14 +2,14 @@ # cython: c_string_encoding=ascii, language_level=3 from cython.operator cimport dereference as deref, preincrement as preinc +from libcpp.pair cimport pair from libcpp.string cimport string from libcpp.vector cimport vector from libcpp.unordered_set cimport unordered_set from libc.stdint cimport uint32_t -from libcpp.map cimport map from .common cimport CANParser as cpp_CANParser -from .common cimport SignalParseOptions, MessageParseOptions, dbc_lookup, SignalValue, DBC +from .common cimport dbc_lookup, SignalValue, DBC import numbers from collections import defaultdict @@ -19,7 +19,6 @@ cdef class CANParser: cdef: cpp_CANParser *can const DBC *dbc - map[uint32_t, string] address_to_msg_name vector[SignalValue] can_values cdef readonly: @@ -28,10 +27,7 @@ cdef class CANParser: dict ts_nanos string dbc_name - def __init__(self, dbc_name, signals, checks=None, bus=0, enforce_checks=True): - if checks is None: - checks = [] - + def __init__(self, dbc_name, messages, bus=0): self.dbc_name = dbc_name self.dbc = dbc_lookup(dbc_name) if not self.dbc: @@ -41,18 +37,15 @@ cdef class CANParser: self.vl_all = {} self.ts_nanos = {} msg_name_to_address = {} - msg_address_to_signals = {} + address_to_msg_name = {} for i in range(self.dbc[0].msgs.size()): msg = self.dbc[0].msgs[i] name = msg.name.decode("utf8") msg_name_to_address[name] = msg.address - msg_address_to_signals[msg.address] = set() - for sig in msg.sigs: - msg_address_to_signals[msg.address].add(sig.name.decode("utf8")) + address_to_msg_name[msg.address] = name - self.address_to_msg_name[msg.address] = name self.vl[msg.address] = {} self.vl[name] = self.vl[msg.address] self.vl_all[msg.address] = {} @@ -60,52 +53,16 @@ cdef class CANParser: self.ts_nanos[msg.address] = {} self.ts_nanos[name] = self.ts_nanos[msg.address] - # Convert message names into addresses - for i in range(len(signals)): - s = signals[i] - address = s[1] if isinstance(s[1], numbers.Number) else msg_name_to_address.get(s[1]) - if address not in msg_address_to_signals: - raise RuntimeError(f"could not find message {repr(s[1])} in DBC {self.dbc_name}") - if s[0] not in msg_address_to_signals[address]: - raise RuntimeError(f"could not find signal {repr(s[0])} in {repr(s[1])}, DBC {self.dbc_name}") - - signals[i] = (s[0], address) - - for i in range(len(checks)): - c = checks[i] - if not isinstance(c[0], numbers.Number): - if c[0] not in msg_name_to_address: - print(msg_name_to_address) - raise RuntimeError(f"could not find message {repr(c[0])} in DBC {self.dbc_name}") - c = (msg_name_to_address[c[0]], c[1]) - checks[i] = c - - if enforce_checks: - checked_addrs = {c[0] for c in checks} - signal_addrs = {s[1] for s in signals} - unchecked = signal_addrs - checked_addrs - if len(unchecked): - err_msg = ", ".join(f"{self.address_to_msg_name[addr].decode()} ({hex(addr)})" for addr in unchecked) - raise RuntimeError(f"Unchecked addrs: {err_msg}") - - cdef vector[SignalParseOptions] signal_options_v - cdef SignalParseOptions spo - for sig_name, sig_address in signals: - spo.address = sig_address - spo.name = sig_name - signal_options_v.push_back(spo) - - message_options = dict((address, 0) for _, address in signals) - message_options.update(dict(checks)) - - cdef vector[MessageParseOptions] message_options_v - cdef MessageParseOptions mpo - for msg_address, freq in message_options.items(): - mpo.address = msg_address - mpo.check_frequency = freq - message_options_v.push_back(mpo) - - self.can = new cpp_CANParser(bus, dbc_name, message_options_v, signal_options_v) + # Convert message names into addresses and check existence in DBC + cdef vector[pair[uint32_t, int]] message_v + for i in range(len(messages)): + c = messages[i] + address = c[0] if isinstance(c[0], numbers.Number) else msg_name_to_address.get(c[0]) + if address not in address_to_msg_name: + raise RuntimeError(f"could not find message {repr(c[0])} in DBC {self.dbc_name}") + message_v.push_back((address, c[1])) + + self.can = new cpp_CANParser(bus, dbc_name, message_v) self.update_strings([]) def update_strings(self, strings, sendcan=False): diff --git a/can/tests/test_checksums.py b/can/tests/test_checksums.py index c95184b3c1..93ddbe0074 100755 --- a/can/tests/test_checksums.py +++ b/can/tests/test_checksums.py @@ -11,14 +11,8 @@ class TestCanChecksums(unittest.TestCase): def test_honda_checksum(self): """Test checksums for Honda standard and extended CAN ids""" dbc_file = "honda_accord_2018_can_generated" - - signals = [ - ("CHECKSUM", "LKAS_HUD"), - ("CHECKSUM", "LKAS_HUD_A"), - ] - checks = [("LKAS_HUD", 0), ("LKAS_HUD_A", 0)] - - parser = CANParser(dbc_file, signals, checks, 0) + msgs = [("LKAS_HUD", 0), ("LKAS_HUD_A", 0)] + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) values = { diff --git a/can/tests/test_dbc_exceptions.py b/can/tests/test_dbc_exceptions.py index b865270798..1f13f53927 100755 --- a/can/tests/test_dbc_exceptions.py +++ b/can/tests/test_dbc_exceptions.py @@ -11,13 +11,9 @@ class TestCanParserPackerExceptions(unittest.TestCase): def test_civic_exceptions(self): dbc_file = "honda_civic_touring_2016_can_generated" dbc_invalid = dbc_file + "abcdef" - signals = [ - ("STEER_TORQUE", "STEERING_CONTROL"), - ("STEER_TORQUE_REQUEST", "STEERING_CONTROL"), - ] - checks = [("STEERING_CONTROL", 50)] + msgs = [("STEERING_CONTROL", 50)] with self.assertRaises(RuntimeError): - CANParser(dbc_invalid, signals, checks, 0) + CANParser(dbc_invalid, msgs, 0) with self.assertRaises(RuntimeError): CANPacker(dbc_invalid) with self.assertRaises(RuntimeError): @@ -25,15 +21,13 @@ def test_civic_exceptions(self): with self.assertRaises(KeyError): CANDefine(TEST_DBC) - with self.assertRaises(RuntimeError): - CANParser(dbc_file, signals, [], 0) - - parser = CANParser(dbc_file, signals, checks, 0) + parser = CANParser(dbc_file, msgs, 0) with self.assertRaises(RuntimeError): parser.update_strings([b'']) # Everything is supposed to work below - CANParser(dbc_file, signals, checks, 0) + CANParser(dbc_file, msgs, 0) + CANParser(dbc_file, [], 0) CANPacker(dbc_file) CANDefine(dbc_file) diff --git a/can/tests/test_dbc_parser.py b/can/tests/test_dbc_parser.py index 524f2b74e6..5da41d49b7 100755 --- a/can/tests/test_dbc_parser.py +++ b/can/tests/test_dbc_parser.py @@ -21,7 +21,7 @@ def test_parse_all_dbcs(self): for dbc in ALL_DBCS: with self.subTest(dbc=dbc): - CANParser(dbc, [], [], 0) + CANParser(dbc, [], 0) if __name__ == "__main__": diff --git a/can/tests/test_packer_parser.py b/can/tests/test_packer_parser.py index 1b7399c40f..fc1898a332 100755 --- a/can/tests/test_packer_parser.py +++ b/can/tests/test_packer_parser.py @@ -43,12 +43,9 @@ def test_packer(self): self.assertEqual(dat[0], i) def test_packer_counter(self): - signals = [ - ("COUNTER", "CAN_FD_MESSAGE"), - ] - checks = [("CAN_FD_MESSAGE", 0), ] + msgs = [("CAN_FD_MESSAGE", 0), ] packer = CANPacker(TEST_DBC) - parser = CANParser(TEST_DBC, signals, checks, 0) + parser = CANParser(TEST_DBC, msgs, 0) # packer should increment the counter for i in range(1000): @@ -76,12 +73,9 @@ def test_packer_counter(self): self.assertEqual(parser.vl["CAN_FD_MESSAGE"]["COUNTER"], (cnt + i) % 256) def test_parser_can_valid(self): - signals = [ - ("COUNTER", "CAN_FD_MESSAGE"), - ] - checks = [("CAN_FD_MESSAGE", 10), ] + msgs = [("CAN_FD_MESSAGE", 10), ] packer = CANPacker(TEST_DBC) - parser = CANParser(TEST_DBC, signals, checks, 0) + parser = CANParser(TEST_DBC, msgs, 0) # shouldn't be valid initially self.assertFalse(parser.can_valid) @@ -101,23 +95,13 @@ def test_parser_can_valid(self): self.assertTrue(parser.can_valid) def test_packer_parser(self): - - signals = [ - ("COUNTER", "STEERING_CONTROL"), - ("CHECKSUM", "STEERING_CONTROL"), - ("STEER_TORQUE", "STEERING_CONTROL"), - ("STEER_TORQUE_REQUEST", "STEERING_CONTROL"), - - ("Signal1", "Brake_Status"), - - ("COUNTER", "CAN_FD_MESSAGE"), - ("64_BIT_LE", "CAN_FD_MESSAGE"), - ("64_BIT_BE", "CAN_FD_MESSAGE"), - ("SIGNED", "CAN_FD_MESSAGE"), + msgs = [ + ("Brake_Status", 0), + ("CAN_FD_MESSAGE", 0), + ("STEERING_CONTROL", 0), ] - packer = CANPacker(TEST_DBC) - parser = CANParser(TEST_DBC, signals, [], 0, enforce_checks=False) + parser = CANParser(TEST_DBC, msgs, 0) for steer in range(-256, 255): for active in (1, 0): @@ -151,13 +135,8 @@ def test_packer_parser(self): def test_scale_offset(self): """Test that both scale and offset are correctly preserved""" dbc_file = "honda_civic_touring_2016_can_generated" - - signals = [ - ("USER_BRAKE", "VSA_STATUS"), - ] - checks = [("VSA_STATUS", 50)] - - parser = CANParser(dbc_file, signals, checks, 0) + msgs = [("VSA_STATUS", 50)] + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) for brake in range(0, 100): @@ -174,15 +153,9 @@ def test_subaru(self): dbc_file = "subaru_global_2017_generated" - signals = [ - ("COUNTER", "ES_LKAS"), - ("LKAS_Output", "ES_LKAS"), - ("LKAS_Request", "ES_LKAS"), - ("SET_1", "ES_LKAS"), - ] - checks = [("ES_LKAS", 50)] + msgs = [("ES_LKAS", 50)] - parser = CANParser(dbc_file, signals, checks, 0) + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) idx = 0 @@ -209,9 +182,9 @@ def test_bus_timeout(self): dbc_file = "honda_civic_touring_2016_can_generated" freq = 100 - checks = [("VSA_STATUS", freq), ("STEER_MOTOR_TORQUE", freq/2)] + msgs = [("VSA_STATUS", freq), ("STEER_MOTOR_TORQUE", freq/2)] - parser = CANParser(dbc_file, [], checks, 0) + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) i = 0 @@ -245,11 +218,8 @@ def send_msg(blank=False): def test_updated(self): """Test updated value dict""" dbc_file = "honda_civic_touring_2016_can_generated" - - signals = [("USER_BRAKE", "VSA_STATUS")] - checks = [("VSA_STATUS", 50)] - - parser = CANParser(dbc_file, signals, checks, 0) + msgs = [("VSA_STATUS", 50)] + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) # Make sure nothing is updated @@ -279,16 +249,12 @@ def test_timestamp_nanos(self): """Test message timestamp dict""" dbc_file = "honda_civic_touring_2016_can_generated" - signals = [ - ("USER_BRAKE", "VSA_STATUS"), - ("PEDAL_GAS", "POWERTRAIN_DATA"), - ] - checks = [ + msgs = [ ("VSA_STATUS", 50), ("POWERTRAIN_DATA", 100), ] - parser = CANParser(dbc_file, signals, checks, 0) + parser = CANParser(dbc_file, msgs, 0) packer = CANPacker(dbc_file) # Check the default timestamp is zero @@ -314,23 +280,34 @@ def test_timestamp_nanos(self): ts_nanos = parser.ts_nanos["POWERTRAIN_DATA"].values() self.assertEqual(set(ts_nanos), {0}) - def test_undefined_signals(self): - # Ensure we don't allow messages or signals not in the DBC - existing_signals = { - "STEERING_CONTROL": ["STEER_TORQUE_REQUEST", "SET_ME_X00_2", "COUNTER"], - 228: ["STEER_TORQUE_REQUEST", "SET_ME_X00_2", "COUNTER"], - "CAN_FD_MESSAGE": ["SIGNED", "64_BIT_LE", "64_BIT_BE", "COUNTER"], - 245: ["SIGNED", "64_BIT_LE", "64_BIT_BE", "COUNTER"], - } - - for msg, sigs in existing_signals.items(): - for sig in sigs: - CANParser(TEST_DBC, [(sig, msg)], [(msg, 0)]) + def test_nonexistent_messages(self): + # Ensure we don't allow messages not in the DBC + existing_messages = ("STEERING_CONTROL", 228, "CAN_FD_MESSAGE", 245) + + for msg in existing_messages: + CANParser(TEST_DBC, [(msg, 0)]) + with self.assertRaises(RuntimeError): new_msg = msg + "1" if isinstance(msg, str) else msg + 1 - self.assertRaises(RuntimeError, CANParser, TEST_DBC, [(sig + "1", msg)], [(msg, 0)]) - self.assertRaises(RuntimeError, CANParser, TEST_DBC, [(sig, new_msg)], [(msg, 0)]) - self.assertRaises(RuntimeError, CANParser, TEST_DBC, [(sig, msg)], [(new_msg, 0)]) - self.assertRaises(RuntimeError, CANParser, TEST_DBC, [(sig, new_msg)], [(new_msg, 0)]) + CANParser(TEST_DBC, [(new_msg, 0)]) + + def test_track_all_signals(self): + parser = CANParser("toyota_nodsu_pt_generated", [("ACC_CONTROL", 0)]) + self.assertEqual(parser.vl["ACC_CONTROL"], { + "ACCEL_CMD": 0, + "ALLOW_LONG_PRESS": 0, + "ACC_MALFUNCTION": 0, + "RADAR_DIRTY": 0, + "DISTANCE": 0, + "MINI_CAR": 0, + "ACC_TYPE": 0, + "CANCEL_REQ": 0, + "ACC_CUT_IN": 0, + "PERMIT_BRAKING": 0, + "RELEASE_STANDSTILL": 0, + "ITS_CONNECT_LEAD": 0, + "ACCEL_CMD_ALT": 0, + "CHECKSUM": 0, + }) if __name__ == "__main__": diff --git a/can/tests/test_parser_performance.py b/can/tests/test_parser_performance.py index 4fef0099ad..2010fa4bf7 100755 --- a/can/tests/test_parser_performance.py +++ b/can/tests/test_parser_performance.py @@ -9,8 +9,8 @@ @unittest.skip("TODO: varies too much between machines") class TestParser(unittest.TestCase): - def _benchmark(self, signals, checks, thresholds, n): - parser = CANParser('toyota_new_mc_pt_generated', signals, checks, 0, False) + def _benchmark(self, checks, thresholds, n): + parser = CANParser('toyota_new_mc_pt_generated', checks, 0) packer = CANPacker('toyota_new_mc_pt_generated') can_msgs = [] @@ -46,31 +46,9 @@ def _benchmark(self, signals, checks, thresholds, n): self.assertLess(avg_nanos, maxx) self.assertGreater(avg_nanos, minn, "Performance seems to have improved, update test thresholds.") - def test_performance_one_signal(self): - signals = [ - ("ACCEL_CMD", "ACC_CONTROL"), - ] - self._benchmark(signals, [('ACC_CONTROL', 10)], (4000, 18000), 1) - self._benchmark(signals, [('ACC_CONTROL', 10)], (700, 3000), 10) - def test_performance_all_signals(self): - signals = [ - ("ACCEL_CMD", "ACC_CONTROL"), - ("ALLOW_LONG_PRESS", "ACC_CONTROL"), - ("ACC_MALFUNCTION", "ACC_CONTROL"), - ("RADAR_DIRTY", "ACC_CONTROL"), - ("DISTANCE", "ACC_CONTROL"), - ("MINI_CAR", "ACC_CONTROL"), - ("CANCEL_REQ", "ACC_CONTROL"), - ("ACC_CUT_IN", "ACC_CONTROL"), - ("PERMIT_BRAKING", "ACC_CONTROL"), - ("RELEASE_STANDSTILL", "ACC_CONTROL"), - ("ITS_CONNECT_LEAD", "ACC_CONTROL"), - ("ACCEL_CMD_ALT", "ACC_CONTROL"), - ("CHECKSUM", "ACC_CONTROL"), - ] - self._benchmark(signals, [('ACC_CONTROL', 10)], (10000, 19000), 1) - self._benchmark(signals, [('ACC_CONTROL', 10)], (1300, 5000), 10) + self._benchmark([('ACC_CONTROL', 10)], (10000, 19000), 1) + self._benchmark([('ACC_CONTROL', 10)], (1300, 5000), 10) if __name__ == "__main__":