merge upstream and refactoring

.github/workflows/test.yaml

@@ -61,6 +61,8 @@ jobs:
         run: $RUN "scons -j4"
       - name: Test communication protocols
         run: $RUN "cd tests/usbprotocol && ./test.sh"
+      - name: Test logging
+        run: $RUN "cd tests/logging && ./test.sh"
 
   safety:
     name: safety

.gitignore

@@ -1,3 +1,4 @@
+*.tmp
 *.pyc
 .*.swp
 .*.swo

.pre-commit-config.yaml

@@ -1,19 +1,19 @@
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.0.1
+    rev: v4.4.0
     hooks:
     -   id: check-ast
     -   id: check-yaml
     -   id: check-merge-conflict
     -   id: check-symlinks
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.910-1
+    rev: v1.4.1
     hooks:
     -   id: mypy
         additional_dependencies: ['git+https://github.com/numpy/numpy-stubs', 'types-requests', 'types-atomicwrites',
                                   'types-pycurl']
 -   repo: https://github.com/PyCQA/flake8
-    rev: 4.0.1
+    rev: 6.0.0
     hooks:
     -   id: flake8
         args:

Dockerfile

@@ -57,8 +57,8 @@ ENV OPENPILOT_REF="3c153cd1998adf3963b90e65ae9c9beb8592c54d"
 ENV OPENDBC_REF="8e58b58ec2dc2b85022e40657be04e6487e2d388"
 
 COPY requirements.txt /tmp/
-RUN pyenv install 3.8.10 && \
-    pyenv global 3.8.10 && \
+RUN pyenv install 3.11.4 && \
+    pyenv global 3.11.4 && \
     pyenv rehash && \
     pip install --no-cache-dir -r /tmp/requirements.txt
 

__init__.py

@@ -1,5 +1,6 @@
 from .python.constants import McuType, BASEDIR, FW_PATH  # noqa: F401
+from .python.spi import PandaSpiException, PandaProtocolMismatch  # noqa: F401
 from .python.serial import PandaSerial  # noqa: F401
 from .python import (Panda, PandaDFU, # noqa: F401
-                     pack_can_buffer, unpack_can_buffer, calculate_checksum,
+                     pack_can_buffer, unpack_can_buffer, calculate_checksum, unpack_log,
                      DLC_TO_LEN, LEN_TO_DLC, ALTERNATIVE_EXPERIENCE, USBPACKET_MAX_SIZE, CANPACKET_HEAD_SIZE)

board/boards/board_declarations.h

@@ -11,7 +11,7 @@ typedef void (*board_set_ir_power)(uint8_t percentage);
 typedef void (*board_set_fan_enabled)(bool enabled);
 typedef void (*board_set_phone_power)(bool enabled);
 typedef void (*board_set_siren)(bool enabled);
-typedef void (*board_board_tick)(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted);
+typedef bool (*board_board_tick)(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted);
 typedef bool (*board_read_som_gpio)(void);
 
 struct board {

board/boards/dos.h

@@ -53,16 +53,19 @@ void dos_set_bootkick(bool enabled){
   set_gpio_output(GPIOC, 4, !enabled);
 }
 
-void dos_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
+bool dos_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
+  bool ret = false;
   if ((ignition && !usb_enum) || harness_inserted) {
     // enable bootkick if ignition seen or if plugged into a harness
+    ret = true;
     dos_set_bootkick(true);
   } else if (heartbeat_seen) {
     // disable once openpilot is up
     dos_set_bootkick(false);
   } else {
 
   }
+  return ret;
 }
 
 void dos_set_can_mode(uint8_t mode){

board/boards/red_v2.h

@@ -36,5 +36,6 @@ const board board_red_v2 = {
   .set_fan_enabled = unused_set_fan_enabled,
   .set_ir_power = unused_set_ir_power,
   .set_phone_power = unused_set_phone_power,
-  .set_siren = unused_set_siren
+  .set_siren = unused_set_siren,
+  .read_som_gpio = unused_read_som_gpio
 };

board/boards/tres.h

@@ -19,10 +19,12 @@ void tres_set_bootkick(bool enabled){
 }
 
 bool tres_ignition_prev = false;
-void tres_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
+bool tres_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
   UNUSED(usb_enum);
+  bool ret = false;
   if ((ignition && !tres_ignition_prev) || harness_inserted) {
     // enable bootkick on rising edge of ignition
+    ret = true;
     tres_set_bootkick(true);
   } else if (heartbeat_seen) {
     // disable once openpilot is up
@@ -31,6 +33,7 @@ void tres_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool har
 
   }
   tres_ignition_prev = ignition;
+  return ret;
 }
 
 void tres_set_fan_enabled(bool enabled) {

board/boards/uno.h

@@ -126,7 +126,7 @@ void uno_set_can_mode(uint8_t mode){
   }
 }
 
