Skip to content

Commit 6f83fdf

Browse files
sshanesshane
authored
Move car safety modes to opendbc (#1713)
* move safety tests * move libsafety * move safety * rename imports * copy over needed (minimalized) board and driver code * dont test safety here * add new job for safety tests fix * try fix * ubsan * ? ? * missing cffi * should be final fix * mac fix * no mac * use setup script * no cd * this is the correct way to do it * add misra * misra fixes * run * setup misra * add missing files * is this used? * add that * Revert "is this used?" This reverts commit 2f34762. * need this * misra mutation test * fix * no race conditions * test * cache cppcheck fix * setup * good timeouts * mutation test * fix * no * Revert "no" This reverts commit 39e10a0. * already tested * move Safety Model readme section to opendbc * fix * fix * disable mutation tests for merge * namespace * test no cache * 1m 1m
1 parent 0fdb59f commit 6f83fdf

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

73 files changed

+12201
-8
lines changed

‎.github/workflows/tests.yml

+83
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,86 @@ jobs:
2020
- uses: commaai/timeout@v1
2121
- uses: actions/checkout@v4
2222
- run: ./test.sh
23+
24+
safety_tests:
25+
name: safety
26+
runs-on: ${{ ((github.repository == 'commaai/opendbc') && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.full_name == 'commaai/opendbc'))) && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}
27+
strategy:
28+
fail-fast: false
29+
matrix:
30+
flags: ['', '--ubsan']
31+
steps:
32+
- uses: commaai/timeout@v1
33+
- uses: actions/checkout@v4
34+
- name: Run safety tests
35+
run: ./opendbc/safety/tests/test.sh ${{ matrix.flags }}
36+
37+
misra_linter:
38+
name: MISRA C:2012 Linter
39+
runs-on: ${{ ((github.repository == 'commaai/opendbc') && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.full_name == 'commaai/opendbc'))) && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}
40+
timeout-minutes: 20
41+
steps:
42+
- name: Set up
43+
run: sudo apt-get install -y --no-install-recommends gcc-arm-none-eabi libnewlib-arm-none-eabi
44+
- uses: actions/checkout@v4
45+
- name: Restore cached cppcheck
46+
uses: actions/cache@v4
47+
with:
48+
path: opendbc/safety/tests/misra/cppcheck/
49+
key: cppcheck-cache-${{ runner.os }}-${{ github.ref }}
50+
restore-keys: |
51+
cppcheck-cache-${{ runner.os }}-${{ github.ref }}
52+
cppcheck-cache-${{ runner.os }}-
53+
- name: Run MISRA C:2012 analysis
54+
timeout-minutes: ${{ ((steps.restore-scons-cache.outputs.cache-hit == 'true') && 1 || 2) }}
55+
run: cd opendbc/safety/tests/misra && ./test_misra.sh
56+
- name: Save cppcheck cache
57+
uses: actions/cache@v4
58+
with:
59+
path: opendbc/safety/tests/misra/cppcheck/
60+
key: cppcheck-cache-${{ runner.os }}-${{ github.ref }}
61+
62+
misra_mutation:
63+
name: MISRA C:2012 Mutation
64+
runs-on: ${{ ((github.repository == 'commaai/opendbc') && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.full_name == 'commaai/opendbc'))) && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}
65+
timeout-minutes: 20
66+
steps:
67+
- name: Set up
68+
run: sudo apt-get install -y --no-install-recommends gcc-arm-none-eabi libnewlib-arm-none-eabi
69+
- uses: actions/checkout@v4
70+
- name: Restore cached cppcheck
71+
uses: actions/cache@v4
72+
with:
73+
path: opendbc/safety/tests/misra/cppcheck/
74+
key: cppcheck-cache-${{ runner.os }}-${{ github.ref }}
75+
restore-keys: |
76+
cppcheck-cache-${{ runner.os }}-${{ github.ref }}
77+
cppcheck-cache-${{ runner.os }}-
78+
- name: MISRA mutation tests
79+
timeout-minutes: 1
80+
run: |
81+
source setup.sh
82+
scons -j8
83+
cd opendbc/safety/tests/misra
84+
./install.sh # cppcheck
85+
pytest -s -n8 --randomly-seed $RANDOM test_mutation.py
86+
- name: Save cppcheck cache
87+
uses: actions/cache@v4
88+
with:
89+
path: opendbc/safety/tests/misra/cppcheck/
90+
key: cppcheck-cache-${{ runner.os }}-${{ github.ref }}
91+
92+
# mutation:
93+
# name: Safety mutation tests
94+
# runs-on: ${{ ((github.repository == 'commaai/opendbc') && ((github.event_name != 'pull_request') || (github.event.pull_request.head.repo.full_name == 'commaai/opendbc'))) && 'namespace-profile-amd64-8x16' || 'ubuntu-latest' }}
95+
# timeout-minutes: 20
96+
# steps:
97+
# - uses: actions/checkout@v4
98+
# with:
99+
# fetch-depth: 0 # need master to get diff
100+
# - name: Run mutation tests
101+
# timeout-minutes: 5
102+
# run: |
103+
# source setup.sh
104+
# scons -j8
105+
# GIT_REF=${{ github.event_name == 'push' && github.ref == 'refs/heads/master' && github.event.before || 'origin/master' }} cd opendbc/safety/tests && ./mutation.sh

