Cleanup dead code in CAN parser + packer (#573)

* remove duplicate implementation

* Cleanup dead code in CAN parser + packer

* little more

can/SConscript

@@ -22,4 +22,4 @@ parser = lenv.Program('parser_pyx.so', 'parser_pyx.pyx')
 packer = lenv.Program('packer_pyx.so', 'packer_pyx.pyx')
 
 lenv.Depends(parser, libdbc)
-lenv.Depends(packer, libdbc)
+lenv.Depends(packer, libdbc)

can/common.h

@@ -12,6 +12,11 @@
 #include "cereal/gen/cpp/log.capnp.h"
 #endif
 
+#define INFO printf
+#define WARN printf
+#define DEBUG(...)
+// #define DEBUG printf
+
 #define MAX_BAD_COUNTER 5
 
 // Helper functions

can/common.pxd

@@ -1,5 +1,5 @@
 # distutils: language = c++
-#cython: language_level=3
+# cython: language_level=3
 
 from libc.stdint cimport uint32_t, uint64_t, uint16_t
 from libcpp.vector cimport vector

can/dbc.py

@@ -1,7 +1,6 @@
 #!/usr/bin/env python3
 import re
 import os
-import struct
 import sys
 import numbers
 from collections import namedtuple, defaultdict
@@ -14,9 +13,8 @@ def int_or_float(s):
     return float(s)
 
 
-DBCSignal = namedtuple(
-  "DBCSignal", ["name", "start_bit", "size", "is_little_endian", "is_signed",
-                "factor", "offset", "tmin", "tmax", "units"])
+DBCSignal = namedtuple("DBCSignal", ["name", "start_bit", "size", "is_little_endian", "is_signed",
+                                     "factor", "offset", "tmin", "tmax", "units"])
 
 
 class dbc():
@@ -122,136 +120,6 @@ class dbc():
       msg_id = self.msg_name_to_address[msg_id]
     return msg_id
 
-  def reverse_bytes(self, x):
-    return ((x & 0xff00000000000000) >> 56) | \
-           ((x & 0x00ff000000000000) >> 40) | \
-           ((x & 0x0000ff0000000000) >> 24) | \
-           ((x & 0x000000ff00000000) >> 8) | \
-           ((x & 0x00000000ff000000) << 8) | \
-           ((x & 0x0000000000ff0000) << 24) | \
-           ((x & 0x000000000000ff00) << 40) | \
-           ((x & 0x00000000000000ff) << 56)
-
-  def encode(self, msg_id, dd):
-    """Encode a CAN message using the dbc.
-
-       Inputs:
-        msg_id: The message ID.
-        dd: A dictionary mapping signal name to signal data.
-    """
-    msg_id = self.lookup_msg_id(msg_id)
-
-    msg_def = self.msgs[msg_id]
-    size = msg_def[0][1]
-
-    result = 0
-    for s in msg_def[1]:
-      ival = dd.get(s.name)
-      if ival is not None:
-
-        ival = (ival - s.offset) / s.factor
-        ival = int(round(ival))
-
-        if s.is_signed and ival < 0:
-          ival = (1 << s.size) + ival
-
-        if s.is_little_endian:
-          shift = s.start_bit
-        else:
-          b1 = (s.start_bit // 8) * 8 + (-s.start_bit - 1) % 8
-          shift = 64 - (b1 + s.size)
-
-        mask = ((1 << s.size) - 1) << shift
-        dat = (ival & ((1 << s.size) - 1)) << shift
-
-        if s.is_little_endian:
-          mask = self.reverse_bytes(mask)
-          dat = self.reverse_bytes(dat)
-
-        result &= ~mask
-        result |= dat
-
-    result = struct.pack('>Q', result)
-    return result[:size]
-
-  def decode(self, x, arr=None, debug=False):
-    """Decode a CAN message using the dbc.
-
-       Inputs:
-        x: A collection with elements (address, time, data), where address is
-           the CAN address, time is the bus time, and data is the CAN data as a
-           hex string.
-        arr: Optional list of signals which should be decoded and returned.
-        debug: True to print debugging statements.
-
-       Returns:
-        A tuple (name, data), where name is the name of the CAN message and data
-        is the decoded result. If arr is None, data is a dict of properties.
-        Otherwise data is a list of the same length as arr.
-
-        Returns (None, None) if the message could not be decoded.
-    """
-
-    if arr is None:
-      out = {}
-    else:
-      out = [None] * len(arr)
-
-    msg = self.msgs.get(x[0])
-    if msg is None:
-      if x[0] not in self._warned_addresses:
-        # print("WARNING: Unknown message address {}".format(x[0]))
-        self._warned_addresses.add(x[0])
-      return None, None
-
-    name = msg[0][0]
-    if debug:
-      print(name)
-
-    st = x[2].ljust(8, b'\x00')
-    le, be = None, None
-
-    for s in msg[1]:
-      if arr is not None and s[0] not in arr:
-        continue
-
-      start_bit = s[1]
-      signal_size = s[2]
-      little_endian = s[3]
-      signed = s[4]
-      factor = s[5]
-      offset = s[6]
-
-      if little_endian:
-        if le is None:
-          le = struct.unpack("<Q", st)[0]
-        tmp = le
-        shift_amount = start_bit
-      else:
-        if be is None:
-          be = struct.unpack(">Q", st)[0]
-        tmp = be
-        b1 = (start_bit // 8) * 8 + (-start_bit - 1) % 8
-        shift_amount = 64 - (b1 + signal_size)
-
-      if shift_amount < 0:
-        continue
-
-      tmp = (tmp >> shift_amount) & ((1 << signal_size) - 1)
-      if signed and (tmp >> (signal_size - 1)):
-        tmp -= (1 << signal_size)
-
-      tmp = tmp * factor + offset
-
-      # if debug:
-      #   print("%40s  %2d %2d  %7.2f %s" % (s[0], s[1], s[2], tmp, s[-1]))
-
-      if arr is None:
-        out[s[0]] = tmp
-      else:
-        out[arr.index(s[0])] = tmp
-    return name, out
-
   def get_signals(self, msg):
     msg = self.lookup_msg_id(msg)
     return [sgs.name for sgs in self.msgs[msg][1]]

can/packer.cc

@@ -109,9 +109,5 @@ uint64_t CANPacker::pack(uint32_t address, const std::vector<SignalPackValue> &s
     }
   }
 
-  return ret;
-}
-
-Msg* CANPacker::lookup_message(uint32_t address) {
-  return &message_lookup[address];
+  return ReverseBytes(ret);
 }

can/packer_pyx.pyx

@@ -43,16 +43,6 @@ cdef class CANPacker:
 
     return self.packer.pack(addr, values_thing, counter)
 
-  cdef inline uint64_t ReverseBytes(self, uint64_t x):
-    return (((x & 0xff00000000000000ull) >> 56) |
-           ((x & 0x00ff000000000000ull) >> 40) |
-           ((x & 0x0000ff0000000000ull) >> 24) |
-           ((x & 0x000000ff00000000ull) >> 8) |
-           ((x & 0x00000000ff000000ull) << 8) |
-           ((x & 0x0000000000ff0000ull) << 24) |
-           ((x & 0x000000000000ff00ull) << 40) |
-           ((x & 0x00000000000000ffull) << 56))
-
   cpdef make_can_msg(self, name_or_addr, bus, values, counter=-1):
     cdef int addr, size
     if type(name_or_addr) == int:
@@ -61,5 +51,4 @@ cdef class CANPacker:
     else:
       addr, size = self.name_to_address_and_size[name_or_addr.encode('utf8')]
     cdef uint64_t val = self.pack(addr, values, counter)
-    val = self.ReverseBytes(val)
     return [addr, 0, (<char *>&val)[:size], bus]

can/parser.cc

@@ -9,9 +9,6 @@
 
 #include "common.h"
 
-#define DEBUG(...)
-// #define DEBUG printf
-#define INFO printf
 
 bool MessageState::parse(uint64_t sec, uint8_t * dat) {
   uint64_t dat_le = read_u64_le(dat);

can/tests/test_dbc.py

@@ -1,53 +0,0 @@
-import os
-import unittest
-
-from numpy.testing import assert_almost_equal
-
-from opendbc import DBC_PATH
-from opendbc.can.dbc import dbc
-from opendbc.can.packer import CANPacker
-
-
-def assert_message_equal(msg1, msg2, decimal=10):
-  for key in msg1[1]:
-    assert_almost_equal(msg1[1][key], msg2[1][key], decimal=decimal)
-  return True
-
-
-class TestPythonDBC(unittest.TestCase):
-  def test_toyota(self):
-    dbc_test = dbc(os.path.join(DBC_PATH, 'toyota_nodsu_pt_generated.dbc'))
-    packer_test = CANPacker('toyota_nodsu_pt_generated')
-
-    msg = ('STEER_ANGLE_SENSOR', {'STEER_ANGLE': -6.0, 'STEER_RATE': 4, 'STEER_FRACTION': -0.2})
-    encoded = dbc_test.encode(*msg)
-    assert encoded == packer_test.make_can_msg(msg[0], 0, msg[1])[2]
-
-    decoded = dbc_test.decode((0x25, 0, encoded))
-    assert decoded == msg
-
-    msg = ('WHEEL_SPEEDS', {'WHEEL_SPEED_FR': 100.00, 'WHEEL_SPEED_FL': 100.00,
-                            'WHEEL_SPEED_RR': 166.99, 'WHEEL_SPEED_RL': 166.99})
-    encoded = dbc_test.encode(*msg)
-    assert encoded == packer_test.make_can_msg(msg[0], 0, msg[1])[2]
-
-    decoded = dbc_test.decode((0xAA, 0, encoded))
-    assert_message_equal(decoded, msg, decimal=10)
-
-  def test_hyundai(self):
-    # Test Little Endian
-    dbc_test = dbc(os.path.join(DBC_PATH, 'hyundai_2015_ccan.dbc'))
-    packer_test = CANPacker('hyundai_2015_ccan')
-    decoded = dbc_test.decode((0x2b0, 0, b'\xfa\xfe\x00\x07\x12'))
-    assert abs(decoded[1]['SAS_Angle'] - (-26.2)) < 0.001
-
-    msg = ('SAS11', {'SAS_Stat': 7.0, 'MsgCount': 0.0, 'SAS_Angle': -26.200000000000003, 'SAS_Speed': 0.0, 'CheckSum': 0.0})
-    encoded = dbc_test.encode(*msg)
-    assert encoded == packer_test.make_can_msg(msg[0], 0, msg[1])[2]
-
-    decoded = dbc_test.decode((0x2b0, 0, encoded))
-    assert decoded == msg
-
-
-if __name__ == "__main__":
-  unittest.main()