Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

safety: move RX addr check into top level hook #1727

Merged
merged 8 commits into from
Nov 20, 2023
Merged

safety: move RX addr check into top level hook #1727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0e44af3
safety: move RX addr check into top level hook
adeebshihadeh Nov 20, 2023
b9e851d
tesla
adeebshihadeh Nov 20, 2023
babb4a3
rest
adeebshihadeh Nov 20, 2023
e6b68e3
can't forget about the body
adeebshihadeh Nov 20, 2023
6b4174e
rm more
adeebshihadeh Nov 20, 2023
e3859a9
fix that
adeebshihadeh Nov 20, 2023
16b1059
cleanup
adeebshihadeh Nov 20, 2023
1e6e381
drop the fn
adeebshihadeh Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions board/safety.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,20 @@ const addr_checks *current_rx_checks = &default_rx_checks;

bool safety_rx_hook(CANPacket_t *to_push) {
bool controls_allowed_prev = controls_allowed;
int ret = current_hooks->rx(to_push);

bool valid = addr_safety_check(to_push, current_rx_checks, current_hooks->get_checksum,
Copy link
Contributor

@sshane sshane Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still returns true if the address isn't in the whitelisted struct btw, so Honda bug isn't caught

But this makes it much easier to write that, so I'll take it up in a new PR.

current_hooks->compute_checksum, current_hooks->get_counter,
current_hooks->get_quality_flag_valid);
if (valid) {
current_hooks->rx(to_push);
}

// reset mismatches on rising edge of controls_allowed to avoid rare race condition
if (controls_allowed && !controls_allowed_prev) {
heartbeat_engaged_mismatches = 0;
}

return ret;
return valid;
}