‎README.md

+29-6
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ At its most basic, a car port will control the steering on a car. A "complete" c
6969
The first step is to get connected to the car with a comma 3X and a car harness.
7070
The car harness gets you connected to two different CAN buses and splits one of those buses to send our own actuation messages.
7171

72-
If you're lucky, a harness compatible with your car will already be designed and sold on comma.ai/shop.
72+
If you're lucky, a harness compatible with your car will already be designed and sold on comma.ai/shop.
7373
If you're not so lucky, start with a "developer harness" from comma.ai/shop and crimp on whatever connector you need.
7474

7575
### Structure of a port
@@ -78,11 +78,11 @@ Depending on , most of this basic structure will already be in place.
7878

7979
The entirery of a car port lives in `opendbc/car/<brand>/`:
8080
* `carstate.py`: parses out the relevant information from the CAN stream using the car's DBC file
81-
* `carcontroller.py`: outputs CAN messages to control the car
81+
* `carcontroller.py`: outputs CAN messages to control the car
8282
* `<brand>can.py`: thin Python helpers around the DBC file to build CAN messages
8383
* `fingerprints.py`: database of ECU firmware versions for identifying car models
8484
* `interface.py`: high level class for interfacing with the car
85-
* `radar_interface.py`: parses out the radar
85+
* `radar_interface.py`: parses out the radar
8686
* `values.py`: enumerates the brand's supported cars
8787

