packer/parser: generic counter handling (commaai#659)

* packer/parser: generic counter handling * pedal isn't special
spektor56 · Jul 26, 2022 · 74c3320 · 74c3320
1 parent 58f78de
commit 74c3320
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 19 deletions.
diff --git a/can/common.pxd b/can/common.pxd
@@ -14,17 +14,14 @@ ctypedef unsigned int (*calc_checksum_type)(uint32_t, const Signal&, const vecto
 cdef extern from "common_dbc.h":
   ctypedef enum SignalType:
     DEFAULT,
+    COUNTER,
     HONDA_CHECKSUM,
-    HONDA_COUNTER,
     TOYOTA_CHECKSUM,
     PEDAL_CHECKSUM,
-    PEDAL_COUNTER,
     VOLKSWAGEN_MQB_CHECKSUM,
-    VOLKSWAGEN_MQB_COUNTER,
     SUBARU_CHECKSUM,
     CHRYSLER_CHECKSUM
     HKG_CAN_FD_CHECKSUM,
-    HKG_CAN_FD_COUNTER,
 
   cdef struct Signal:
     string name

diff --git a/can/common_dbc.h b/can/common_dbc.h
@@ -31,17 +31,14 @@ struct SignalValue {
 
 enum SignalType {
   DEFAULT,
+  COUNTER,
   HONDA_CHECKSUM,
-  HONDA_COUNTER,
   TOYOTA_CHECKSUM,
   PEDAL_CHECKSUM,
-  PEDAL_COUNTER,
   VOLKSWAGEN_MQB_CHECKSUM,
-  VOLKSWAGEN_MQB_COUNTER,
   SUBARU_CHECKSUM,
   CHRYSLER_CHECKSUM,
   HKG_CAN_FD_CHECKSUM,
-  HKG_CAN_FD_COUNTER,
 };
 
 struct Signal {

diff --git a/can/dbc.cc b/can/dbc.cc
@@ -60,19 +60,19 @@ typedef struct ChecksumState {
 ChecksumState* get_checksum(const std::string& dbc_name) {
   ChecksumState* s = nullptr;
   if (startswith(dbc_name, {"honda_", "acura_"})) {
-    s = new ChecksumState({4, 2, 3, 5, false, HONDA_CHECKSUM, HONDA_COUNTER, &honda_checksum});
+    s = new ChecksumState({4, 2, 3, 5, false, HONDA_CHECKSUM, COUNTER, &honda_checksum});
   } else if (startswith(dbc_name, {"toyota_", "lexus_"})) {
     s = new ChecksumState({8, -1, 7, -1, false, TOYOTA_CHECKSUM, DEFAULT, &toyota_checksum});
   } else if (startswith(dbc_name, "kia_ev6")) {
-    s = new ChecksumState({16, 8, 0, 0, true, HKG_CAN_FD_CHECKSUM, HKG_CAN_FD_COUNTER, &hkg_can_fd_checksum});
+    s = new ChecksumState({16, 8, 0, 0, true, HKG_CAN_FD_CHECKSUM, COUNTER, &hkg_can_fd_checksum});
   } else if (startswith(dbc_name, {"vw_mqb_2010"})) {
-    s = new ChecksumState({8, 4, 0, 0, true, VOLKSWAGEN_MQB_CHECKSUM, VOLKSWAGEN_MQB_COUNTER, &volkswagen_mqb_checksum});
+    s = new ChecksumState({8, 4, 0, 0, true, VOLKSWAGEN_MQB_CHECKSUM, COUNTER, &volkswagen_mqb_checksum});
   } else if (startswith(dbc_name, "subaru_global_")) {
     s = new ChecksumState({8, -1, 0, -1, true, SUBARU_CHECKSUM, DEFAULT, &subaru_checksum});
   } else if (startswith(dbc_name, "chrysler_")) {
     s = new ChecksumState({8, -1, 7, -1, false, CHRYSLER_CHECKSUM, DEFAULT, &chrysler_checksum});
   } else if (startswith(dbc_name, "comma_body")) {
-    s = new ChecksumState({8, 4, 7, 3, false, PEDAL_CHECKSUM, PEDAL_COUNTER, &pedal_checksum});
+    s = new ChecksumState({8, 4, 7, 3, false, PEDAL_CHECKSUM, COUNTER, &pedal_checksum});
   }
   return s;
 }
@@ -94,12 +94,14 @@ void set_signal_type(Signal& s, ChecksumState* chk, const std::string& dbc_name,
       s.type = chk->counter_type;
     }
   }
+
+  // TODO: CAN packer/parser shouldn't know anything about interceptors or pedals
   if (s.name == "CHECKSUM_PEDAL") {
     DBC_ASSERT(s.size == 8, "INTERCEPTOR CHECKSUM is not 8 bits long");
     s.type = PEDAL_CHECKSUM;
   } else if (s.name == "COUNTER_PEDAL") {
     DBC_ASSERT(s.size == 4, "INTERCEPTOR COUNTER is not 4 bits long");
-    s.type = PEDAL_COUNTER;
+    s.type = COUNTER;
   }
 }
 

diff --git a/can/packer.cc b/can/packer.cc
@@ -69,11 +69,6 @@ std::vector<uint8_t> CANPacker::pack(uint32_t address, const std::vector<SignalP
       return ret;
     }
     const auto& sig = sig_it->second;
-
-    if ((sig.type != SignalType::HONDA_COUNTER) && (sig.type != SignalType::VOLKSWAGEN_MQB_COUNTER)) {
-      //WARN("COUNTER signal type not valid\n");
-    }
-
     set_value(ret, sig, counter);
   }
 

diff --git a/can/parser.cc b/can/parser.cc
@@ -52,7 +52,7 @@ bool MessageState::parse(uint64_t sec, const std::vector<uint8_t> &dat) {
 
     bool counter_failed = false;
     if (!ignore_counter) {
-      if (sig.type == SignalType::HONDA_COUNTER || sig.type == SignalType::VOLKSWAGEN_MQB_COUNTER || sig.type == SignalType::PEDAL_COUNTER) {
+      if (sig.type == SignalType::COUNTER) {
         counter_failed = !update_counter_generic(tmp, sig.size);
       }
     }