Merge remote-tracking branch 'upstream/master' into commathree_master

.github/workflows/tests.yml

@@ -3,7 +3,7 @@ name: tests
 on: [push, pull_request]
 
 env:
-  RUN: docker run -v $GITHUB_WORKSPACE:/project/opendbc -w /project/opendbc -e PYTHONWARNINGS="error,default::DeprecationWarning" --shm-size 1G --rm opendbc /bin/bash -c
+  RUN: docker run -e PYTHONWARNINGS="error,default::DeprecationWarning" --shm-size 1G --rm opendbc /bin/bash -c
   BUILD: docker buildx build --pull --load --cache-to type=inline --cache-from type=registry,ref=ghcr.io/commaai/opendbc:latest -t opendbc -f Dockerfile .
   PYTHONWARNINGS: error
 
@@ -19,10 +19,8 @@ jobs:
     - uses: actions/checkout@v4
     - name: Build Docker image
       run: eval "$BUILD"
-    - name: Build opendbc
-      run: ${{ env.RUN }} "cd ../ && scons -j$(nproc) --minimal"
     - name: Unit tests
-      run: ${{ env.RUN }} "pytest -n logical"
+      run: ${{ env.RUN }} "pytest -n logical --durations=0"
 
   static-analysis:
     name: static analysis
@@ -31,13 +29,9 @@ jobs:
     - uses: actions/checkout@v4
     - name: Build Docker image
       run: eval "$BUILD"
-    - name: Build opendbc
-      run: ${{ env.RUN }} "cd ../ && scons -j$(nproc) --minimal"
     - name: pre-commit
       # TODO: a package pre-commit installs has a warning, remove the unset once that's fixed
-      run: ${{ env.RUN }} "unset PYTHONWARNINGS && pre-commit run --all"
-    - name: Generator test
-      run: ${{ env.RUN }} "generator/test_generator.py"
+      run: ${{ env.RUN }} "git init && git add -A && unset PYTHONWARNINGS && pre-commit run --all"
 
   docker-push:
     name: docker push

.gitignore

@@ -8,11 +8,11 @@
 .DS_Store
 .sconsign.dblite
 