-void uno_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
+bool uno_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
   UNUSED(ignition);
   UNUSED(usb_enum);
   UNUSED(heartbeat_seen);
@@ -136,6 +136,7 @@ void uno_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harn
   } else {
     uno_set_bootkick(false);
   }
+  return false;
 }
 
 bool uno_check_ignition(void){

board/boards/unused_funcs.h

@@ -22,11 +22,12 @@ uint32_t unused_read_current(void) {
   return 0U;
 }
 
-void unused_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
+bool unused_board_tick(bool ignition, bool usb_enum, bool heartbeat_seen, bool harness_inserted) {
   UNUSED(ignition);
   UNUSED(usb_enum);
   UNUSED(heartbeat_seen);
   UNUSED(harness_inserted);
+  return false;
 }
 
 bool unused_read_som_gpio(void) {

board/crc.h

@@ -1,3 +1,5 @@
+#pragma once
+
 uint8_t crc_checksum(uint8_t *dat, int len, const uint8_t poly) {
   uint8_t crc = 0xFFU;
   int i;

board/drivers/bxcan.h

@@ -3,6 +3,11 @@
 //       CAN3_TX, CAN3_RX0, CAN3_SCE
 
 CAN_TypeDef *cans[] = {CAN1, CAN2, CAN3};
+uint8_t can_irq_number[3][3] = {
+  { CAN1_TX_IRQn, CAN1_RX0_IRQn, CAN1_SCE_IRQn },
+  { CAN2_TX_IRQn, CAN2_RX0_IRQn, CAN2_SCE_IRQn },
+  { CAN3_TX_IRQn, CAN3_RX0_IRQn, CAN3_SCE_IRQn },
+};
 
 bool can_set_speed(uint8_t can_number) {
   bool ret = true;
@@ -68,16 +73,10 @@ void can_set_gmlan(uint8_t bus) {
   }
 }
 
-void update_can_health_pkt(uint8_t can_number, bool error_irq) {
+void update_can_health_pkt(uint8_t can_number, uint32_t ir_reg) {
   CAN_TypeDef *CAN = CANIF_FROM_CAN_NUM(can_number);
   uint32_t esr_reg = CAN->ESR;
 
-  if (error_irq) {
-    can_health[can_number].total_error_cnt += 1U;
-    CAN->MSR = CAN_MSR_ERRI;
-    llcan_clear_send(CAN);
-  }
-
   can_health[can_number].bus_off = ((esr_reg & CAN_ESR_BOFF) >> CAN_ESR_BOFF_Pos);
   can_health[can_number].bus_off_cnt += can_health[can_number].bus_off;
   can_health[can_number].error_warning = ((esr_reg & CAN_ESR_EWGF) >> CAN_ESR_EWGF_Pos);
@@ -90,16 +89,31 @@ void update_can_health_pkt(uint8_t can_number, bool error_irq) {
 
   can_health[can_number].receive_error_cnt = ((esr_reg & CAN_ESR_REC) >> CAN_ESR_REC_Pos);
   can_health[can_number].transmit_error_cnt = ((esr_reg & CAN_ESR_TEC) >> CAN_ESR_TEC_Pos);
+
+  can_health[can_number].irq0_call_rate = interrupts[can_irq_number[can_number][0]].call_rate;
+  can_health[can_number].irq1_call_rate = interrupts[can_irq_number[can_number][1]].call_rate;
+  can_health[can_number].irq2_call_rate = interrupts[can_irq_number[can_number][2]].call_rate;
+
+  if (ir_reg != 0U) {
+    can_health[can_number].total_error_cnt += 1U;
+
+    // RX message lost due to FIFO overrun
+    if ((CAN->RF0R & (CAN_RF0R_FOVR0)) != 0) {
+      can_health[can_number].total_rx_lost_cnt += 1U;
+      CAN->RF0R &= ~(CAN_RF0R_FOVR0);
+    }
+    can_health[can_number].can_core_reset_cnt += 1U;
+    llcan_clear_send(CAN);
+  }
 }
 
-// CAN error
+// ***************************** CAN *****************************
+// CANx_SCE IRQ Handler
 void can_sce(uint8_t can_number) {
-  ENTER_CRITICAL();
-  update_can_health_pkt(can_number, true);
-  EXIT_CRITICAL();
+  update_can_health_pkt(can_number, 1U);
 }
 
-// ***************************** CAN *****************************
+// CANx_TX IRQ Handler
 void process_can(uint8_t can_number) {
   if (can_number != 0xffU) {
 
@@ -155,22 +169,16 @@ void process_can(uint8_t can_number) {
       }
     }
 
-    update_can_health_pkt(can_number, false);
     EXIT_CRITICAL();
   }
 }
 
-// CAN receive handlers
+// CANx_RX0 IRQ Handler
 // blink blue when we are receiving CAN messages
 void can_rx(uint8_t can_number) {
   CAN_TypeDef *CAN = CANIF_FROM_CAN_NUM(can_number);
   uint8_t bus_number = BUS_NUM_FROM_CAN_NUM(can_number);
 
-  if ((CAN->RF0R & (CAN_RF0R_FOVR0)) != 0) { // RX message lost due to FIFO overrun
-    can_health[can_number].total_rx_lost_cnt += 1U;
-    CAN->RF0R &= ~(CAN_RF0R_FOVR0);
-  }
-
   while ((CAN->RF0R & CAN_RF0R_FMP0) != 0) {
     can_health[can_number].total_rx_cnt += 1U;
 
@@ -215,7 +223,6 @@ void can_rx(uint8_t can_number) {
     rx_buffer_overflow += can_push(&can_rx_q, &to_push) ? 0U : 1U;
 
     // next
-    update_can_health_pkt(can_number, false);
     CAN->RF0R |= CAN_RF0R_RFOM0;
   }
 }

board/drivers/fdcan.h

@@ -11,6 +11,14 @@ typedef struct {
 
 FDCAN_GlobalTypeDef *cans[] = {FDCAN1, FDCAN2, FDCAN3};
 
+uint8_t can_irq_number[3][2] = {
+  { FDCAN1_IT0_IRQn, FDCAN1_IT1_IRQn },
+  { FDCAN2_IT0_IRQn, FDCAN2_IT1_IRQn },
+  { FDCAN3_IT0_IRQn, FDCAN3_IT1_IRQn },
+};
+
+#define CAN_ACK_ERROR 3U
+
 bool can_set_speed(uint8_t can_number) {
   bool ret = true;
   FDCAN_GlobalTypeDef *CANx = CANIF_FROM_CAN_NUM(can_number);
@@ -32,10 +40,7 @@ void can_set_gmlan(uint8_t bus) {
   print("GMLAN not available on red panda\n");
 }
 
-// ***************************** CAN *****************************
-void update_can_health_pkt(uint8_t can_number, bool error_irq) {
-  ENTER_CRITICAL();
-
+void update_can_health_pkt(uint8_t can_number, uint32_t ir_reg) {
   FDCAN_GlobalTypeDef *CANx = CANIF_FROM_CAN_NUM(can_number);
   uint32_t psr_reg = CANx->PSR;
   uint32_t ecr_reg = CANx->ECR;
@@ -58,17 +63,30 @@ void update_can_health_pkt(uint8_t can_number, bool error_irq) {
   can_health[can_number].receive_error_cnt = ((ecr_reg & FDCAN_ECR_REC) >> FDCAN_ECR_REC_Pos);
   can_health[can_number].transmit_error_cnt = ((ecr_reg & FDCAN_ECR_TEC) >> FDCAN_ECR_TEC_Pos);
 
+  can_health[can_number].irq0_call_rate = interrupts[can_irq_number[can_number][0]].call_rate;
+  can_health[can_number].irq1_call_rate = interrupts[can_irq_number[can_number][1]].call_rate;
+
 
-  if (error_irq) {
+  if (ir_reg != 0U) {
+    // Clear error interrupts
+    CANx->IR |= (FDCAN_IR_PED | FDCAN_IR_PEA | FDCAN_IR_EP | FDCAN_IR_BO | FDCAN_IR_RF0L);
     can_health[can_number].total_error_cnt += 1U;
-    if ((CANx->IR & (FDCAN_IR_TEFL)) != 0) {
-      can_health[can_number].total_tx_lost_cnt += 1U;
+    // Check for RX FIFO overflow
+    if ((ir_reg & (FDCAN_IR_RF0L)) != 0) {
+      can_health[can_number].total_rx_lost_cnt += 1U;
+    }
+    // While multiplexing between buses 1 and 3 we are getting ACK errors that overwhelm CAN core
+    // By resseting CAN core when no ACK is detected for a while(until TEC counter reaches 127) it can recover faster
+    if (((can_health[can_number].last_error == CAN_ACK_ERROR) || (can_health[can_number].last_data_error == CAN_ACK_ERROR)) && (can_health[can_number].transmit_error_cnt > 127U)) {
+      can_health[can_number].can_core_reset_cnt += 1U;
+      can_health[can_number].total_tx_lost_cnt += (FDCAN_TX_FIFO_EL_CNT - (CANx->TXFQS & FDCAN_TXFQS_TFFL)); // TX FIFO msgs will be lost after reset
+      llcan_clear_send(CANx);
     }
-    llcan_clear_send(CANx);
   }
-  EXIT_CRITICAL();
 }
 
+// ***************************** CAN *****************************
+// FDCANx_IT1 IRQ Handler (TX)
 void process_can(uint8_t can_number) {
   if (can_number != 0xffU) {
     ENTER_CRITICAL();
@@ -122,18 +140,18 @@ void process_can(uint8_t can_number) {
         refresh_can_tx_slots_available();
       }
     }
-
-    update_can_health_pkt(can_number, false);
     EXIT_CRITICAL();
   }
 }
 
-// CAN receive handlers
+// FDCANx_IT0 IRQ Handler (RX and errors)
 // blink blue when we are receiving CAN messages
 void can_rx(uint8_t can_number) {
   FDCAN_GlobalTypeDef *CANx = CANIF_FROM_CAN_NUM(can_number);
   uint8_t bus_number = BUS_NUM_FROM_CAN_NUM(can_number);
 
+  uint32_t ir_reg = CANx->IR;
+
   // Clear all new messages from Rx FIFO 0
   CANx->IR |= FDCAN_IR_RF0N;
   while((CANx->RXF0S & FDCAN_RXF0S_F0FL) != 0) {
@@ -212,8 +230,9 @@ void can_rx(uint8_t can_number) {
   }
 
   // Error handling
-  bool error_irq = ((CANx->IR & (FDCAN_IR_PED | FDCAN_IR_PEA | FDCAN_IR_EW | FDCAN_IR_EP | FDCAN_IR_ELO | FDCAN_IR_BO | FDCAN_IR_TEFL | FDCAN_IR_RF0L)) != 0);
-  update_can_health_pkt(can_number, error_irq);
+  if ((ir_reg & (FDCAN_IR_PED | FDCAN_IR_PEA | FDCAN_IR_EP | FDCAN_IR_BO | FDCAN_IR_RF0L)) != 0) {
+    update_can_health_pkt(can_number, ir_reg);
+  }
 }
 
 void FDCAN1_IT0_IRQ_Handler(void) { can_rx(0); }

board/drivers/interrupts.h

@@ -2,6 +2,7 @@ typedef struct interrupt {
   IRQn_Type irq_type;
   void (*handler)(void);
   uint32_t call_counter;
+  uint32_t call_rate;
   uint32_t max_call_rate;   // Call rate is defined as the amount of calls each second
   uint32_t call_rate_fault;
 } interrupt;
@@ -17,11 +18,12 @@ void unused_interrupt_handler(void) {
 
 interrupt interrupts[NUM_INTERRUPTS];
 
-#define REGISTER_INTERRUPT(irq_num, func_ptr, call_rate, rate_fault) \
+#define REGISTER_INTERRUPT(irq_num, func_ptr, call_rate_max, rate_fault) \
   interrupts[irq_num].irq_type = (irq_num); \
   interrupts[irq_num].handler = (func_ptr);  \
   interrupts[irq_num].call_counter = 0U;   \
-  interrupts[irq_num].max_call_rate = (call_rate); \
+  interrupts[irq_num].call_rate = 0U;   \
+  interrupts[irq_num].max_call_rate = (call_rate_max); \
   interrupts[irq_num].call_rate_fault = (rate_fault);
 
 bool check_interrupt_rate = false;
@@ -70,6 +72,7 @@ void interrupt_timer_handler(void) {
       }
 
       // Reset interrupt counters
+      interrupts[i].call_rate = interrupts[i].call_counter;
       interrupts[i].call_counter = 0U;
     }