From 39a5345924d2414f05c40d258d359eb8412a0b03 Mon Sep 17 00:00:00 2001 From: Dean Lee Date: Wed, 31 Jul 2024 15:10:20 +0800 Subject: [PATCH] CANParser: remove dependence on cereal (#934) * move car interfaces to opendbc * Revert "move car interfaces to opendbc" This reverts commit 0589ea0dbec5e3d25ed4498257eb162d656780bb. * remove dependence on cereal * parse can strings in a c++ helper function * remove all cereal * reserve * use itervalues * rebase master * cleanup * keep the same style * revert unrelated changes * truly no cereal! * same capitalization same capitalization * parser: declutter nanos usage (#1067) thought there'd be more * revert more --------- Co-authored-by: Shane Smiskol --- Dockerfile | 5 -- SConstruct | 9 --- can/SConscript | 4 +- can/common.h | 32 +++++----- can/common.pxd | 11 +++- can/parser.cc | 92 +++++++--------------------- can/parser_pyx.pyx | 31 +++++++++- can/tests/test_checksums.py | 4 +- can/tests/test_packer_parser.py | 63 +++++-------------- can/tests/test_parser_performance.py | 4 +- 10 files changed, 93 insertions(+), 162 deletions(-) diff --git a/Dockerfile b/Dockerfile index df3cb56b8d..f11ed990b1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -44,11 +44,6 @@ ENV PYTHONPATH=/project RUN git config --global --add safe.directory '*' WORKDIR /project -RUN git clone https://github.com/commaai/cereal.git /project/cereal && \ - cd /project/cereal && \ - git checkout 861144c136c91f70dcbc652c2ffe99f57440ad47 && \ - rm -rf .git && \ - scons -j$(nproc) --minimal COPY SConstruct . COPY ./site_scons /project/site_scons diff --git a/SConstruct b/SConstruct index 34e3061e78..2401720f17 100644 --- a/SConstruct +++ b/SConstruct @@ -6,8 +6,6 @@ import numpy as np zmq = 'zmq' arch = subprocess.check_output(["uname", "-m"], encoding='utf8').rstrip() -cereal_dir = Dir('.') - python_path = sysconfig.get_paths()['include'] cpppath = [ '#', @@ -56,11 +54,6 @@ env = Environment( common = '' Export('env', 'zmq', 'arch', 'common') -cereal = [File('#cereal/libcereal.a')] -messaging = [File('#cereal/libmessaging.a')] -Export('cereal', 'messaging') - - envCython = env.Clone() envCython["CPPPATH"] += [np.get_include()] envCython["CCFLAGS"] += ["-Wno-#warnings", "-Wno-shadow", "-Wno-deprecated-declarations"] @@ -80,6 +73,4 @@ envCython["LIBS"] = python_libs Export('envCython') - -SConscript(['cereal/SConscript']) SConscript(['opendbc/can/SConscript']) diff --git a/can/SConscript b/can/SConscript index 63654ac863..baa9537632 100644 --- a/can/SConscript +++ b/can/SConscript @@ -1,4 +1,4 @@ -Import('env', 'envCython', 'cereal', 'common', 'arch') +Import('env', 'envCython', 'common', 'arch') import os @@ -6,7 +6,7 @@ envDBC = env.Clone() dbc_file_path = '-DDBC_FILE_PATH=\'"%s"\'' % (envDBC.Dir("..").abspath) envDBC['CXXFLAGS'] += [dbc_file_path] src = ["dbc.cc", "parser.cc", "packer.cc", "common.cc"] -libs = [common, "capnp", "kj", "zmq"] +libs = [common, "zmq"] # shared library for openpilot LINKFLAGS = envDBC["LINKFLAGS"] diff --git a/can/common.h b/can/common.h index 46b3159b9f..8b5b8d320d 100644 --- a/can/common.h +++ b/can/common.h @@ -6,13 +6,6 @@ #include #include -#include -#include - -#ifndef DYNAMIC_CAPNP -#include "cereal/gen/cpp/log.capnp.h" -#endif - #include "opendbc/can/logger.h" #include "opendbc/can/common_dbc.h" @@ -34,6 +27,17 @@ unsigned int xor_checksum(uint32_t address, const Signal &sig, const std::vector unsigned int hkg_can_fd_checksum(uint32_t address, const Signal &sig, const std::vector &d); unsigned int pedal_checksum(uint32_t address, const Signal &sig, const std::vector &d); +struct CanFrame { + long src; + uint32_t address; + std::vector dat; +}; + +struct CanData { + uint64_t nanos; + std::vector frames; +}; + class MessageState { public: std::string name; @@ -60,8 +64,6 @@ class MessageState { class CANParser { private: const int bus; - kj::Array aligned_buf; - const DBC *dbc = NULL; std::unordered_map message_states; @@ -77,14 +79,12 @@ class CANParser { CANParser(int abus, const std::string& dbc_name, 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); - void update_strings(const std::vector &data, std::vector &vals, bool sendcan); - void UpdateCans(uint64_t nanos, const capnp::List::Reader& cans); - #endif - void UpdateCans(uint64_t nanos, const capnp::DynamicStruct::Reader& cans); - void UpdateValid(uint64_t nanos); + void update(const std::vector &can_data, std::vector &vals); void query_latest(std::vector &vals, uint64_t last_ts = 0); + +protected: + void UpdateCans(const CanData &can); + void UpdateValid(uint64_t nanos); }; class CANPacker { diff --git a/can/common.pxd b/can/common.pxd index be9c430843..21e276fa07 100644 --- a/can/common.pxd +++ b/can/common.pxd @@ -67,11 +67,20 @@ cdef extern from "common_dbc.h": cdef extern from "common.h": cdef const DBC* dbc_lookup(const string) except + + cdef struct CanFrame: + long src + uint32_t address + vector[uint8_t] dat + + cdef struct CanData: + uint64_t nanos + vector[CanFrame] frames + cdef cppclass CANParser: bool can_valid bool bus_timeout CANParser(int, string, vector[pair[uint32_t, int]]) except + - void update_strings(vector[string]&, vector[SignalValue]&, bool) except + + void update(vector[CanData]&, vector[SignalValue]&) except + cdef cppclass CANPacker: CANPacker(string) diff --git a/can/parser.cc b/can/parser.cc index 3cd9afabc7..a65a8ec2d6 100644 --- a/can/parser.cc +++ b/can/parser.cc @@ -93,7 +93,7 @@ bool MessageState::update_counter_generic(int64_t v, int cnt_size) { CANParser::CANParser(int abus, const std::string& dbc_name, const std::vector> &messages) - : bus(abus), aligned_buf(kj::heapArray(1024)) { + : bus(abus) { dbc = dbc_lookup(dbc_name); assert(dbc); @@ -157,64 +157,42 @@ CANParser::CANParser(int abus, const std::string& dbc_name, bool ignore_checksum } } -#ifndef DYNAMIC_CAPNP -void CANParser::update_string(const std::string &data, bool sendcan) { - // format for board, make copy due to alignment issues. - const size_t buf_size = (data.length() / sizeof(capnp::word)) + 1; - if (aligned_buf.size() < buf_size) { - aligned_buf = kj::heapArray(buf_size); - } - memcpy(aligned_buf.begin(), data.data(), data.length()); - - // extract the messages - capnp::FlatArrayMessageReader cmsg(aligned_buf.slice(0, buf_size)); - cereal::Event::Reader event = cmsg.getRoot(); - - if (first_nanos == 0) { - first_nanos = event.getLogMonoTime(); - } - last_nanos = event.getLogMonoTime(); - - auto cans = sendcan ? event.getSendcan() : event.getCan(); - UpdateCans(last_nanos, cans); - - UpdateValid(last_nanos); -} - -void CANParser::update_strings(const std::vector &data, std::vector &vals, bool sendcan) { +void CANParser::update(const std::vector &can_data, std::vector &vals) { uint64_t current_nanos = 0; - for (const auto &d : data) { - update_string(d, sendcan); + for (const auto &c : can_data) { + if (first_nanos == 0) { + first_nanos = c.nanos; + } if (current_nanos == 0) { - current_nanos = last_nanos; + current_nanos = c.nanos; } + last_nanos = c.nanos; + + UpdateCans(c); + UpdateValid(last_nanos); } query_latest(vals, current_nanos); } -void CANParser::UpdateCans(uint64_t nanos, const capnp::List::Reader& cans) { - //DEBUG("got %d messages\n", cans.size()); +void CANParser::UpdateCans(const CanData &can) { + //DEBUG("got %zu messages\n", can.frames.size()); bool bus_empty = true; - // parse the messages - for (const auto cmsg : cans) { - if (cmsg.getSrc() != bus) { + for (const auto &frame : can.frames) { + if (frame.src != bus) { // DEBUG("skip %d: wrong bus\n", cmsg.getAddress()); continue; } bus_empty = false; - auto state_it = message_states.find(cmsg.getAddress()); + auto state_it = message_states.find(frame.address); if (state_it == message_states.end()) { // DEBUG("skip %d: not specified\n", cmsg.getAddress()); continue; } - - auto dat = cmsg.getDat(); - - if (dat.size() > 64) { - DEBUG("got message longer than 64 bytes: 0x%X %zu\n", cmsg.getAddress(), dat.size()); + if (frame.dat.size() > 64) { + DEBUG("got message longer than 64 bytes: 0x%X %zu\n", frame.address, frame.dat.size()); continue; } @@ -224,42 +202,14 @@ void CANParser::UpdateCans(uint64_t nanos, const capnp::List::R // continue; //} - // TODO: can remove when we ignore unexpected can msg lengths - // make sure the data_size is not less than state_it->second.size - size_t data_size = std::max(dat.size(), state_it->second.size); - std::vector data(data_size, 0); - memcpy(data.data(), dat.begin(), dat.size()); - state_it->second.parse(nanos, data); + state_it->second.parse(can.nanos, frame.dat); } // update bus timeout if (!bus_empty) { - last_nonempty_nanos = nanos; - } - bus_timeout = (nanos - last_nonempty_nanos) > bus_timeout_threshold; -} -#endif - -void CANParser::UpdateCans(uint64_t nanos, const capnp::DynamicStruct::Reader& cmsg) { - // assume message struct is `cereal::CanData` and parse - assert(cmsg.has("address") && cmsg.has("src") && cmsg.has("dat")); - - if (cmsg.get("src").as() != bus) { - DEBUG("skip %d: wrong bus\n", cmsg.get("address").as()); - return; + last_nonempty_nanos = can.nanos; } - - auto state_it = message_states.find(cmsg.get("address").as()); - if (state_it == message_states.end()) { - DEBUG("skip %d: not specified\n", cmsg.get("address").as()); - return; - } - - auto dat = cmsg.get("dat").as(); - if (dat.size() > 64) return; // shouldn't ever happen - std::vector data(dat.size(), 0); - memcpy(data.data(), dat.begin(), dat.size()); - state_it->second.parse(nanos, data); + bus_timeout = (can.nanos - last_nonempty_nanos) > bus_timeout_threshold; } void CANParser::UpdateValid(uint64_t nanos) { diff --git a/can/parser_pyx.pyx b/can/parser_pyx.pyx index a1feca2cb2..9fb0c9f021 100644 --- a/can/parser_pyx.pyx +++ b/can/parser_pyx.pyx @@ -8,7 +8,7 @@ from libcpp.vector cimport vector from libc.stdint cimport uint32_t from .common cimport CANParser as cpp_CANParser -from .common cimport dbc_lookup, SignalValue, DBC +from .common cimport dbc_lookup, SignalValue, DBC, CanData, CanFrame import numbers from collections import defaultdict @@ -65,17 +65,42 @@ cdef class CANParser: del self.can def update_strings(self, strings, sendcan=False): + # input format: + # [nanos, [[address, data, src], ...]] + # [[nanos, [[address, data, src], ...], ...]] for address in self.addresses: self.vl_all[address].clear() - cdef vector[SignalValue] new_vals cur_address = -1 vl = {} vl_all = {} ts_nanos = {} updated_addrs = set() - self.can.update_strings(strings, new_vals, sendcan) + cdef vector[SignalValue] new_vals + cdef CanFrame* frame + cdef CanData* can_data + cdef vector[CanData] can_data_array + + try: + if len(strings) and not isinstance(strings[0], (list, tuple)): + strings = [strings] + + can_data_array.reserve(len(strings)) + for s in strings: + can_data = &(can_data_array.emplace_back()) + can_data.nanos = s[0] + can_data.frames.reserve(len(s[1])) + for f in s[1]: + frame = &(can_data.frames.emplace_back()) + frame.address = f[0] + frame.dat = f[1] + frame.src = f[2] + except TypeError: + raise RuntimeError("invalid parameter") + + self.can.update(can_data_array, new_vals) + cdef vector[SignalValue].iterator it = new_vals.begin() cdef SignalValue* cv while it != new_vals.end(): diff --git a/can/tests/test_checksums.py b/can/tests/test_checksums.py index 25eadd64cd..c347f3a68b 100644 --- a/can/tests/test_checksums.py +++ b/can/tests/test_checksums.py @@ -1,6 +1,5 @@ from opendbc.can.parser import CANParser from opendbc.can.packer import CANPacker -from opendbc.can.tests.test_packer_parser import can_list_to_can_capnp class TestCanChecksums: @@ -28,8 +27,7 @@ def test_honda_checksum(self): packer.make_can_msg("LKAS_HUD", 0, values), packer.make_can_msg("LKAS_HUD_A", 0, values), ] - can_strings = [can_list_to_can_capnp(msgs), ] - parser.update_strings(can_strings) + parser.update_strings([0, msgs]) assert parser.vl['LKAS_HUD']['CHECKSUM'] == std assert parser.vl['LKAS_HUD_A']['CHECKSUM'] == ext diff --git a/can/tests/test_packer_parser.py b/can/tests/test_packer_parser.py index 4961d46d0e..acb5acebe9 100644 --- a/can/tests/test_packer_parser.py +++ b/can/tests/test_packer_parser.py @@ -1,7 +1,6 @@ import pytest import random -import cereal.messaging as messaging from opendbc.can.parser import CANParser from opendbc.can.packer import CANPacker from opendbc.can.tests import TEST_DBC @@ -9,26 +8,6 @@ MAX_BAD_COUNTER = 5 -# Python implementation so we don't have to depend on boardd -def can_list_to_can_capnp(can_msgs, msgtype='can', logMonoTime=None): - dat = messaging.new_message(msgtype, len(can_msgs)) - - if logMonoTime is not None: - dat.logMonoTime = logMonoTime - - for i, can_msg in enumerate(can_msgs): - if msgtype == 'sendcan': - cc = dat.sendcan[i] - else: - cc = dat.can[i] - - cc.address = can_msg[0] - cc.dat = bytes(can_msg[1]) - cc.src = can_msg[2] - - return dat.to_bytes() - - class TestCanParserPacker: def test_packer(self): packer = CANPacker(TEST_DBC) @@ -49,8 +28,7 @@ def test_packer_counter(self): # packer should increment the counter for i in range(1000): msg = packer.make_can_msg("CAN_FD_MESSAGE", 0, {}) - dat = can_list_to_can_capnp([msg, ]) - parser.update_strings([dat]) + parser.update_strings([0, [msg]]) assert parser.vl["CAN_FD_MESSAGE"]["COUNTER"] == (i % 256) # setting COUNTER should override @@ -60,16 +38,14 @@ def test_packer_counter(self): "COUNTER": cnt, "SIGNED": 0 }) - dat = can_list_to_can_capnp([msg, ]) - parser.update_strings([dat]) + parser.update_strings([0, [msg]]) assert parser.vl["CAN_FD_MESSAGE"]["COUNTER"] == cnt # then, should resume counting from the override value cnt = parser.vl["CAN_FD_MESSAGE"]["COUNTER"] for i in range(100): msg = packer.make_can_msg("CAN_FD_MESSAGE", 0, {}) - dat = can_list_to_can_capnp([msg, ]) - parser.update_strings([dat]) + parser.update_strings([0, [msg]]) assert parser.vl["CAN_FD_MESSAGE"]["COUNTER"] == ((cnt + i) % 256) def test_parser_can_valid(self): @@ -82,16 +58,14 @@ def test_parser_can_valid(self): # not valid until the message is seen for _ in range(100): - dat = can_list_to_can_capnp([]) - parser.update_strings([dat]) + parser.update_strings([0, []]) assert not parser.can_valid # valid once seen for i in range(1, 100): t = int(0.01 * i * 1e9) msg = packer.make_can_msg("CAN_FD_MESSAGE", 0, {}) - dat = can_list_to_can_capnp([msg, ], logMonoTime=t) - parser.update_strings([dat]) + parser.update_strings([t, [msg]]) assert parser.can_valid def test_parser_counter_can_valid(self): @@ -106,17 +80,15 @@ def test_parser_counter_can_valid(self): parser = CANParser("honda_civic_touring_2016_can_generated", msgs, 0) msg = packer.make_can_msg("STEERING_CONTROL", 0, {"COUNTER": 0}) - bts = can_list_to_can_capnp([msg]) # bad static counter, invalid once it's seen MAX_BAD_COUNTER messages for idx in range(0x1000): - parser.update_strings([bts]) + parser.update_strings([0, [msg]]) assert ((idx + 1) < MAX_BAD_COUNTER) == parser.can_valid # one to recover msg = packer.make_can_msg("STEERING_CONTROL", 0, {"COUNTER": 1}) - bts = can_list_to_can_capnp([msg]) - parser.update_strings([bts]) + parser.update_strings([0, [msg]]) assert parser.can_valid def test_parser_no_partial_update(self): @@ -138,8 +110,7 @@ def rx_steering_msg(values, bad_checksum=False): msg[1] = bytearray(msg[1]) msg[1][4] = (msg[1][4] & 0xF0) | ((msg[1][4] & 0x0F) + 1) - bts = can_list_to_can_capnp([msg]) - parser.update_strings([bts]) + parser.update_strings([0, [msg]]) rx_steering_msg({"STEER_TORQUE": 100}, bad_checksum=False) assert parser.vl["STEERING_CONTROL"]["STEER_TORQUE"] == 100 @@ -182,8 +153,7 @@ def test_packer_parser(self): } msgs = [packer.make_can_msg(k, 0, v) for k, v in values.items()] - bts = can_list_to_can_capnp(msgs) - parser.update_strings([bts]) + parser.update_strings([0, msgs]) for k, v in values.items(): for key, val in v.items(): @@ -203,9 +173,7 @@ def test_scale_offset(self): for brake in range(100): values = {"USER_BRAKE": brake} msgs = packer.make_can_msg("VSA_STATUS", 0, values) - bts = can_list_to_can_capnp([msgs]) - - parser.update_strings([bts]) + parser.update_strings([0, [msgs]]) assert parser.vl["VSA_STATUS"]["USER_BRAKE"] == pytest.approx(brake) @@ -229,8 +197,7 @@ def test_subaru(self): } msgs = packer.make_can_msg("ES_LKAS", 0, values) - bts = can_list_to_can_capnp([msgs]) - parser.update_strings([bts]) + parser.update_strings([0, [msgs]]) assert parser.vl["ES_LKAS"]["LKAS_Output"] == pytest.approx(steer) assert parser.vl["ES_LKAS"]["LKAS_Request"] == pytest.approx(active) @@ -259,8 +226,7 @@ def send_msg(blank=False): else: msgs = [packer.make_can_msg("VSA_STATUS", 0, {}), ] - can = can_list_to_can_capnp(msgs, logMonoTime=t) - parser.update_strings([can, ]) + parser.update_strings([t, msgs]) # all good, no timeout for _ in range(1000): @@ -298,8 +264,7 @@ def test_updated(self): can_msgs[frame].append(packer.make_can_msg("VSA_STATUS", 0, values)) idx += 1 - can_strings = [can_list_to_can_capnp(msgs) for msgs in can_msgs] - parser.update_strings(can_strings) + parser.update_strings([[0, m] for m in can_msgs]) vl_all = parser.vl_all["VSA_STATUS"]["USER_BRAKE"] assert vl_all == user_brake_vals @@ -333,7 +298,7 @@ def test_timestamp_nanos(self): for i in range(10): log_mono_time = int(0.01 * i * 1e+9) can_msg = packer.make_can_msg("VSA_STATUS", 0, {}) - can_strings.append(can_list_to_can_capnp([can_msg], logMonoTime=log_mono_time)) + can_strings.append((log_mono_time, [can_msg])) parser.update_strings(can_strings) ts_nanos = parser.ts_nanos["VSA_STATUS"].values() diff --git a/can/tests/test_parser_performance.py b/can/tests/test_parser_performance.py index fe0392b825..c4ca5a3c82 100644 --- a/can/tests/test_parser_performance.py +++ b/can/tests/test_parser_performance.py @@ -3,7 +3,6 @@ from opendbc.can.parser import CANParser from opendbc.can.packer import CANPacker -from opendbc.can.tests.test_packer_parser import can_list_to_can_capnp @pytest.mark.skip("TODO: varies too much between machines") @@ -16,8 +15,7 @@ def _benchmark(self, checks, thresholds, n): for i in range(50000): values = {"ACC_CONTROL": {"ACC_TYPE": 1, "ALLOW_LONG_PRESS": 3}} msgs = [packer.make_can_msg(k, 0, v) for k, v in values.items()] - bts = can_list_to_can_capnp(msgs, logMonoTime=int(0.01 * i * 1e9)) - can_msgs.append(bts) + can_msgs.append([int(0.01 * i * 1e9), msgs]) ets = [] for _ in range(25):