From bfa9902d5d3f84fd876b56fc06df33958aa27b4f Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 13:09:32 -0800 Subject: [PATCH 1/6] remove some unused --- board/boards/dos.h | 4 ---- board/boards/unused_funcs.h | 4 ---- board/drivers/gmlan_alt.h | 17 ----------------- board/drivers/uart.h | 15 --------------- board/stm32fx/lluart.h | 29 ----------------------------- tests/libpanda/safety_helpers.h | 7 ------- 6 files changed, 76 deletions(-) diff --git a/board/boards/dos.h b/board/boards/dos.h index d0a9bbaf49..428bbf2a5e 100644 --- a/board/boards/dos.h +++ b/board/boards/dos.h @@ -90,10 +90,6 @@ bool dos_check_ignition(void){ return harness_check_ignition(); } -void dos_set_usb_switch(bool phone){ - set_gpio_output(GPIOB, 3, phone); -} - void dos_set_ir_power(uint8_t percentage){ pwm_set(TIM4, 2, percentage); } diff --git a/board/boards/unused_funcs.h b/board/boards/unused_funcs.h index 689b5b7c42..7bfde01391 100644 --- a/board/boards/unused_funcs.h +++ b/board/boards/unused_funcs.h @@ -13,10 +13,6 @@ void unused_set_siren(bool enabled) { UNUSED(enabled); } -uint32_t unused_read_voltage(void) { - return 0U; -} - uint32_t unused_read_current(void) { return 0U; } diff --git a/board/drivers/gmlan_alt.h b/board/drivers/gmlan_alt.h index 407062bbbd..56e606d3ef 100644 --- a/board/drivers/gmlan_alt.h +++ b/board/drivers/gmlan_alt.h @@ -147,17 +147,6 @@ int inverted_bit_to_send = GMLAN_HIGH; int gmlan_switch_below_timeout = -1; int gmlan_switch_timeout_enable = 0; -void gmlan_switch_init(int timeout_enable) { - gmlan_switch_timeout_enable = timeout_enable; - gmlan_alt_mode = GPIO_SWITCH; - gmlan_switch_below_timeout = 1; - set_gpio_mode(GPIOB, 13, MODE_OUTPUT); - - setup_timer(); - - inverted_bit_to_send = GMLAN_LOW; //We got initialized, set the output low -} - void set_gmlan_digital_output(int to_set) { inverted_bit_to_send = to_set; /* @@ -167,12 +156,6 @@ void set_gmlan_digital_output(int to_set) { */ } -void reset_gmlan_switch_timeout(void) { - can_timeout_counter = GMLAN_TICKS_PER_SECOND; - gmlan_switch_below_timeout = 1; - gmlan_alt_mode = GPIO_SWITCH; -} - void set_bitbanged_gmlan(int val) { if (val != 0) { register_set_bits(&(GPIOB->ODR), (1UL << 13)); diff --git a/board/drivers/uart.h b/board/drivers/uart.h index e37dfcd6bd..01d8c2ac05 100644 --- a/board/drivers/uart.h +++ b/board/drivers/uart.h @@ -129,21 +129,6 @@ bool put_char(uart_ring *q, char elem) { return ret; } -// Seems dangerous to use (might lock CPU if called with interrupts disabled f.e.) -// TODO: Remove? Not used anyways -void uart_flush(const uart_ring *q) { - while (q->w_ptr_tx != q->r_ptr_tx) { - __WFI(); - } -} - -void uart_flush_sync(uart_ring *q) { - // empty the TX buffer - while (q->w_ptr_tx != q->r_ptr_tx) { - uart_tx_ring(q); - } -} - void clear_uart_buff(uart_ring *q) { ENTER_CRITICAL(); q->w_ptr_tx = 0; diff --git a/board/stm32fx/lluart.h b/board/stm32fx/lluart.h index 1e4e9015df..ffe7e74da0 100644 --- a/board/stm32fx/lluart.h +++ b/board/stm32fx/lluart.h @@ -90,32 +90,3 @@ void USART2_IRQ_Handler(void) { uart_interrupt_handler(&uart_ring_debug); } void uart_set_baud(USART_TypeDef *u, unsigned int baud) { u->BRR = USART_BRR_(APB1_FREQ*1000000U, baud); } - -void uart_init(uart_ring *q, int baud) { - if(q->uart != NULL){ - // Register interrupts (max data rate: 115200 baud) - if (q->uart == USART2){ - REGISTER_INTERRUPT(USART2_IRQn, USART2_IRQ_Handler, 150000U, FAULT_INTERRUPT_RATE_UART_2) - } else { - // UART not used. Skip registering interrupts - } - - // Set baud and enable peripheral with TX and RX mode - uart_set_baud(q->uart, baud); - q->uart->CR1 = USART_CR1_UE | USART_CR1_TE | USART_CR1_RE; - if ((q->uart == USART2) || (q->uart == USART3) || (q->uart == UART5)) { - q->uart->CR1 |= USART_CR1_RXNEIE; - } - - // Enable UART interrupts - if (q->uart == USART2){ - NVIC_EnableIRQ(USART2_IRQn); - } else if (q->uart == USART3){ - NVIC_EnableIRQ(USART3_IRQn); - } else if (q->uart == UART5){ - NVIC_EnableIRQ(UART5_IRQn); - } else { - // UART not used. Skip enabling interrupts - } - } -} diff --git a/tests/libpanda/safety_helpers.h b/tests/libpanda/safety_helpers.h index ae150a8389..11fa5e4482 100644 --- a/tests/libpanda/safety_helpers.h +++ b/tests/libpanda/safety_helpers.h @@ -196,10 +196,3 @@ void init_tests(void){ void set_gmlan_digital_output(int to_set){ } - -void reset_gmlan_switch_timeout(void){ -} - -void gmlan_switch_init(int timeout_enable){ -} - From 6fb15606213669c3eb0223c2bd1b05c734df039e Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 13:12:23 -0800 Subject: [PATCH 2/6] more --- board/drivers/gmlan_alt.h | 9 --------- tests/libpanda/safety_helpers.h | 3 --- 2 files changed, 12 deletions(-) diff --git a/board/drivers/gmlan_alt.h b/board/drivers/gmlan_alt.h index 56e606d3ef..a4377a675e 100644 --- a/board/drivers/gmlan_alt.h +++ b/board/drivers/gmlan_alt.h @@ -147,15 +147,6 @@ int inverted_bit_to_send = GMLAN_HIGH; int gmlan_switch_below_timeout = -1; int gmlan_switch_timeout_enable = 0; -void set_gmlan_digital_output(int to_set) { - inverted_bit_to_send = to_set; - /* - print("Writing "); - puth(inverted_bit_to_send); - print("\n"); - */ -} - void set_bitbanged_gmlan(int val) { if (val != 0) { register_set_bits(&(GPIOB->ODR), (1UL << 13)); diff --git a/tests/libpanda/safety_helpers.h b/tests/libpanda/safety_helpers.h index 11fa5e4482..074463d319 100644 --- a/tests/libpanda/safety_helpers.h +++ b/tests/libpanda/safety_helpers.h @@ -193,6 +193,3 @@ void init_tests(void){ valid_steer_req_count = 0; invalid_steer_req_count = 0; } - -void set_gmlan_digital_output(int to_set){ -} From 34c7c22bd858751ac0052297cb22750af4ab7540 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 13:32:03 -0800 Subject: [PATCH 3/6] kinda works --- board/drivers/interrupts.h | 2 +- board/flasher.h | 23 ----------------------- board/provision.h | 6 ------ tests/misra/suppressions.txt | 4 +++- tests/misra/test_misra.sh | 18 +++++++++++------- 5 files changed, 15 insertions(+), 38 deletions(-) diff --git a/board/drivers/interrupts.h b/board/drivers/interrupts.h index 9cb46d4b29..79c87cccc2 100644 --- a/board/drivers/interrupts.h +++ b/board/drivers/interrupts.h @@ -78,7 +78,7 @@ void interrupt_timer_handler(void) { // Calculate interrupt load // The bootstub does not have the FPU enabled, so can't do float operations. -#if !defined(PEDAL) && !defined(BOOTSTUB) +#if !defined(BOOTSTUB) interrupt_load = ((busy_time + idle_time) > 0U) ? ((float) busy_time) / (busy_time + idle_time) : 0.0f; #endif idle_time = 0U; diff --git a/board/flasher.h b/board/flasher.h index 52906f9df1..04349aabbc 100644 --- a/board/flasher.h +++ b/board/flasher.h @@ -120,29 +120,6 @@ void comms_endpoint2_write(const uint8_t *data, uint32_t len) { } -int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) { - UNUSED(len); - ControlPacket_t control_req; - - int resp_len = 0; - switch (data[0]) { - case 0: - // control transfer - control_req.request = ((USB_Setup_TypeDef *)(data+4))->b.bRequest; - control_req.param1 = ((USB_Setup_TypeDef *)(data+4))->b.wValue.w; - control_req.param2 = ((USB_Setup_TypeDef *)(data+4))->b.wIndex.w; - control_req.length = ((USB_Setup_TypeDef *)(data+4))->b.wLength.w; - - resp_len = comms_control_handler(&control_req, data_out); - break; - case 2: - // ep 2, flash! - comms_endpoint2_write(data+4, data[2]); - break; - } - return resp_len; -} - void soft_flasher_start(void) { print("\n\n\n************************ FLASHER START ************************\n"); diff --git a/board/provision.h b/board/provision.h index db22ff63fa..02768c93d0 100644 --- a/board/provision.h +++ b/board/provision.h @@ -11,9 +11,3 @@ void get_provision_chunk(uint8_t *resp) { (void)memcpy(resp, "unprovisioned\x00\x00\x00testing123\x00\x00\xa3\xa6\x99\xec", 0x20); } } - -uint8_t chunk[PROVISION_CHUNK_LEN]; -bool is_provisioned(void) { - (void)memcpy(chunk, (uint8_t *)PROVISION_CHUNK_ADDRESS, PROVISION_CHUNK_LEN); - return (memcmp(chunk, unprovisioned_text, 0x20) != 0); -} diff --git a/tests/misra/suppressions.txt b/tests/misra/suppressions.txt index 2c91f868ab..bf4386b80b 100644 --- a/tests/misra/suppressions.txt +++ b/tests/misra/suppressions.txt @@ -12,11 +12,13 @@ misra-c2012-20.10 # needed since not all of these suppressions are applicable to all builds unmatchedSuppression +# +unusedFunction:*/interrupt_handlers*.h + # all of the below suppressions are from new checks introduced after updating # cppcheck from 2.5 -> 2.13. they are listed here to separate the update from # fixing the violations and all are intended to be removed soon after -unusedFunction misra-config misra-c2012-1.2 # this is from the extensions (e.g. __typeof__) used in the MIN, MAX, ABS, and CLAMP macros diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh index 35a98db215..f3fe4487ef 100755 --- a/tests/misra/test_misra.sh +++ b/tests/misra/test_misra.sh @@ -2,7 +2,7 @@ set -e DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" -PANDA_DIR=$DIR/../../ +PANDA_DIR=$(realpath $DIR/../../) GREEN="\e[1;32m" NC='\033[0m' @@ -31,23 +31,27 @@ fi cppcheck() { # note that cppcheck build cache results in inconsistent results as of v2.13.0 OUTPUT=$DIR/.output.log - $CPPCHECK_DIR/cppcheck --enable=all --force --inline-suppr -I $PANDA_DIR/board/ \ + $CPPCHECK_DIR/cppcheck --force --inline-suppr -I $PANDA_DIR/board/ \ -I $gcc_inc "$(arm-none-eabi-gcc -print-file-name=include)" \ --suppressions-list=$DIR/suppressions.txt --suppress=*:*inc/* \ - --suppress=*:*include/* --error-exitcode=2 --addon=misra \ - --check-level=exhaustive "$@" |& tee $OUTPUT + --suppress=*:*include/* --error-exitcode=2 -rp=$PANDA_DIR \ + "$@" |& tee $OUTPUT # cppcheck bug: some MISRA errors won't result in the error exit code, # so check the output (https://trac.cppcheck.net/ticket/12440#no1) - if grep -e "misra violation" -e "error" $OUTPUT > /dev/null; then + if grep -e "misra violation" -e "error" -e "style: " $OUTPUT > /dev/null; then exit 1 fi } printf "\n${GREEN}** PANDA F4 CODE **${NC}\n" -cppcheck -DPANDA -DSTM32F4 -DUID_BASE $PANDA_DIR/board/main.c +cppcheck --disable=unusedFunction -DPANDA -DSTM32F4 -DUID_BASE $PANDA_DIR/board/main.c printf "\n${GREEN}** PANDA H7 CODE **${NC}\n" -cppcheck -DPANDA -DSTM32H7 -DUID_BASE $PANDA_DIR/board/main.c +cppcheck --disable=unusedFunction -DPANDA -DSTM32H7 -DUID_BASE $PANDA_DIR/board/main.c + +# unused needs to run globally +printf "\n${GREEN}** UNUSED ALL CODE **${NC}\n" +cppcheck --enable=unusedFunction --quiet $PANDA_DIR/board/ printf "\n${GREEN}Success!${NC} took $SECONDS seconds\n" From a97018f5f4fae589e60b2af5d169f4b91786419c Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 13:53:42 -0800 Subject: [PATCH 4/6] rest are false positives --- board/bootstub.c | 1 + board/flasher.h | 4 ---- tests/misra/test_misra.sh | 11 +++++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/board/bootstub.c b/board/bootstub.c index e2977e938c..aee665e7e0 100644 --- a/board/bootstub.c +++ b/board/bootstub.c @@ -19,6 +19,7 @@ #include "obj/gitversion.h" #include "flasher.h" +// cppcheck-suppress unusedFunction ; used in headers not included in cppcheck void __initialize_hardware_early(void) { early_initialization(); } diff --git a/board/flasher.h b/board/flasher.h index 04349aabbc..d6c2c40211 100644 --- a/board/flasher.h +++ b/board/flasher.h @@ -4,10 +4,6 @@ bool unlocked = false; void spi_init(void); -#ifdef uart_ring -void debug_ring_callback(uart_ring *ring) {} -#endif - int comms_control_handler(ControlPacket_t *req, uint8_t *resp) { int resp_len = 0; diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh index f3fe4487ef..e95d7f3104 100755 --- a/tests/misra/test_misra.sh +++ b/tests/misra/test_misra.sh @@ -34,7 +34,7 @@ cppcheck() { $CPPCHECK_DIR/cppcheck --force --inline-suppr -I $PANDA_DIR/board/ \ -I $gcc_inc "$(arm-none-eabi-gcc -print-file-name=include)" \ --suppressions-list=$DIR/suppressions.txt --suppress=*:*inc/* \ - --suppress=*:*include/* --error-exitcode=2 -rp=$PANDA_DIR \ + --suppress=*:*include/* --error-exitcode=2 \ "$@" |& tee $OUTPUT # cppcheck bug: some MISRA errors won't result in the error exit code, @@ -44,14 +44,17 @@ cppcheck() { fi } +PANDA_OPTS="--enable=all --disable=unusedFunction -DPANDA --addon=misra" + printf "\n${GREEN}** PANDA F4 CODE **${NC}\n" -cppcheck --disable=unusedFunction -DPANDA -DSTM32F4 -DUID_BASE $PANDA_DIR/board/main.c +cppcheck $PANDA_OPTS -DSTM32F4 -DUID_BASE $PANDA_DIR/board/main.c printf "\n${GREEN}** PANDA H7 CODE **${NC}\n" -cppcheck --disable=unusedFunction -DPANDA -DSTM32H7 -DUID_BASE $PANDA_DIR/board/main.c +cppcheck $PANDA_OPTS -DSTM32H7 -DUID_BASE $PANDA_DIR/board/main.c # unused needs to run globally printf "\n${GREEN}** UNUSED ALL CODE **${NC}\n" -cppcheck --enable=unusedFunction --quiet $PANDA_DIR/board/ +cppcheck --enable=unusedFunction $PANDA_DIR/board/ printf "\n${GREEN}Success!${NC} took $SECONDS seconds\n" + From ea646a72b2552a68373cb0620cc352073069501a Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 14:10:38 -0800 Subject: [PATCH 5/6] disable for now --- tests/misra/suppressions.txt | 4 +--- tests/misra/test_misra.sh | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/misra/suppressions.txt b/tests/misra/suppressions.txt index bf4386b80b..d79064f1f2 100644 --- a/tests/misra/suppressions.txt +++ b/tests/misra/suppressions.txt @@ -12,14 +12,12 @@ misra-c2012-20.10 # needed since not all of these suppressions are applicable to all builds unmatchedSuppression -# +# All interrupt handlers are defined, including ones we don't use unusedFunction:*/interrupt_handlers*.h # all of the below suppressions are from new checks introduced after updating # cppcheck from 2.5 -> 2.13. they are listed here to separate the update from # fixing the violations and all are intended to be removed soon after - - misra-config misra-c2012-1.2 # this is from the extensions (e.g. __typeof__) used in the MIN, MAX, ABS, and CLAMP macros misra-c2012-2.5 diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh index e95d7f3104..d44661f6db 100755 --- a/tests/misra/test_misra.sh +++ b/tests/misra/test_misra.sh @@ -53,8 +53,8 @@ printf "\n${GREEN}** PANDA H7 CODE **${NC}\n" cppcheck $PANDA_OPTS -DSTM32H7 -DUID_BASE $PANDA_DIR/board/main.c # unused needs to run globally -printf "\n${GREEN}** UNUSED ALL CODE **${NC}\n" -cppcheck --enable=unusedFunction $PANDA_DIR/board/ +#printf "\n${GREEN}** UNUSED ALL CODE **${NC}\n" +#cppcheck --enable=unusedFunction --quiet $PANDA_DIR/board/ printf "\n${GREEN}Success!${NC} took $SECONDS seconds\n" From 81505e0aba264f2aaecc68741d3a28ff0fd802f6 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Sat, 17 Feb 2024 14:14:25 -0800 Subject: [PATCH 6/6] add back exhaustive --- tests/misra/test_misra.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh index d44661f6db..15bd6aca8a 100755 --- a/tests/misra/test_misra.sh +++ b/tests/misra/test_misra.sh @@ -34,7 +34,7 @@ cppcheck() { $CPPCHECK_DIR/cppcheck --force --inline-suppr -I $PANDA_DIR/board/ \ -I $gcc_inc "$(arm-none-eabi-gcc -print-file-name=include)" \ --suppressions-list=$DIR/suppressions.txt --suppress=*:*inc/* \ - --suppress=*:*include/* --error-exitcode=2 \ + --suppress=*:*include/* --error-exitcode=2 --check-level=exhaustive \ "$@" |& tee $OUTPUT # cppcheck bug: some MISRA errors won't result in the error exit code,