8888
### Reverse Engineer CAN messages
@@ -97,7 +97,7 @@ Use the [longitudinal maneuvers](https://github.com/commaai/openpilot/tree/maste
9797

9898
## Contributing
9999

100-
All opendbc development is coordinated on GitHub and [Discord](https://discord.comma.ai). Check out the `#dev-opendbc-cars` channel and `Vehicle Specific` section.
100+
All opendbc development is coordinated on GitHub and [Discord](https://discord.comma.ai). Check out the `#dev-opendbc-cars` channel and `Vehicle Specific` section.
101101

102102
### Roadmap
103103

@@ -116,6 +116,29 @@ Longer term
116116

117117
Contributions towards anything here are welcome.
118118

119+
## Safety Model
120+
121+
When a [panda](https://comma.ai/shop/panda) powers up with [opendbc safety firmware](opendbc/safety), by default it's in `SAFETY_SILENT` mode. While in `SAFETY_SILENT` mode, the CAN buses are forced to be silent. In order to send messages, you have to select a safety mode. Some of safety modes (for example `SAFETY_ALLOUTPUT`) are disabled in release firmwares. In order to use them, compile and flash your own build.
122+
123+
Safety modes optionally support `controls_allowed`, which allows or blocks a subset of messages based on a customizable state in the board.
124+
125+
## Code Rigor
126+
127+
The opendbc safety firmware is written for its use in conjunction with [openpilot](https://github.com/commaai/openpilot) and [panda](https://github.com/commaai/panda). The safety firmware, through its safety model, provides and enforces the
128+
[openpilot safety](https://github.com/commaai/openpilot/blob/master/docs/SAFETY.md). Due to its critical function, it's important that the application code rigor within the `safety` folder is held to high standards.
129+
130+
These are the [CI regression tests](https://github.com/commaai/opendbc/actions) we have in place:
131+
* A generic static code analysis is performed by [cppcheck](https://github.com/danmar/cppcheck/).
132+
* In addition, [cppcheck](https://github.com/danmar/cppcheck/) has a specific addon to check for [MISRA C:2012](https://misra.org.uk/) violations. See [current coverage](opendbc/safety/tests/misra/coverage_table).
133+
* Compiler options are relatively strict: the flags `-Wall -Wextra -Wstrict-prototypes -Werror` are enforced.
134+
* The [safety logic](opendbc/safety) is tested and verified by [unit tests](opendbc/safety/tests) for each supported car variant.
135+
136+
The above tests are themselves tested by:
137+
* a [mutation test](opendbc/safety/tests/misra/test_mutation.py) on the MISRA coverage
138+
* 100% line coverage enforced on the safety unit tests
139+
140+
In addition, we run the [ruff linter](https://github.com/astral-sh/ruff) and [mypy](https://mypy-lang.org/) on the car interface library.
141+
119142
### Bounties
120143

121144
Every car port is eligible for a bounty:
@@ -137,7 +160,7 @@ In addition to the standard bounties, we also offer higher value bounties for mo
137160

138161
***How does this work?*** In short, we designed hardware to replace your car's built-in lane keep and adaptive cruise features. See [this talk](https://www.youtube.com/watch?v=FL8CxUSfipM) for an in-depth explanation.
139162

140-
***Is there a timeline or roadmap for adding car support?*** No, most car support comes from the community, with comma doing final safety and quality validation. The more complete the community car port is and the more popular the car is, the more likely we are to pick it up as the next one to validate.
163+
***Is there a timeline or roadmap for adding car support?*** No, most car support comes from the community, with comma doing final safety and quality validation. The more complete the community car port is and the more popular the car is, the more likely we are to pick it up as the next one to validate.
141164

142165
### Terms
143166

@@ -163,7 +186,7 @@ In addition to the standard bounties, we also offer higher value bounties for mo
163186
* [*How to Port a Car*](https://www.youtube.com/watch?v=XxPS5TpTUnI&t=142s&pp=ygUPamFzb24gY29tbWEgY29u) by [@jyoung8607](https://github.com/jyoung8607) from COMMA_CON 2023
164187
* [commaCarSegments](https://huggingface.co/datasets/commaai/commaCarSegments): a massive dataset of CAN data from 300 different car models
165188
* [cabana](https://github.com/commaai/openpilot/tree/master/tools/cabana#readme): our tool for reverse engineering CAN messages
166-
* [can_print_changes.py](https://github.com/commaai/openpilot/blob/master/selfdrive/debug/can_print_changes.py): diff the whole CAN bus across two drives, such as one without any LKAS and one with LKAS
189+
* [can_print_changes.py](https://github.com/commaai/openpilot/blob/master/selfdrive/debug/can_print_changes.py): diff the whole CAN bus across two drives, such as one without any LKAS and one with LKAS
167190
* [longitudinal maneuvers](https://github.com/commaai/openpilot/tree/master/tools/longitudinal_maneuvers): a tool for evaluating and tuning longitudinal control
168191
* [opendbc data](https://commaai.github.io/opendbc-data/): a repository of longitudinal maneuver evaluations
169192

‎SConscript

+4
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,7 @@ Import("env")
22

33
SConscript(['opendbc/can/SConscript'], exports={'env': env})
44
SConscript(['opendbc/dbc/SConscript'], exports={'env': env})
5+
6+
# test files
7+
if GetOption('extras'):
8+
SConscript('opendbc/safety/tests/libsafety/SConscript')

‎SConstruct

+13
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,19 @@ AddOption('--asan',
2525
action='store_true',
2626
help='turn on ASAN')
2727

28+
# safety options
29+
AddOption('--ubsan',
30+
action='store_true',
31+
help='turn on UBSan')
32+
33+
AddOption('--coverage',
34+
action='store_true',
35+
help='build with test coverage options')
36+
37+
AddOption('--mutation',
38+
action='store_true',
39+
help='generate mutation-ready code')
40+
2841
ccflags_asan = ["-fsanitize=address", "-fno-omit-frame-pointer"] if GetOption('asan') else []
2942
ldflags_asan = ["-fsanitize=address"] if GetOption('asan') else []
3043

‎opendbc/safety/__init__.py

+5
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
# constants from panda/python/__init__.py
2+
DLC_TO_LEN = [0, 1, 2, 3, 4, 5, 6, 7, 8, 12, 16, 20, 24, 32, 48, 64]
3+
LEN_TO_DLC = {length: dlc for (dlc, length) in enumerate(DLC_TO_LEN)}
4+
5+
16
class Safety:
27
# matches opendbc.structs.CarParams.SafetyModel
38
# TODO: just use SafetyModel in panda repo

‎opendbc/safety/board/can.h

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#pragma once
2+
#include "can_declarations.h"
3+
4+
static const unsigned char dlc_to_len[] = {0U, 1U, 2U, 3U, 4U, 5U, 6U, 7U, 8U, 12U, 16U, 20U, 24U, 32U, 48U, 64U};

‎opendbc/safety/board/can_declarations.h

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#pragma once
2+
3+
#define CANPACKET_HEAD_SIZE 6U
4+
5+
// TODO: this is always CANFD
6+
#if !defined(STM32F4)
7+
#define CANFD
8+
#define CANPACKET_DATA_SIZE_MAX 64U
9+
#else
10+
#define CANPACKET_DATA_SIZE_MAX 8U
11+
#endif
12+
13+
typedef struct {
14+
unsigned char fd : 1;
15+
unsigned char bus : 3;
16+
unsigned char data_len_code : 4; // lookup length with dlc_to_len
17+
unsigned char rejected : 1;
18+
unsigned char returned : 1;
19+
unsigned char extended : 1;
20+
unsigned int addr : 29;
21+
unsigned char checksum;
22+
unsigned char data[CANPACKET_DATA_SIZE_MAX];
23+
} __attribute__((packed, aligned(4))) CANPacket_t;
24+
25+
#define GET_BUS(msg) ((msg)->bus)
26+
#define GET_LEN(msg) (dlc_to_len[(msg)->data_len_code])
27+
#define GET_ADDR(msg) ((msg)->addr)

‎opendbc/safety/board/drivers/can_common.h

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#include "can_common_declarations.h"
2+
3+
uint8_t calculate_checksum(const uint8_t *dat, uint32_t len) {
4+
uint8_t checksum = 0U;
5+
for (uint32_t i = 0U; i < len; i++) {
6+
checksum ^= dat[i];
7+
}
8+
return checksum;
9+
}
10+
11+
void can_set_checksum(CANPacket_t *packet) {
12+
packet->checksum = 0U;
13+
packet->checksum = calculate_checksum((uint8_t *) packet, CANPACKET_HEAD_SIZE + GET_LEN(packet));
14+
}
15+
16+
bool can_check_checksum(CANPacket_t *packet) {
17+
return (calculate_checksum((uint8_t *) packet, CANPACKET_HEAD_SIZE + GET_LEN(packet)) == 0U);
18+
}

‎opendbc/safety/board/drivers/can_common_declarations.h

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#pragma once
2+
3+
uint8_t calculate_checksum(const uint8_t *dat, uint32_t len);
4+
void can_set_checksum(CANPacket_t *packet);

‎opendbc/safety/board/fake_stm.h

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// minimal code to fake a panda for tests
2+
#include <stdio.h>
3+
#include <stdint.h>
4+
#include <stdlib.h>
5+
6+
#include "utils.h"
7+
8+
#define ALLOW_DEBUG
9+
#define PANDA
10+
11+
void print(const char *a) {
12+
printf("%s", a);
13+
}
14+
15+
void puth(unsigned int i) {
16+
printf("%u", i);
17+
}
18+
19+
typedef struct {
20+
uint32_t CNT;
21+
} TIM_TypeDef;
22+
23+
TIM_TypeDef timer;
24+
TIM_TypeDef *MICROSECOND_TIMER = &timer;
25+
uint32_t microsecond_timer_get(void);
26+
27+
uint32_t microsecond_timer_get(void) {
28+
return MICROSECOND_TIMER->CNT;
29+
}

‎opendbc/safety/board/faults.h

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#include "faults_declarations.h"
2+
3+
uint8_t fault_status = FAULT_STATUS_NONE;
4+
uint32_t faults = 0U;
5+
6+
void fault_occurred(uint32_t fault) {
7+
if ((faults & fault) == 0U) {
8+
if ((PERMANENT_FAULTS & fault) != 0U) {
9+
print("Permanent fault occurred: 0x"); puth(fault); print("\n");
10+
fault_status = FAULT_STATUS_PERMANENT;
11+
} else {
12+
print("Temporary fault occurred: 0x"); puth(fault); print("\n");
13+
fault_status = FAULT_STATUS_TEMPORARY;
14+
}
15+
}
16+
faults |= fault;
17+
}
18+
19+
void fault_recovered(uint32_t fault) {
20+
if ((PERMANENT_FAULTS & fault) == 0U) {
21+
faults &= ~fault;
22+
} else {
23+
print("Cannot recover from a permanent fault!\n");
24+
}
25+
}

‎opendbc/safety/board/faults_declarations.h

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#pragma once
2+
3+
#define FAULT_STATUS_NONE 0U
4+
#define FAULT_STATUS_TEMPORARY 1U
5+
#define FAULT_STATUS_PERMANENT 2U
6+
7+
// Fault types, excerpt from cereal.log.PandaState.FaultType for safety tests
8+
#define FAULT_RELAY_MALFUNCTION (1UL << 0)
9+
// ...
10+
11+
// Permanent faults
12+
#define PERMANENT_FAULTS 0U
13+
14+
extern uint8_t fault_status;
15+
extern uint32_t faults;
16+
17+
void fault_occurred(uint32_t fault);
18+
void fault_recovered(uint32_t fault);

‎opendbc/safety/board/utils.h

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// cppcheck-suppress-macro misra-c2012-1.2; allow __typeof__ extension
2+
#define MIN(a, b) ({ \
3+
__typeof__ (a) _a = (a); \
4+
__typeof__ (b) _b = (b); \
5+
(_a < _b) ? _a : _b; \
6+
})
7+
8+
// cppcheck-suppress-macro misra-c2012-1.2; allow __typeof__ extension
9+
#define MAX(a, b) ({ \
10+
__typeof__ (a) _a = (a); \
11+
__typeof__ (b) _b = (b); \
12+
(_a > _b) ? _a : _b; \
13+
})
14+
15+
// cppcheck-suppress-macro misra-c2012-1.2; allow __typeof__ extension
16+
#define CLAMP(x, low, high) ({ \
17+
__typeof__(x) __x = (x); \
18+
__typeof__(low) __low = (low);\
19+
__typeof__(high) __high = (high);\
20+
(__x > __high) ? __high : ((__x < __low) ? __low : __x); \
21+
})
22+
23+
// cppcheck-suppress-macro misra-c2012-1.2; allow __typeof__ extension
24+
#define ABS(a) ({ \
25+
__typeof__ (a) _a = (a); \
26+
(_a > 0) ? _a : (-_a); \
27+
})
28+
29+
#ifndef NULL
30+
// this just provides a standard implementation of NULL
31+
// in lieu of including libc in the panda build
32+
// cppcheck-suppress [misra-c2012-21.1]
33+
#define NULL ((void*)0)
34+
#endif
35+
36+
// STM32 HAL defines this
37+
#ifndef UNUSED
38+
#define UNUSED(x) ((void)(x))
39+
#endif
40+
41+
#define COMPILE_TIME_ASSERT(pred) ((void)sizeof(char[1 - (2 * (!(pred) ? 1 : 0))]))
42+
43+
// compute the time elapsed (in microseconds) from 2 counter samples
44+
// case where ts < ts_last is ok: overflow is properly re-casted into uint32_t
45+
uint32_t get_ts_elapsed(uint32_t ts, uint32_t ts_last) {
46+
return ts - ts_last;
47+
}

‎opendbc/safety/main.c

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#include "safety.h"
2+
3+
// this file is checked by cppcheck
4+
5+
// Ignore misra-c2012-8.7 as these functions are only called from panda and libsafety
6+
UNUSED(heartbeat_engaged);
7+
8+
UNUSED(safety_rx_hook);
9+
UNUSED(safety_tx_hook);
10+
UNUSED(safety_fwd_hook);
11+
UNUSED(safety_tick);
12+
UNUSED(set_safety_hooks);

0 commit comments

Comments
 (0)