Misra pedal (commaai#245)

* Fix misra on pedal
emmertex · Jul 7, 2019 · 1f40d1e · 1f40d1e
1 parent a4554e3
commit 1f40d1e
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 54 deletions.
diff --git a/board/drivers/dac.h b/board/drivers/dac.h
@@ -1,4 +1,4 @@
-void dac_init() {
+void dac_init(void) {
   // no buffers required since we have an opamp
   //DAC->CR = DAC_CR_EN1 | DAC_CR_BOFF1 | DAC_CR_EN2 | DAC_CR_BOFF2;
   DAC->DHR12R1 = 0;

diff --git a/board/pedal/Makefile b/board/pedal/Makefile
@@ -1,7 +1,7 @@
 # :set noet
 PROJ_NAME = comma
 
-CFLAGS  = -O2 -Wall -std=gnu11 -DPEDAL
+CFLAGS  = -O2 -Wall -Wextra -Wstrict-prototypes -std=gnu11 -DPEDAL
 CFLAGS += -mlittle-endian -mthumb -mcpu=cortex-m3
 CFLAGS += -msoft-float -DSTM32F2 -DSTM32F205xx
 CFLAGS += -I ../inc -I ../ -I ../../ -nostdlib

diff --git a/board/pedal/main.c b/board/pedal/main.c
@@ -19,14 +19,18 @@
   #include "drivers/usb.h"
 #else
   // no serial either
-  void puts(const char *a) {}
-  void puth(unsigned int i) {}
+  void puts(const char *a) {
+    UNUSED(a);
+  }
+  void puth(unsigned int i) {
+    UNUSED(i);
+  }
 #endif
 
 #define ENTER_BOOTLOADER_MAGIC 0xdeadbeef
 uint32_t enter_bootloader_mode;
 
-void __initialize_hardware_early() {
+void __initialize_hardware_early(void) {
   early();
 }
 
@@ -36,31 +40,54 @@ void __initialize_hardware_early() {
 
 void debug_ring_callback(uart_ring *ring) {
   char rcv;
-  while (getc(ring, &rcv)) {
-    putc(ring, rcv);
+  while (getc(ring, &rcv) != 0) {
+    (void)putc(ring, rcv);
   }
 }
 
-int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired) { return 0; }
-void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired) {}
-void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired) {}
-void usb_cb_enumeration_complete() {}
+int usb_cb_ep1_in(uint8_t *usbdata, int len, bool hardwired) {
+  UNUSED(usbdata);
+  UNUSED(len);
+  UNUSED(hardwired);
+  return 0;
+}
+void usb_cb_ep2_out(uint8_t *usbdata, int len, bool hardwired) {
+  UNUSED(usbdata);
+  UNUSED(len);
+  UNUSED(hardwired);
+}
+void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired) {
+  UNUSED(usbdata);
+  UNUSED(len);
+  UNUSED(hardwired);
+}
+void usb_cb_enumeration_complete(void) {}
 
 int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired) {
-  int resp_len = 0;
+  UNUSED(hardwired);
+  unsigned int resp_len = 0;
   uart_ring *ur = NULL;
   switch (setup->b.bRequest) {
     // **** 0xe0: uart read
     case 0xe0:
       ur = get_ring_by_number(setup->b.wValue.w);
-      if (!ur) break;
-      if (ur == &esp_ring) uart_dma_drain();
+      if (!ur) {
+        break;
+      }
+      if (ur == &esp_ring) {
+        uart_dma_drain();
+      }
       // read
       while ((resp_len < MIN(setup->b.wLength.w, MAX_RESP_LEN)) &&
                          getc(ur, (char*)&resp[resp_len])) {
         ++resp_len;
       }
       break;
+    default:
+      puts("NO HANDLER ");
+      puth(setup->b.bRequest);
+      puts("\n");
+      break;
   }
   return resp_len;
 }
@@ -76,7 +103,7 @@ uint8_t pedal_checksum(uint8_t *dat, int len) {
   for (i = len - 1; i >= 0; i--) {
     crc ^= dat[i];
     for (j = 0; j < 8; j++) {
-      if ((crc & 0x80) != 0) {
+      if ((crc & 0x80U) != 0U) {
         crc = (uint8_t)((crc << 1) ^ poly);
       }
       else {
@@ -91,11 +118,11 @@ uint8_t pedal_checksum(uint8_t *dat, int len) {
 
 // addresses to be used on CAN
 #define CAN_GAS_INPUT  0x200
-#define CAN_GAS_OUTPUT 0x201
+#define CAN_GAS_OUTPUT 0x201U
 #define CAN_GAS_SIZE 6
-#define COUNTER_CYCLE 0xF
+#define COUNTER_CYCLE 0xFU
 
-void CAN1_TX_IRQHandler() {
+void CAN1_TX_IRQHandler(void) {
   // clear interrupt
   CAN->TSR |= CAN_TSR_RQCP0;
 }
@@ -104,25 +131,25 @@ void CAN1_TX_IRQHandler() {
 uint16_t gas_set_0 = 0;
 uint16_t gas_set_1 = 0;
 
-#define MAX_TIMEOUT 10
+#define MAX_TIMEOUT 10U
 uint32_t timeout = 0;
 uint32_t current_index = 0;
 
-#define NO_FAULT 0
-#define FAULT_BAD_CHECKSUM 1
-#define FAULT_SEND 2
-#define FAULT_SCE 3
-#define FAULT_STARTUP 4
-#define FAULT_TIMEOUT 5
-#define FAULT_INVALID 6
+#define NO_FAULT 0U
+#define FAULT_BAD_CHECKSUM 1U
+#define FAULT_SEND 2U
+#define FAULT_SCE 3U
+#define FAULT_STARTUP 4U
+#define FAULT_TIMEOUT 5U
+#define FAULT_INVALID 6U
 uint8_t state = FAULT_STARTUP;
 
-void CAN1_RX0_IRQHandler() {
-  while (CAN->RF0R & CAN_RF0R_FMP0) {
+void CAN1_RX0_IRQHandler(void) {
+  while ((CAN->RF0R & CAN_RF0R_FMP0) != 0) {
     #ifdef DEBUG
       puts("CAN RX\n");
     #endif
-    uint32_t address = CAN->sFIFOMailBox[0].RIR>>21;
+    int address = CAN->sFIFOMailBox[0].RIR >> 21;
     if (address == CAN_GAS_INPUT) {
       // softloader entry
       if (CAN->sFIFOMailBox[0].RDLR == 0xdeadface) {
@@ -132,6 +159,8 @@ void CAN1_RX0_IRQHandler() {
         } else if (CAN->sFIFOMailBox[0].RDHR == 0x02b00b1e) {
           enter_bootloader_mode = ENTER_BOOTLOADER_MAGIC;
           NVIC_SystemReset();
+        } else {
+          puts("Failed entering Softloader or Bootloader\n");
         }
       }
 
@@ -145,10 +174,10 @@ void CAN1_RX0_IRQHandler() {
       }
       uint16_t value_0 = (dat[0] << 8) | dat[1];
       uint16_t value_1 = (dat[2] << 8) | dat[3];
-      uint8_t enable = (dat[4] >> 7) & 1;
+      bool enable = ((dat[4] >> 7) & 1U) != 0U;
       uint8_t index = dat[4] & COUNTER_CYCLE;
       if (pedal_checksum(dat, CAN_GAS_SIZE - 1) == dat[5]) {
-        if (((current_index + 1) & COUNTER_CYCLE) == index) {
+        if (((current_index + 1U) & COUNTER_CYCLE) == index) {
           #ifdef DEBUG
             puts("setting gas ");
             puth(value);
@@ -159,12 +188,13 @@ void CAN1_RX0_IRQHandler() {
             gas_set_1 = value_1;
           } else {
             // clear the fault state if values are 0
-            if (value_0 == 0 && value_1 == 0) {
+            if ((value_0 == 0U) && (value_1 == 0U)) {
               state = NO_FAULT;
             } else {
               state = FAULT_INVALID;
             }
-            gas_set_0 = gas_set_1 = 0;
+            gas_set_0 = 0;
+            gas_set_1 = 0;
           }
           // clear the timeout
           timeout = 0;
@@ -180,17 +210,18 @@ void CAN1_RX0_IRQHandler() {
   }
 }
 
-void CAN1_SCE_IRQHandler() {
+void CAN1_SCE_IRQHandler(void) {
   state = FAULT_SCE;
   llcan_clear_send(CAN);
 }
 
-int pdl0 = 0, pdl1 = 0;
-int pkt_idx = 0;
+uint32_t pdl0 = 0;
+uint32_t pdl1 = 0;
+unsigned int pkt_idx = 0;
 
 int led_value = 0;
 
-void TIM3_IRQHandler() {
+void TIM3_IRQHandler(void) {
   #ifdef DEBUG
     puth(TIM3->CNT);
     puts(" ");
@@ -203,16 +234,16 @@ void TIM3_IRQHandler() {
   // check timer for sending the user pedal and clearing the CAN
   if ((CAN->TSR & CAN_TSR_TME0) == CAN_TSR_TME0) {
     uint8_t dat[8];
-    dat[0] = (pdl0>>8) & 0xFF;
-    dat[1] = (pdl0>>0) & 0xFF;
-    dat[2] = (pdl1>>8) & 0xFF;
-    dat[3] = (pdl1>>0) & 0xFF;
-    dat[4] = (state & 0xF) << 4 | pkt_idx;
+    dat[0] = (pdl0 >> 8) & 0xFFU;
+    dat[1] = (pdl0 >> 0) & 0xFFU;
+    dat[2] = (pdl1 >> 8) & 0xFFU;
+    dat[3] = (pdl1 >> 0) & 0xFFU;
+    dat[4] = ((state & 0xFU) << 4) | pkt_idx;
     dat[5] = pedal_checksum(dat, CAN_GAS_SIZE - 1);
-    CAN->sTxMailBox[0].TDLR = dat[0] | (dat[1]<<8) | (dat[2]<<16) | (dat[3]<<24);
-    CAN->sTxMailBox[0].TDHR = dat[4] | (dat[5]<<8);
+    CAN->sTxMailBox[0].TDLR = dat[0] | (dat[1] << 8) | (dat[2] << 16) | (dat[3] << 24);
+    CAN->sTxMailBox[0].TDHR = dat[4] | (dat[5] << 8);
     CAN->sTxMailBox[0].TDTR = 6;  // len of packet is 5
-    CAN->sTxMailBox[0].TIR = (CAN_GAS_OUTPUT << 21) | 1;
+    CAN->sTxMailBox[0].TIR = (CAN_GAS_OUTPUT << 21) | 1U;
     ++pkt_idx;
     pkt_idx &= COUNTER_CYCLE;
   } else {
@@ -233,13 +264,13 @@ void TIM3_IRQHandler() {
   if (timeout == MAX_TIMEOUT) {
     state = FAULT_TIMEOUT;
   } else {
-    timeout += 1;
+    timeout += 1U;
   }
 }
 
 // ***************************** main code *****************************
 
-void pedal() {
+void pedal(void) {
   // read/write
   pdl0 = adc_get(ADCCHAN_ACCEL0);
   pdl1 = adc_get(ADCCHAN_ACCEL1);
@@ -256,7 +287,7 @@ void pedal() {
   watchdog_feed();
 }
 
-int main() {
+int main(void) {
   __disable_irq();
 
   // init devices
@@ -274,7 +305,11 @@ int main() {
   adc_init();
 
   // init can
-  llcan_set_speed(CAN1, 5000, false, false);
+  bool llcan_speed_set = llcan_set_speed(CAN1, 5000, false, false);
+  if (!llcan_speed_set) {
+    puts("Failed to set llcan speed");
+  }
+
   llcan_init(CAN1);
 
   // 48mhz / 65536 ~= 732

diff --git a/board/spi_flasher.h b/board/spi_flasher.h
@@ -105,7 +105,7 @@ void usb_cb_ep3_out(uint8_t *usbdata, int len, bool hardwired) {
 }
 
 int is_enumerated = 0;
-void usb_cb_enumeration_complete() {
+void usb_cb_enumeration_complete(void) {
   puts("USB enumeration complete\n");
   is_enumerated = 1;
 }
@@ -151,7 +151,7 @@ int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) {
 #define CAN_BL_INPUT 0x1
 #define CAN_BL_OUTPUT 0x2
 
-void CAN1_TX_IRQHandler() {
+void CAN1_TX_IRQHandler(void) {
   // clear interrupt
   CAN->TSR |= CAN_TSR_RQCP0;
 }
@@ -178,7 +178,7 @@ void bl_can_send(uint8_t *odat) {
   CAN->sTxMailBox[0].TIR = (CAN_BL_OUTPUT << 21) | 1;
 }
 
-void CAN1_RX0_IRQHandler() {
+void CAN1_RX0_IRQHandler(void) {
   while (CAN->RF0R & CAN_RF0R_FMP0) {
     if ((CAN->sFIFOMailBox[0].RIR>>21) == CAN_BL_INPUT) {
       uint8_t dat[8];
@@ -252,7 +252,7 @@ void CAN1_RX0_IRQHandler() {
   }
 }
 
-void CAN1_SCE_IRQHandler() {
+void CAN1_SCE_IRQHandler(void) {
   llcan_clear_send(CAN);
 }
 

diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh
@@ -24,5 +24,5 @@ then
 fi
 
 # pedal code
-tests/misra/cppcheck/cppcheck --suppressions-list=tests/misra/suppressions.txt --dump --enable=all --inline-suppr board/pedal/main.c 2>/tmp/misra/cppcheck_pedal_output.txt || true
+tests/misra/cppcheck/cppcheck --suppressions-list=tests/misra/suppressions.txt -I board/ --dump --enable=all --inline-suppr board/pedal/main.c 2>/tmp/misra/cppcheck_pedal_output.txt || true
 python tests/misra/cppcheck/addons/misra.py board/pedal/main.c.dump 2>/tmp/misra/misra_pedal_output.txt || true