From 78c24982a92443e89a509af46ed1b07c0ef8b425 Mon Sep 17 00:00:00 2001 From: Adeeb Shihadeh Date: Mon, 21 Jun 2021 13:54:09 -0700 Subject: [PATCH] Remove non-EON panda build (#671) * Remove non-EON panda build * missed that * fixup readme * more readme * simplify ci * add USB command to disable heartbeat checks in debug mode * clean that up too * more cleanup * fix path * disable heartbeat in set_safety_mode * more red * remove one more EON ref --- Dockerfile | 3 -- Jenkinsfile | 44 ++++----------- README.md | 20 +++---- board/SConscript | 8 --- board/boards/white.h | 72 +------------------------ board/main.c | 111 +++++++++++++------------------------- board/main_declarations.h | 1 + python/__init__.py | 9 +++- run_automated_tests.sh | 17 ------ tests/automated/test.sh | 3 ++ tests/misra/test_misra.sh | 2 +- 11 files changed, 72 insertions(+), 218 deletions(-) delete mode 100755 run_automated_tests.sh create mode 100755 tests/automated/test.sh diff --git a/Dockerfile b/Dockerfile index 3ecff0f5c..c0ba30013 100644 --- a/Dockerfile +++ b/Dockerfile @@ -63,9 +63,6 @@ RUN pip install --upgrade pip==18.0 COPY requirements.txt /tmp/ RUN pip install -r /tmp/requirements.txt -RUN mkdir -p /home/batman -ENV HOME /home/batman - ENV PYTHONPATH /tmp:$PYTHONPATH RUN cd /tmp && git clone https://github.com/commaai/panda_jungle.git diff --git a/Jenkinsfile b/Jenkinsfile index 6731f2502..6702b870f 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,13 +1,7 @@ pipeline { agent any environment { - AUTHOR = """${sh( - returnStdout: true, - script: "git --no-pager show -s --format='%an' ${GIT_COMMIT}" - ).trim()}""" - DOCKER_IMAGE_TAG = "panda:build-${env.GIT_COMMIT}" - DOCKER_NAME = "panda-test-${env.GIT_COMMIT}" } stages { stage('Build Docker Image') { @@ -20,41 +14,21 @@ pipeline { } } } - stage('Test Dev Build') { + stage('HITL tests') { steps { - lock(resource: "Pandas", inversePrecedence: true, quantity: 1){ - timeout(time: 60, unit: 'MINUTES') { + lock(resource: "pandas", inversePrecedence: true, quantity: 1) { + timeout(time: 20, unit: 'MINUTES') { script { - sh "docker run --name ${env.DOCKER_NAME} --privileged --volume /dev/bus/usb:/dev/bus/usb --volume /var/run/dbus:/var/run/dbus --net host ${env.DOCKER_IMAGE_TAG} bash -c 'cd /tmp/panda; scons; ./run_automated_tests.sh'" - sh "docker cp ${env.DOCKER_NAME}:/tmp/panda/nosetests.xml test_results_dev.xml" - sh "docker rm ${env.DOCKER_NAME}" + sh "docker run --rm --privileged \ + --volume /dev/bus/usb:/dev/bus/usb \ + --volume /var/run/dbus:/var/run/dbus \ + --net host \ + ${env.DOCKER_IMAGE_TAG} \ + bash -c 'cd /tmp/panda && scons && ./tests/automated/test.sh'" } } } } } - stage('Test EON Build') { - steps { - lock(resource: "Pandas", inversePrecedence: true, quantity: 1){ - timeout(time: 60, unit: 'MINUTES') { - script { - sh "docker run --name ${env.DOCKER_NAME} --privileged --volume /dev/bus/usb:/dev/bus/usb --volume /var/run/dbus:/var/run/dbus --net host ${env.DOCKER_IMAGE_TAG} bash -c 'touch /EON; cd /tmp/panda; scons; ./run_automated_tests.sh'" - sh "docker cp ${env.DOCKER_NAME}:/tmp/panda/nosetests.xml test_results_eon.xml" - sh "docker rm ${env.DOCKER_NAME}" - } - } - } - } - } - } - post { - failure { - script { - sh "docker rm ${env.DOCKER_NAME} || true" - } - } - always { - junit "test_results*.xml" - } } } diff --git a/README.md b/README.md index b8a798094..cc7d0171b 100644 --- a/README.md +++ b/README.md @@ -58,10 +58,13 @@ As a universal car interface, it should support every reasonable software interf ## Directory structure -- board -- Code that runs on the STM32 -- drivers -- Drivers (not needed for use with python) -- python   -- Python userspace library for interfacing with the panda -- tests -- Tests and helper programs for panda +``` +. +├── board # Code that runs on the STM32 +├── drivers # Drivers (not needed for use with python) +├── python # Python userspace library for interfacing with the panda +├── tests # Tests and helper programs for panda +``` ## Programming @@ -79,20 +82,19 @@ Safety modes optionally supports `controls_allowed`, which allows or blocks a su ## Code Rigor -When compiled from a comma device, the panda FW is configured and optimized (at compile time) for its use in -conjuction with [openpilot](https://github.com/commaai/openpilot). The panda FW, through its safety model, provides and enforces the +The panda firmware is written for its use in conjuction with [openpilot](https://github.com/commaai/openpilot). The panda firmware, through its safety model, provides and enforces the [openpilot safety](https://github.com/commaai/openpilot/blob/devel/SAFETY.md). Due to its critical function, it's important that the application code rigor within the `board` folder is held to high standards. These are the [CI regression tests](https://github.com/commaai/panda/actions) we have in place: * A generic static code analysis is performed by [cppcheck](https://github.com/danmar/cppcheck/). * In addition, [cppcheck](https://github.com/danmar/cppcheck/) has a specific addon to check for [MISRA C:2012](https://www.misra.org.uk/MISRAHome/MISRAC2012/tabid/196/Default.aspx) violations. See [current coverage](https://github.com/commaai/panda/blob/master/tests/misra/coverage_table). -* Compiler options are relatively strict: the flags `-Wall -Wextra -Wstrict-prototypes -Werror` are enforced on board and pedal Makefiles. +* Compiler options are relatively strict: the flags `-Wall -Wextra -Wstrict-prototypes -Werror` are enforced. * The [safety logic](https://github.com/commaai/panda/tree/master/board/safety) is tested and verified by [unit tests](https://github.com/commaai/panda/tree/master/tests/safety) for each supported car variant. * A recorded drive for each supported car variant is [replayed through the safety logic](https://github.com/commaai/panda/tree/master/tests/safety_replay) to ensure that the behavior remains unchanged. * An internal Hardware-in-the-loop test, which currently only runs on pull requests opened by comma.ai's organization members, verifies the following functionalities: - * compiling the code in various configuration and flashing it both through USB. - * Receiving, sending and forwarding CAN messages on all buses, over USB. + * compiling the code and flashing it through USB. + * receiving, sending, and forwarding CAN messages on all buses, over USB. In addition, we run the [pylint](https://www.pylint.org/) and [flake8](https://github.com/PyCQA/flake8) linters on all python files within the panda repo. diff --git a/board/SConscript b/board/SConscript index 167da9e24..11fdc2c6b 100644 --- a/board/SConscript +++ b/board/SConscript @@ -1,10 +1,6 @@ import os import subprocess -EON = os.path.isfile('/EON') -TICI = os.path.isfile('/TICI') -PC = not (EON or TICI) - PREFIX = "arm-none-eabi-" BUILDER = "DEV" @@ -36,10 +32,6 @@ else: "-g", ] - if not PC: - PROJECT_FLAGS += ["-DEON"] - BUILDER = "EON" - def get_version(builder, build_type): version_file = File('../VERSION').srcnode().abspath diff --git a/board/boards/white.h b/board/boards/white.h index 44d8f512b..61c1c05c1 100644 --- a/board/boards/white.h +++ b/board/boards/white.h @@ -78,13 +78,6 @@ void white_set_gps_mode(uint8_t mode) { set_gpio_output(GPIOC, 14, 0); set_gpio_output(GPIOC, 5, 0); break; -#ifndef EON - case GPS_ENABLED: - // ESP ON - set_gpio_output(GPIOC, 14, 1); - set_gpio_output(GPIOC, 5, 1); - break; -#endif case GPS_BOOTMODE: set_gpio_output(GPIOC, 14, 1); set_gpio_output(GPIOC, 5, 0); @@ -158,71 +151,8 @@ uint32_t white_read_current(void){ return adc_get(ADCCHAN_CURRENT); } -uint32_t marker = 0; void white_usb_power_mode_tick(uint32_t uptime){ - - // on EON or BOOTSTUB, no state machine -#if !defined(BOOTSTUB) && !defined(EON) - #define CURRENT_THRESHOLD 0xF00U - #define CLICKS 5U // 5 seconds to switch modes - - uint32_t current = white_read_current(); - - // ~0x9a = 500 ma - // puth(current); puts("\n"); - - switch (usb_power_mode) { - case USB_POWER_CLIENT: - if ((uptime - marker) >= CLICKS) { - if (!is_enumerated) { - puts("USBP: didn't enumerate, switching to CDP mode\n"); - // switch to CDP - white_set_usb_power_mode(USB_POWER_CDP); - marker = uptime; - } - } - // keep resetting the timer if it's enumerated - if (is_enumerated) { - marker = uptime; - } - break; - case USB_POWER_CDP: - // been CLICKS clicks since we switched to CDP - if ((uptime - marker) >= CLICKS) { - // measure current draw, if positive and no enumeration, switch to DCP - if (!is_enumerated && (current < CURRENT_THRESHOLD)) { - puts("USBP: no enumeration with current draw, switching to DCP mode\n"); - white_set_usb_power_mode(USB_POWER_DCP); - marker = uptime; - } - } - // keep resetting the timer if there's no current draw in CDP - if (current >= CURRENT_THRESHOLD) { - marker = uptime; - } - break; - case USB_POWER_DCP: - // been at least CLICKS clicks since we switched to DCP - if ((uptime - marker) >= CLICKS) { - // if no current draw, switch back to CDP - if (current >= CURRENT_THRESHOLD) { - puts("USBP: no current draw, switching back to CDP mode\n"); - white_set_usb_power_mode(USB_POWER_CDP); - marker = uptime; - } - } - // keep resetting the timer if there's current draw in DCP - if (current < CURRENT_THRESHOLD) { - marker = uptime; - } - break; - default: - puts("USB power mode invalid\n"); // set_usb_power_mode prevents assigning invalid values - break; - } -#else UNUSED(uptime); -#endif } void white_set_ir_power(uint8_t percentage){ @@ -313,7 +243,7 @@ void white_grey_common_init(void) { // Init usb power mode uint32_t voltage = adc_get_voltage(); // Init in CDP mode only if panda is powered by 12V. - // Otherwise a PC would not be able to flash a standalone panda with EON build + // Otherwise a PC would not be able to flash a standalone panda if (voltage > 8000U) { // 8V threshold white_set_usb_power_mode(USB_POWER_CDP); } else { diff --git a/board/main.c b/board/main.c index 8337bd169..02b060fea 100644 --- a/board/main.c +++ b/board/main.c @@ -1,4 +1,3 @@ -//#define EON //#define PANDA // ********************* Includes ********************* @@ -31,10 +30,6 @@ #include "gpio.h" -#ifndef EON -#include "drivers/spi.h" -#endif - #include "power_saving.h" #include "safety.h" @@ -601,6 +596,7 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired) { heartbeat_counter = 0U; heartbeat_lost = false; + heartbeat_disabled = false; break; } // **** 0xf4: k-line/l-line 5 baud initialization @@ -626,6 +622,12 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired) case 0xf7: green_led_enabled = (setup->b.wValue.w != 0U); break; +#ifdef ALLOW_DEBUG + // **** 0xf8: disable heartbeat checks + case 0xf8: + heartbeat_disabled = true; + break; +#endif default: puts("NO HANDLER "); puth(setup->b.bRequest); @@ -635,38 +637,6 @@ int usb_cb_control_msg(USB_Setup_TypeDef *setup, uint8_t *resp, bool hardwired) return resp_len; } -#ifndef EON -int spi_cb_rx(uint8_t *data, int len, uint8_t *data_out) { - // data[0] = endpoint - // data[2] = length - // data[4:] = data - UNUSED(len); - int resp_len = 0; - switch (data[0]) { - case 0: - // control transfer - resp_len = usb_cb_control_msg((USB_Setup_TypeDef *)(data+4), data_out, 0); - break; - case 1: - // ep 1, read - resp_len = usb_cb_ep1_in(data_out, 0x40, 0); - break; - case 2: - // ep 2, send serial - usb_cb_ep2_out(data+4, data[2], 0); - break; - case 3: - // ep 3, send CAN - usb_cb_ep3_out(data+4, data[2], 0); - break; - default: - puts("SPI data invalid"); - break; - } - return resp_len; -} -#endif - // ***************************** main code ***************************** // cppcheck-suppress unusedFunction ; used in headers not included in cppcheck @@ -679,9 +649,9 @@ void __attribute__ ((noinline)) enable_fpu(void) { SCB->CPACR |= ((3UL << (10U * 2U)) | (3UL << (11U * 2U))); } -// go into SILENT when the EON does not send a heartbeat for this amount of seconds. -#define EON_HEARTBEAT_IGNITION_CNT_ON 5U -#define EON_HEARTBEAT_IGNITION_CNT_OFF 2U +// go into SILENT when heartbeat isn't received for this amount of seconds. +#define HEARTBEAT_IGNITION_CNT_ON 5U +#define HEARTBEAT_IGNITION_CNT_OFF 2U // called at 8Hz uint8_t loop_counter = 0U; @@ -724,40 +694,39 @@ void TIM1_BRK_TIM9_IRQ_Handler(void) { heartbeat_counter += 1U; } - #ifdef EON - // check heartbeat counter if we are running EON code. - // if the heartbeat has been gone for a while, go to SILENT safety mode and enter power save - if (heartbeat_counter >= (check_started() ? EON_HEARTBEAT_IGNITION_CNT_ON : EON_HEARTBEAT_IGNITION_CNT_OFF)) { - puts("EON hasn't sent a heartbeat for 0x"); - puth(heartbeat_counter); - puts(" seconds. Safety is set to SILENT mode.\n"); - if (current_safety_mode != SAFETY_SILENT) { - set_safety_mode(SAFETY_SILENT, 0U); - } - if (power_save_status != POWER_SAVE_STATUS_ENABLED) { - set_power_save_state(POWER_SAVE_STATUS_ENABLED); + if (!heartbeat_disabled) { + // if the heartbeat has been gone for a while, go to SILENT safety mode and enter power save + if (heartbeat_counter >= (check_started() ? HEARTBEAT_IGNITION_CNT_ON : HEARTBEAT_IGNITION_CNT_OFF)) { + puts("device hasn't sent a heartbeat for 0x"); + puth(heartbeat_counter); + puts(" seconds. Safety is set to SILENT mode.\n"); + if (current_safety_mode != SAFETY_SILENT) { + set_safety_mode(SAFETY_SILENT, 0U); + } + if (power_save_status != POWER_SAVE_STATUS_ENABLED) { + set_power_save_state(POWER_SAVE_STATUS_ENABLED); + } + + // set flag to indicate the heartbeat was lost + heartbeat_lost = true; + + // Also disable IR when the heartbeat goes missing + current_board->set_ir_power(0U); + + // If enumerated but no heartbeat (phone up, boardd not running), turn the fan on to cool the device + if(usb_enumerated()){ + current_board->set_fan_power(50U); + } else { + current_board->set_fan_power(0U); + } } - // set flag to indicate the heartbeat was lost - heartbeat_lost = true; - - // Also disable IR when the heartbeat goes missing - current_board->set_ir_power(0U); - - // If enumerated but no heartbeat (phone up, boardd not running), turn the fan on to cool the device - if(usb_enumerated()){ - current_board->set_fan_power(50U); - } else { - current_board->set_fan_power(0U); + // enter CDP mode when car starts to ensure we are charging a turned off EON + if (check_started() && (usb_power_mode != USB_POWER_CDP)) { + current_board->set_usb_power_mode(USB_POWER_CDP); } } - // enter CDP mode when car starts to ensure we are charging a turned off EON - if (check_started() && (usb_power_mode != USB_POWER_CDP)) { - current_board->set_usb_power_mode(USB_POWER_CDP); - } - #endif - // check registers check_registers(); @@ -854,10 +823,6 @@ int main(void) { // enable CAN TXs current_board->enable_can_transceivers(true); -#ifndef EON - spi_init(); -#endif - // 8hz timer_init(TIM9, 183); NVIC_EnableIRQ(TIM1_BRK_TIM9_IRQn); diff --git a/board/main_declarations.h b/board/main_declarations.h index 8181d6e32..b5215512a 100644 --- a/board/main_declarations.h +++ b/board/main_declarations.h @@ -14,5 +14,6 @@ bool is_enumerated = 0; uint32_t heartbeat_counter = 0; uint32_t uptime_cnt = 0; bool heartbeat_lost = false; +bool heartbeat_disabled = false; bool siren_enabled = false; bool green_led_enabled = false; diff --git a/python/__init__.py b/python/__init__.py index 1ef4d2a4a..9ec5633c2 100644 --- a/python/__init__.py +++ b/python/__init__.py @@ -420,8 +420,10 @@ class Panda(object): self._handle.controlWrite(Panda.REQUEST_OUT, 0xda, int(bootmode), 0, b'') time.sleep(0.2) - def set_safety_mode(self, mode=SAFETY_SILENT): + def set_safety_mode(self, mode=SAFETY_SILENT, disable_heartbeat=True): self._handle.controlWrite(Panda.REQUEST_OUT, 0xdc, mode, 0, b'') + if disable_heartbeat: + self.set_heartbeat_disabled() def set_can_forwarding(self, from_bus, to_bus): # TODO: This feature may not work correctly with saturated buses @@ -619,6 +621,11 @@ class Panda(object): def send_heartbeat(self): self._handle.controlWrite(Panda.REQUEST_OUT, 0xf3, 0, 0, b'') + # disable heartbeat checks for use outside of openpilot + # sending a heartbeat will reenable the checks + def set_heartbeat_disabled(self): + self._handle.controlWrite(Panda.REQUEST_OUT, 0xf8, 0, 0, b'') + # ******************* RTC ******************* def set_datetime(self, dt): self._handle.controlWrite(Panda.REQUEST_OUT, 0xa1, int(dt.year), 0, b'') diff --git a/run_automated_tests.sh b/run_automated_tests.sh deleted file mode 100755 index 6bfdcf533..000000000 --- a/run_automated_tests.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/bin/bash -e -TEST_FILENAME=${TEST_FILENAME:-nosetests.xml} -if [ -f "/EON" ]; then - TESTSUITE_NAME="Panda_Test-EON" -else - TESTSUITE_NAME="Panda_Test-DEV" -fi - -TEST_SCRIPTS=$(ls tests/automated/$1*.py) - -IFS=$'\n' -for NAME in $(nmcli --fields NAME con show | grep panda | awk '{$1=$1};1') -do - nmcli connection delete "$NAME" -done - -PYTHONPATH="." $(which nosetests) -v --with-xunit --xunit-file=./$TEST_FILENAME --xunit-testsuite-name=$TESTSUITE_NAME -s $TEST_SCRIPTS diff --git a/tests/automated/test.sh b/tests/automated/test.sh new file mode 100755 index 000000000..9a99c6b24 --- /dev/null +++ b/tests/automated/test.sh @@ -0,0 +1,3 @@ +#!/bin/bash -e +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" +$(which nosetests) -v -s $(ls $DIR/$1*.py) diff --git a/tests/misra/test_misra.sh b/tests/misra/test_misra.sh index ec9dd8ff6..71b78a097 100755 --- a/tests/misra/test_misra.sh +++ b/tests/misra/test_misra.sh @@ -8,7 +8,7 @@ mkdir /tmp/misra || true #python tests/misra/cppcheck/addons/misra.py -generate-table > tests/misra/coverage_table printf "\nPANDA CODE\n" -cppcheck -DPANDA -UPEDAL -DCAN3 -DUID_BASE -DEON \ +cppcheck -DPANDA -UPEDAL -DCAN3 -DUID_BASE \ --suppressions-list=suppressions.txt \ --dump --enable=all --inline-suppr --force \ $PANDA_DIR/board/main.c 2>/tmp/misra/cppcheck_output.txt