From 4aad54589d0c16e6001f902ca38b1146659536a3 Mon Sep 17 00:00:00 2001 From: Robert Brown Date: Tue, 26 Jun 2018 17:10:40 -0700 Subject: [PATCH 01/14] Add signed value to brake request Prior to this commit the brake request was set to unsigned float which does not exist. This commit fixes that by changing the value to a signed float. --- api/include/can_protocols/oscc.dbc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/can_protocols/oscc.dbc b/api/include/can_protocols/oscc.dbc index 61613101..dbde5789 100644 --- a/api/include/can_protocols/oscc.dbc +++ b/api/include/can_protocols/oscc.dbc @@ -44,7 +44,7 @@ BO_ 113 BRAKE_DISABLE: 8 BRAKE BO_ 114 BRAKE_COMMAND: 8 BRAKE SG_ brake_command_magic : 0|16@1+ (1,0) [0|0] "" BRAKE - SG_ brake_command_pedal_request : 16|32@1+ (1,0) [0|1] "" BRAKE + SG_ brake_command_pedal_request : 16|32@1- (1,0) [0|1] "" BRAKE SG_ brake_command_reserved : 48|16@1+ (1,0) [0|0] "" BRAKE BO_ 115 BRAKE_REPORT: 8 BRAKE From cf05d79eab572c8472d017e851efaef5816a1a09 Mon Sep 17 00:00:00 2001 From: Russell Mull Date: Tue, 24 Jul 2018 18:30:00 -0400 Subject: [PATCH 02/14] Build in docker --- Dockerfile | 50 +++++++++++++++++++++++++ Jenkinsfile | 105 +++++++++++++++++++++++++++------------------------- 2 files changed, 104 insertions(+), 51 deletions(-) create mode 100644 Dockerfile diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 00000000..237cf81f --- /dev/null +++ b/Dockerfile @@ -0,0 +1,50 @@ +FROM ubuntu:16.04 + +WORKDIR /app + +# common packages +RUN apt-get update && \ + apt-get install -y \ + ca-certificates curl file tar clang libclang-dev \ + build-essential cmake libssl-dev zlib1g-dev \ + ruby-dev libboost-dev git wget && \ + rm -rf /var/lib/apt/lists/* + +# install cucumber package +RUN gem install cucumber -v 2.0.0 --no-rdoc --no-ri + +# install rust toolchain +env RUSTUP_HOME=/usr/local/rustup \ + CARGO_HOME=/usr/local/cargo + +RUN curl https://sh.rustup.rs -sSf | \ + sh -s -- --default-toolchain 1.20.0 -y + +env PATH=/usr/local/cargo/bin:${PATH} + +# install arduino toolchain +RUN wget -nv http://arduino.cc/download.php?f=/arduino-1.8.5-linux64.tar.xz -O arduino-1.8.5.tar.xz + +RUN tar -xf arduino-1.8.5.tar.xz && \ + cd arduino-1.8.5 && \ + mkdir -p /usr/share/arduino && \ + cp -R * /usr/share/arduino + + +# Fetch and build cargo deps +RUN mkdir src && echo "fn main() { }" >> build.rs && touch src/tests.rs + +COPY ./firmware/brake/kia_soul_ev_niro/tests/property/Cargo.toml . +RUN cargo build + +COPY ./firmware/brake/kia_soul_petrol/tests/property/Cargo.toml . +RUN cargo build + +COPY ./firmware/common/libs/pid/tests/property/Cargo.toml . +RUN cargo build + +COPY ./firmware/steering/tests/property/Cargo.toml . +RUN cargo build + +COPY ./firmware/throttle/tests/property/Cargo.toml . +RUN cargo build diff --git a/Jenkinsfile b/Jenkinsfile index 5b6866dc..3fe6c198 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,60 +1,63 @@ #!groovy -def cleanCheckout() { - checkout([ - $class: 'GitSCM', - branches: scm.branches, - extensions: scm.extensions + [[$class: 'CleanBeforeCheckout']], - userRemoteConfigs: scm.userRemoteConfigs - ]) -} +node { + checkout scm + + def image = docker.build("cmake-build:${env.BUILD_ID}") + + def builds = [:] + def output = image.inside { + sh returnStdout: true, script: "cmake -LA ./firmware | grep 'VEHICLE_VALUES' | cut -d'=' -f 2" + } + + def platforms = output.trim().tokenize(';') + + for(int j=0; j Date: Tue, 31 Jul 2018 13:56:54 -0700 Subject: [PATCH 03/14] Remove D-CAN filters --- api/src/oscc.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/api/src/oscc.c b/api/src/oscc.c index 237d8ca7..db6ece12 100644 --- a/api/src/oscc.c +++ b/api/src/oscc.c @@ -529,10 +529,7 @@ void oscc_update_status( int sig, siginfo_t *siginfo, void *context ) while( vehicle_can_bytes > 0 ) { - if ( obd_frame_callback != NULL && - ( rx_frame.can_id == KIA_SOUL_OBD_WHEEL_SPEED_CAN_ID || - rx_frame.can_id == KIA_SOUL_OBD_BRAKE_PRESSURE_CAN_ID || - rx_frame.can_id == KIA_SOUL_OBD_STEERING_WHEEL_ANGLE_CAN_ID ) ) + if ( obd_frame_callback != NULL ) { obd_frame_callback( &rx_frame ); } From ca8bca06cd7cd7c54102c95487af9cfe6b9006ed Mon Sep 17 00:00:00 2001 From: Boris Bidault Date: Tue, 31 Jul 2018 13:58:09 -0700 Subject: [PATCH 04/14] Add Soul EV throttle CAN-ID --- api/include/vehicles/kia_soul_ev.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/include/vehicles/kia_soul_ev.h b/api/include/vehicles/kia_soul_ev.h index 7af5d5f8..1b919006 100644 --- a/api/include/vehicles/kia_soul_ev.h +++ b/api/include/vehicles/kia_soul_ev.h @@ -49,6 +49,12 @@ */ #define KIA_SOUL_OBD_BRAKE_PRESSURE_CAN_ID ( 0x220 ) +/* + * @brief ID of the Kia Soul's OBD throttle pressure CAN frame. + * + */ +#define KIA_SOUL_OBD_THROTTLE_PRESSURE_CAN_ID ( 0x200 ) + /* * @brief Factor to scale OBD steering angle to degrees * From c8c742d6b1986733d233b57b51f4a33c07b0f1bb Mon Sep 17 00:00:00 2001 From: Shea Newton Date: Tue, 31 Jul 2018 15:10:33 -0700 Subject: [PATCH 05/14] Increase required steering torque to override Prior to this commit custom applications that required rapid and changes to the steering wheel position were able to programatically apply enough torque to artificially trigger operator override. This resulted in applications where the steering module disabled in unintended ways. This commit addresses that by increasing the torque required to trigger operator override without significantly impeding the a safety driver's ability to trigger override by manually turning the steering wheel. After this commit applications that require rapid changes to steering angle position should not result in any unintended disabling of the steering module. --- api/include/vehicles/kia_niro.h | 2 +- api/include/vehicles/kia_soul_ev.h | 2 +- api/include/vehicles/kia_soul_petrol.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/include/vehicles/kia_niro.h b/api/include/vehicles/kia_niro.h index 74b58310..9067ffcf 100644 --- a/api/include/vehicles/kia_niro.h +++ b/api/include/vehicles/kia_niro.h @@ -366,7 +366,7 @@ typedef struct * override. * */ -#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 1600 ) +#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 2000 ) diff --git a/api/include/vehicles/kia_soul_ev.h b/api/include/vehicles/kia_soul_ev.h index 1b919006..3a1dd80e 100644 --- a/api/include/vehicles/kia_soul_ev.h +++ b/api/include/vehicles/kia_soul_ev.h @@ -345,7 +345,7 @@ typedef struct * override. * */ -#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 1600 ) +#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 2000 ) diff --git a/api/include/vehicles/kia_soul_petrol.h b/api/include/vehicles/kia_soul_petrol.h index 3bffdc17..60e4929d 100644 --- a/api/include/vehicles/kia_soul_petrol.h +++ b/api/include/vehicles/kia_soul_petrol.h @@ -407,7 +407,7 @@ typedef struct * override. * */ -#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 1600 ) +#define TORQUE_DIFFERENCE_OVERRIDE_THRESHOLD ( 2000 ) From eb6b8d0642609abcd3b6812891e7678c4e581bde Mon Sep 17 00:00:00 2001 From: Robert Brown Date: Wed, 1 Aug 2018 10:04:57 -0700 Subject: [PATCH 06/14] Add docker.build call Prior to this commit the parallel builds did not have a docker.build command causing multiple workers to be missing a docker container to work in. This commit fixes that by adding the build command to ensure that each worker node has a container to use. --- Jenkinsfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index 3fe6c198..f06d9479 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -19,6 +19,8 @@ node { node { checkout scm + image = docker.build("cmake-build:${env.BUILD_ID}") + stage("Build ${platform}") { image.inside { sh "cd firmware && \ From 67e96b3c70764f0d1768c7b545d2e0588ef31f4c Mon Sep 17 00:00:00 2001 From: Boris Bidault Date: Thu, 2 Aug 2018 14:02:49 -0700 Subject: [PATCH 07/14] Add Niro throttle CAN-ID --- api/include/vehicles/kia_niro.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api/include/vehicles/kia_niro.h b/api/include/vehicles/kia_niro.h index 74b58310..7647cef8 100644 --- a/api/include/vehicles/kia_niro.h +++ b/api/include/vehicles/kia_niro.h @@ -49,6 +49,12 @@ */ #define KIA_SOUL_OBD_BRAKE_PRESSURE_CAN_ID ( 0x220 ) +/* + * @brief ID of the Kia Niro's OBD throttle pressure CAN frame. + * + */ +#define KIA_SOUL_OBD_THROTTLE_PRESSURE_CAN_ID ( 0x371 ) + /* * @brief ID of the Kia Niro's OBD speed CAN frame. * From 3e05c6acd85f2c40d5891080f5303839ee885bc6 Mon Sep 17 00:00:00 2001 From: Robert Brown Date: Thu, 9 Aug 2018 13:17:26 -0700 Subject: [PATCH 08/14] Clean working directory Prior to this commit failed builds would leave a dirty working directory. This commit fixes that by adding a try, finally block to make sure the directory is cleaned. --- Jenkinsfile | 72 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index f06d9479..037a169f 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -19,47 +19,57 @@ node { node { checkout scm - image = docker.build("cmake-build:${env.BUILD_ID}") + try { + image = docker.build("cmake-build:${env.BUILD_ID}") - stage("Build ${platform}") { - image.inside { - sh "cd firmware && \ - rm -rf build_${platform} && \ - mkdir build_${platform} && \ - cd build_${platform} && \ - cmake -DVEHICLE=${platform} -DCMAKE_BUILD_TYPE=Release .. && \ - make" + stage("Build ${platform}") { + image.inside { + sh "cd firmware && \ + rm -rf build_${platform} && \ + mkdir build_${platform} && \ + cd build_${platform} && \ + cmake -DVEHICLE=${platform} -DCMAKE_BUILD_TYPE=Release .. && \ + make" - echo "${platform}: Build Complete!" + echo "${platform}: Build Complete!" + } } - } - stage("Test ${platform} unit tests") { - image.inside { - sh "cd firmware && \ - rm -rf build_${platform}_tests && \ - mkdir build_${platform}_tests && \ - cd build_${platform}_tests && \ - cmake -DVEHICLE=${platform} \ - -DTESTS=ON \ - -DPORT_SUFFIX=${EXECUTOR_NUMBER}${platform_idx} \ - -DCMAKE_BUILD_TYPE=Release \ - .. && \ - make run-unit-tests" - echo "${platform}: Unit Tests Complete!" + stage("Test ${platform} unit tests") { + image.inside { + sh "cd firmware && \ + rm -rf build_${platform}_tests && \ + mkdir build_${platform}_tests && \ + cd build_${platform}_tests && \ + cmake -DVEHICLE=${platform} \ + -DTESTS=ON \ + -DPORT_SUFFIX=${EXECUTOR_NUMBER}${platform_idx} \ + -DCMAKE_BUILD_TYPE=Release \ + .. && \ + make run-unit-tests" + echo "${platform}: Unit Tests Complete!" + } } - } - stage("Test ${platform} property-based tests") { - image.inside("--user root:root") { - sh "cd firmware/build_${platform}_tests && \ - make run-property-tests" - echo "${platform}: Property-Based Tests Complete!" + stage("Test ${platform} property-based tests") { + image.inside("--user root:root") { + sh "cd firmware/build_${platform}_tests && \ + make run-property-tests" + echo "${platform}: Property-Based Tests Complete!" + } } } + finally { + deleteDir() + } } } } - parallel builds + try { + parallel builds + } + finally { + deleteDir() + } } From b3bbab871e4320ae8541133fb6b2638442820a67 Mon Sep 17 00:00:00 2001 From: Robert Brown Date: Thu, 9 Aug 2018 13:19:20 -0700 Subject: [PATCH 09/14] Move checkout inside try block Prior to this commit the checkout call was outside the try block. This commit fixes that by moving the checkout into the try block incase it fails and the working directory still needs to be cleaned up. --- Jenkinsfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 037a169f..e05c7292 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -17,9 +17,9 @@ node { def platform = platforms[platform_idx] builds[platform] = { node { - checkout scm - try { + checkout scm + image = docker.build("cmake-build:${env.BUILD_ID}") stage("Build ${platform}") { From 2f1dc29711557f588bca32cace533de319ea752a Mon Sep 17 00:00:00 2001 From: Boris Bidault Date: Mon, 20 Aug 2018 12:04:34 -0700 Subject: [PATCH 10/14] Add EV & Niro vehicle speed can ids --- api/include/vehicles/kia_niro.h | 2 +- api/include/vehicles/kia_soul_ev.h | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/api/include/vehicles/kia_niro.h b/api/include/vehicles/kia_niro.h index 8eebd66d..c73ba1d5 100644 --- a/api/include/vehicles/kia_niro.h +++ b/api/include/vehicles/kia_niro.h @@ -59,7 +59,7 @@ * @brief ID of the Kia Niro's OBD speed CAN frame. * */ -#define KIA_SOUL_OBD_SPEED_CAN_ID ( 0x371 ) +#define KIA_SOUL_OBD_SPEED_CAN_ID ( 0x52A ) /* * @brief Factor to scale OBD steering angle to degrees diff --git a/api/include/vehicles/kia_soul_ev.h b/api/include/vehicles/kia_soul_ev.h index 3a1dd80e..7d3ad783 100644 --- a/api/include/vehicles/kia_soul_ev.h +++ b/api/include/vehicles/kia_soul_ev.h @@ -55,6 +55,12 @@ */ #define KIA_SOUL_OBD_THROTTLE_PRESSURE_CAN_ID ( 0x200 ) +/* + * @brief ID of the Kia Niro's OBD speed CAN frame. + * + */ +#define KIA_SOUL_OBD_SPEED_CAN_ID ( 0x524 ) + /* * @brief Factor to scale OBD steering angle to degrees * From 8ba287206d45cb534c632d600c83c11c0ff55ff6 Mon Sep 17 00:00:00 2001 From: evanlivingston Date: Tue, 21 Aug 2018 15:20:29 -0700 Subject: [PATCH 11/14] reduce BREAK_PEDAL_OVERRIDE_THRESHOLD --- api/include/vehicles/kia_niro.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/vehicles/kia_niro.h b/api/include/vehicles/kia_niro.h index c73ba1d5..d29aac16 100644 --- a/api/include/vehicles/kia_niro.h +++ b/api/include/vehicles/kia_niro.h @@ -242,7 +242,7 @@ typedef struct * @brief Value of the accelerator position that indicates operator override. [steps] * */ -#define BRAKE_PEDAL_OVERRIDE_THRESHOLD ( 200 ) +#define BRAKE_PEDAL_OVERRIDE_THRESHOLD ( 130 ) /* * @brief Minimum value of the high spoof signal that activates the brake lights. [steps] From 0a60389b0f1aad35e036a9586c595c264f3bfe28 Mon Sep 17 00:00:00 2001 From: Robert Brown Date: Tue, 21 Aug 2018 15:33:11 -0700 Subject: [PATCH 12/14] Fix jenkins workspace cleanup Prior to this commit some builds would fail trying to clean up the workspace directory. This commit fixes that by removing the inner cleanup since the try block for the parallel builds should catch any errors from workspace builds. --- Jenkinsfile | 69 +++++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index e05c7292..221c543d 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -17,50 +17,45 @@ node { def platform = platforms[platform_idx] builds[platform] = { node { - try { - checkout scm - - image = docker.build("cmake-build:${env.BUILD_ID}") + checkout scm - stage("Build ${platform}") { - image.inside { - sh "cd firmware && \ - rm -rf build_${platform} && \ - mkdir build_${platform} && \ - cd build_${platform} && \ - cmake -DVEHICLE=${platform} -DCMAKE_BUILD_TYPE=Release .. && \ - make" + image = docker.build("cmake-build:${env.BUILD_ID}") - echo "${platform}: Build Complete!" - } - } + stage("Build ${platform}") { + image.inside { + sh "cd firmware && \ + rm -rf build_${platform} && \ + mkdir build_${platform} && \ + cd build_${platform} && \ + cmake -DVEHICLE=${platform} -DCMAKE_BUILD_TYPE=Release .. && \ + make" - stage("Test ${platform} unit tests") { - image.inside { - sh "cd firmware && \ - rm -rf build_${platform}_tests && \ - mkdir build_${platform}_tests && \ - cd build_${platform}_tests && \ - cmake -DVEHICLE=${platform} \ - -DTESTS=ON \ - -DPORT_SUFFIX=${EXECUTOR_NUMBER}${platform_idx} \ - -DCMAKE_BUILD_TYPE=Release \ - .. && \ - make run-unit-tests" - echo "${platform}: Unit Tests Complete!" - } + echo "${platform}: Build Complete!" } + } - stage("Test ${platform} property-based tests") { - image.inside("--user root:root") { - sh "cd firmware/build_${platform}_tests && \ - make run-property-tests" - echo "${platform}: Property-Based Tests Complete!" - } + stage("Test ${platform} unit tests") { + image.inside { + sh "cd firmware && \ + rm -rf build_${platform}_tests && \ + mkdir build_${platform}_tests && \ + cd build_${platform}_tests && \ + cmake -DVEHICLE=${platform} \ + -DTESTS=ON \ + -DPORT_SUFFIX=${EXECUTOR_NUMBER}${platform_idx} \ + -DCMAKE_BUILD_TYPE=Release \ + .. && \ + make run-unit-tests" + echo "${platform}: Unit Tests Complete!" } } - finally { - deleteDir() + + stage("Test ${platform} property-based tests") { + image.inside("--user root:root") { + sh "cd firmware/build_${platform}_tests && \ + make run-property-tests" + echo "${platform}: Property-Based Tests Complete!" + } } } } From fcfe5f73adae079f4f4883c67b71a591a39cc796 Mon Sep 17 00:00:00 2001 From: Boris Bidault Date: Wed, 19 Sep 2018 13:50:55 -0700 Subject: [PATCH 13/14] correct comment --- api/include/vehicles/kia_niro.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/include/vehicles/kia_niro.h b/api/include/vehicles/kia_niro.h index d29aac16..9fce660a 100644 --- a/api/include/vehicles/kia_niro.h +++ b/api/include/vehicles/kia_niro.h @@ -239,7 +239,7 @@ typedef struct BRAKE_SPOOF_HIGH_SIGNAL_VOLTAGE_MIN ) /* - * @brief Value of the accelerator position that indicates operator override. [steps] + * @brief Value of the brake position that indicates operator override. [steps] * */ #define BRAKE_PEDAL_OVERRIDE_THRESHOLD ( 130 ) From e82ac406a796c2df425a132d74c69cce199fdf5a Mon Sep 17 00:00:00 2001 From: Chris Otos Date: Mon, 24 Sep 2018 17:19:44 -0700 Subject: [PATCH 14/14] Throttle operator override not being published Prior to thie commit the signal, "throttle_operator_override" in the message, "THROTTLE_REPORT(0x93)" did not report a status of driver's throttle override, whereas, the signal, "throttle_report_enabled" in the same message reports properly. The override report signals of steering/brake works well. The operator override was being generated but not published. Fix: Used operator override logic the brake is using. Verification: Ran `https://github.com/PolySync/oscc-check/blob/master/oscc-check.py` to enable all modules. Performed a throttle operator override. Verified throttle operator overide ``` $ candump can0,093:7FF can0 093 [8] 05 CC 01 00 00 B5 08 B8 can0 093 [8] 05 CC 01 00 00 B5 08 B8 can0 093 [8] 05 CC 01 00 00 5F 02 1F can0 093 [8] 05 CC 00 01 02 B5 08 B8 can0 093 [8] 05 CC 00 01 02 B5 08 B8 can0 093 [8] 05 CC 00 01 02 B5 08 B8 ``` --- firmware/throttle/src/throttle_control.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/firmware/throttle/src/throttle_control.cpp b/firmware/throttle/src/throttle_control.cpp index 3745a0e8..4c15b5b3 100644 --- a/firmware/throttle/src/throttle_control.cpp +++ b/firmware/throttle/src/throttle_control.cpp @@ -61,8 +61,7 @@ void check_for_faults( void ) DEBUG_PRINTLN( "Bad value read from accelerator position sensor" ); } - else if ( operator_overridden == true - && g_throttle_control_state.operator_override == false ) + else if ( operator_overridden == true ) { disable_control( );