-can/*.so
-can/*.a
-can/build/
-can/obj/
-can/packer_pyx.cpp
-can/parser_pyx.cpp
-can/packer_pyx.html
-can/parser_pyx.html
+opendbc/can/*.so
+opendbc/can/*.a
+opendbc/can/build/
+opendbc/can/obj/
+opendbc/can/packer_pyx.cpp
+opendbc/can/parser_pyx.cpp
+opendbc/can/packer_pyx.html
+opendbc/can/parser_pyx.html

.pre-commit-config.yaml

@@ -56,6 +56,6 @@ repos:
     hooks:
     -   id: generator
         name: dbc generator
-        entry: generator/test_generator.py
+        entry: opendbc/dbc/generator/test_generator.py
         language: script
         pass_filenames: false

Dockerfile

@@ -39,16 +39,9 @@ COPY requirements.txt /tmp/
 RUN pip3 install --break-system-packages --no-cache-dir -r /tmp/requirements.txt
 RUN pip3 install --break-system-packages --no-cache-dir pre-commit==2.15.0 pylint==2.17.4
 
-ENV PYTHONPATH=/project
+WORKDIR /project/opendbc
+ENV PYTHONPATH=/project/opendbc
 
-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
+COPY . .
+RUN ls && rm -rf .git && \
+    scons -c && scons -j$(nproc) \

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'])

opendbc/__init__.py

@@ -0,0 +1,3 @@
+import os
+
+DBC_PATH = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'dbc')

can/SConscript → opendbc/can/SConscript

@@ -1,12 +1,12 @@
-Import('env', 'envCython', 'cereal', 'common', 'arch')
+Import('env', 'envCython', 'common', 'arch')
 
 import os
 
 envDBC = env.Clone()
-dbc_file_path = '-DDBC_FILE_PATH=\'"%s"\'' % (envDBC.Dir("..").abspath)
+dbc_file_path = '-DDBC_FILE_PATH=\'"%s"\'' % (envDBC.Dir("../dbc").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"]

can/common.h → opendbc/can/common.h

@@ -6,13 +6,6 @@
 #include <unordered_map>
 #include <vector>
 
-#include <capnp/dynamic.h>
-#include <capnp/serialize.h>
-
-#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<uint8_t> &d);
 unsigned int pedal_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
 
+struct CanFrame {
+  long src;
+  uint32_t address;
+  std::vector<uint8_t> dat;
+};
+
+struct CanData {
+  uint64_t nanos;
+  std::vector<CanFrame> frames;
+};
+
 class MessageState {
 public:
   std::string name;
@@ -60,8 +64,6 @@ class MessageState {
 class CANParser {
 private:
   const int bus;
-  kj::Array<capnp::word> aligned_buf;
-
   const DBC *dbc = NULL;
   std::unordered_map<uint32_t, MessageState> message_states;
 
@@ -77,14 +79,12 @@ class CANParser {
   CANParser(int abus, const std::string& dbc_name,
             const std::vector<std::pair<uint32_t, int>> &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<std::string> &data, std::vector<SignalValue> &vals, bool sendcan);
-  void UpdateCans(uint64_t nanos, const capnp::List<cereal::CanData>::Reader& cans);
-  #endif
-  void UpdateCans(uint64_t nanos, const capnp::DynamicStruct::Reader& cans);
-  void UpdateValid(uint64_t nanos);
+  void update(const std::vector<CanData> &can_data, std::vector<SignalValue> &vals);
   void query_latest(std::vector<SignalValue> &vals, uint64_t last_ts = 0);
+
+protected:
+  void UpdateCans(const CanData &can);
+  void UpdateValid(uint64_t nanos);
 };
 
 class CANPacker {

can/common.pxd → opendbc/can/common.pxd

@@ -1,7 +1,7 @@
 # distutils: language = c++
 # cython: language_level=3
 
-from libc.stdint cimport uint8_t, uint16_t, uint32_t, uint64_t
+from libc.stdint cimport uint8_t, uint32_t, uint64_t
 from libcpp cimport bool
 from libcpp.pair cimport pair
 from libcpp.string cimport string
@@ -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)

can/dbc.cc → opendbc/can/dbc.cc

@@ -222,7 +222,7 @@ DBC* dbc_parse(const std::string& dbc_path) {
 const std::string get_dbc_root_path() {
   char *basedir = std::getenv("BASEDIR");
   if (basedir != NULL) {
-    return std::string(basedir) + "/opendbc";
+    return std::string(basedir) + "/opendbc/dbc";
   } else {
     return DBC_FILE_PATH;
   }

can/packer_pyx.pyx → opendbc/can/packer_pyx.pyx

@@ -50,4 +50,4 @@ cdef class CANPacker:
         pass
 
     cdef vector[uint8_t] val = self.pack(addr, values)
-    return [addr, 0, (<char *>&val[0])[:val.size()], bus]
+    return [addr, (<char *>&val[0])[:val.size()], bus]

can/parser.cc → opendbc/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<std::pair<uint32_t, int>> &messages)
-  : bus(abus), aligned_buf(kj::heapArray<capnp::word>(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<capnp::word>(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<cereal::Event>();
-
-  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<std::string> &data, std::vector<SignalValue> &vals, bool sendcan) {
+void CANParser::update(const std::vector<CanData> &can_data, std::vector<SignalValue> &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<cereal::CanData>::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,46 +202,18 @@ void CANParser::UpdateCans(uint64_t nanos, const capnp::List<cereal::CanData>::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<size_t>(dat.size(), state_it->second.size);
-    std::vector<uint8_t> 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") && cmsg.has("busTime"));
-
-  if (cmsg.get("src").as<uint8_t>() != bus) {
-    DEBUG("skip %d: wrong bus\n", cmsg.get("address").as<uint32_t>());
-    return;
+    last_nonempty_nanos = can.nanos;
   }
-
-  auto state_it = message_states.find(cmsg.get("address").as<uint32_t>());
-  if (state_it == message_states.end()) {
-    DEBUG("skip %d: not specified\n", cmsg.get("address").as<uint32_t>());
-    return;
-  }
-
-  auto dat = cmsg.get("dat").as<capnp::Data>();
-  if (dat.size() > 64) return; // shouldn't ever happen
-  std::vector<uint8_t> 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) {
-  const bool show_missing = (last_nanos - first_nanos) > 8e9;
+  const bool show_missing = (nanos - first_nanos) > 8e9;
 
   bool _valid = true;
   bool _counters_valid = true;