bool safety_tx_hook(CANPacket_t *to_send) {
Expand Down Expand Up @@ -216,10 +222,10 @@ void update_addr_timestamp(AddrCheckStruct addr_list[], int index) {

bool addr_safety_check(CANPacket_t *to_push,
const addr_checks *rx_checks,
uint32_t (*get_checksum)(CANPacket_t *to_push),
uint32_t (*compute_checksum)(CANPacket_t *to_push),
uint8_t (*get_counter)(CANPacket_t *to_push),
bool (*get_quality_flag_valid)(CANPacket_t *to_push)) {
const get_checksum_t get_checksum,
const compute_checksum_t compute_checksum,
const get_counter_t get_counter,
const get_quality_flag_valid_t get_quality_flag_valid) {

int index = get_addr_check_index(to_push, rx_checks->check, rx_checks->len);
update_addr_timestamp(rx_checks->check, index);
Expand Down
8 changes: 2 additions & 6 deletions board/safety/safety_body.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,14 @@ AddrCheckStruct body_addr_checks[] = {
};
addr_checks body_rx_checks = SET_ADDR_CHECKS(body_addr_checks);

static bool body_rx_hook(CANPacket_t *to_push) {

bool valid = addr_safety_check(to_push, &body_rx_checks, NULL, NULL, NULL, NULL);

static void body_rx_hook(CANPacket_t *to_push) {
// body is never at standstill
vehicle_moving = true;

if (valid && (GET_ADDR(to_push) == 0x201U)) {
if (GET_ADDR(to_push) == 0x201U) {
controls_allowed = true;
}

return valid;
}

static bool body_tx_hook(CANPacket_t *to_send) {
Expand Down
74 changes: 34 additions & 40 deletions board/safety/safety_chrysler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,54 +173,45 @@ static uint8_t chrysler_get_counter(CANPacket_t *to_push) {
return (uint8_t)(GET_BYTE(to_push, 6) >> 4);
}

static bool chrysler_rx_hook(CANPacket_t *to_push) {

bool valid = addr_safety_check(to_push, &chrysler_rx_checks,
chrysler_get_checksum, chrysler_compute_checksum,
chrysler_get_counter, NULL);

static void chrysler_rx_hook(CANPacket_t *to_push) {
const int bus = GET_BUS(to_push);
const int addr = GET_ADDR(to_push);

if (valid) {

// Measured EPS torque
if ((bus == 0) && (addr == chrysler_addrs->EPS_2)) {
int torque_meas_new = ((GET_BYTE(to_push, 4) & 0x7U) << 8) + GET_BYTE(to_push, 5) - 1024U;
update_sample(&torque_meas, torque_meas_new);
}

// enter controls on rising edge of ACC, exit controls on ACC off
const int das_3_bus = (chrysler_platform == CHRYSLER_PACIFICA) ? 0 : 2;
if ((bus == das_3_bus) && (addr == chrysler_addrs->DAS_3)) {
bool cruise_engaged = GET_BIT(to_push, 21U) == 1U;
pcm_cruise_check(cruise_engaged);
}
// Measured EPS torque
if ((bus == 0) && (addr == chrysler_addrs->EPS_2)) {
int torque_meas_new = ((GET_BYTE(to_push, 4) & 0x7U) << 8) + GET_BYTE(to_push, 5) - 1024U;
update_sample(&torque_meas, torque_meas_new);
}

// TODO: use the same message for both
// update vehicle moving
if ((chrysler_platform != CHRYSLER_PACIFICA) && (bus == 0) && (addr == chrysler_addrs->ESP_8)) {
vehicle_moving = ((GET_BYTE(to_push, 4) << 8) + GET_BYTE(to_push, 5)) != 0U;
}
if ((chrysler_platform == CHRYSLER_PACIFICA) && (bus == 0) && (addr == 514)) {
int speed_l = (GET_BYTE(to_push, 0) << 4) + (GET_BYTE(to_push, 1) >> 4);
int speed_r = (GET_BYTE(to_push, 2) << 4) + (GET_BYTE(to_push, 3) >> 4);
vehicle_moving = (speed_l != 0) || (speed_r != 0);
}
// enter controls on rising edge of ACC, exit controls on ACC off
const int das_3_bus = (chrysler_platform == CHRYSLER_PACIFICA) ? 0 : 2;
if ((bus == das_3_bus) && (addr == chrysler_addrs->DAS_3)) {
bool cruise_engaged = GET_BIT(to_push, 21U) == 1U;
pcm_cruise_check(cruise_engaged);
}

// exit controls on rising edge of gas press
if ((bus == 0) && (addr == chrysler_addrs->ECM_5)) {
gas_pressed = GET_BYTE(to_push, 0U) != 0U;
}
// TODO: use the same message for both
// update vehicle moving
if ((chrysler_platform != CHRYSLER_PACIFICA) && (bus == 0) && (addr == chrysler_addrs->ESP_8)) {
vehicle_moving = ((GET_BYTE(to_push, 4) << 8) + GET_BYTE(to_push, 5)) != 0U;
}
if ((chrysler_platform == CHRYSLER_PACIFICA) && (bus == 0) && (addr == 514)) {
int speed_l = (GET_BYTE(to_push, 0) << 4) + (GET_BYTE(to_push, 1) >> 4);
int speed_r = (GET_BYTE(to_push, 2) << 4) + (GET_BYTE(to_push, 3) >> 4);
vehicle_moving = (speed_l != 0) || (speed_r != 0);
}

// exit controls on rising edge of brake press
if ((bus == 0) && (addr == chrysler_addrs->ESP_1)) {
brake_pressed = ((GET_BYTE(to_push, 0U) & 0xFU) >> 2U) == 1U;
}
// exit controls on rising edge of gas press
if ((bus == 0) && (addr == chrysler_addrs->ECM_5)) {
gas_pressed = GET_BYTE(to_push, 0U) != 0U;
}

generic_rx_checks((bus == 0) && (addr == chrysler_addrs->LKAS_COMMAND));
// exit controls on rising edge of brake press
if ((bus == 0) && (addr == chrysler_addrs->ESP_1)) {
brake_pressed = ((GET_BYTE(to_push, 0U) & 0xFU) >> 2U) == 1U;
}
return valid;

generic_rx_checks((bus == 0) && (addr == chrysler_addrs->LKAS_COMMAND));
}

static bool chrysler_tx_hook(CANPacket_t *to_send) {
Expand Down Expand Up @@ -307,4 +298,7 @@ const safety_hooks chrysler_hooks = {
.tx = chrysler_tx_hook,
.tx_lin = nooutput_tx_lin_hook,
.fwd = chrysler_fwd_hook,
.get_counter = chrysler_get_counter,
.get_checksum = chrysler_get_checksum,
.compute_checksum = chrysler_compute_checksum,
};
3 changes: 1 addition & 2 deletions board/safety/safety_defaults.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@ const addr_checks default_rx_checks = {
.len = 0,
};

bool default_rx_hook(CANPacket_t *to_push) {
void default_rx_hook(CANPacket_t *to_push) {
UNUSED(to_push);
return true;
}

// *** no output safety mode ***
Expand Down
12 changes: 6 additions & 6 deletions board/safety/safety_ford.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,8 @@ const SteeringLimits FORD_STEERING_LIMITS = {
.inactive_angle_is_zero = true,
};

static bool ford_rx_hook(CANPacket_t *to_push) {
bool valid = addr_safety_check(to_push, &ford_rx_checks,
ford_get_checksum, ford_compute_checksum, ford_get_counter, ford_get_quality_flag_valid);

if (valid && (GET_BUS(to_push) == FORD_MAIN_BUS)) {
static void ford_rx_hook(CANPacket_t *to_push) {
if (GET_BUS(to_push) == FORD_MAIN_BUS) {
int addr = GET_ADDR(to_push);

// Update in motion state from standstill signal
Expand Down Expand Up @@ -271,7 +268,6 @@ static bool ford_rx_hook(CANPacket_t *to_push) {
generic_rx_checks(stock_ecu_detected);
}

return valid;
}

static bool ford_tx_hook(CANPacket_t *to_send) {
Expand Down Expand Up @@ -432,4 +428,8 @@ const safety_hooks ford_hooks = {
.tx = ford_tx_hook,
.tx_lin = nooutput_tx_lin_hook,
.fwd = ford_fwd_hook,
.get_counter = ford_get_counter,
.get_checksum = ford_get_checksum,
.compute_checksum = ford_compute_checksum,
.get_quality_flag_valid = ford_get_quality_flag_valid,
};
8 changes: 2 additions & 6 deletions board/safety/safety_gm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,8 @@ enum {GM_ASCM, GM_CAM} gm_hw = GM_ASCM;
bool gm_cam_long = false;
bool gm_pcm_cruise = false;

static bool gm_rx_hook(CANPacket_t *to_push) {

bool valid = addr_safety_check(to_push, &gm_rx_checks, NULL, NULL, NULL, NULL);

if (valid && (GET_BUS(to_push) == 0U)) {
static void gm_rx_hook(CANPacket_t *to_push) {
if (GET_BUS(to_push) == 0U) {
int addr = GET_ADDR(to_push);

if (addr == 0x184) {
Expand Down Expand Up @@ -137,7 +134,6 @@ static bool gm_rx_hook(CANPacket_t *to_push) {
}
generic_rx_checks(stock_ecu_detected);
}
return valid;
}

// all commands: gas/regen, friction brake and steering
Expand Down
Loading
Loading