openpilot
openpilot copied to clipboard
commaai
Hyundai: common CAN-FD brake pressed signal
Prep for supporting ICE CAN-FD cars in #25434, seems similar to TCS13 / DriverBraking from the reference DBC for the CAN Hyundai platform
Matched the old signal exactly (visually) on the couple EV6 routes I checked. @sshane can you check this with your notebook on all the supported cars and a few ICE routes from that PR?
Looks like a good change:
Platform: KIA EV6 2022
Total: 6 unique dongles, 150 unique routes
Checked: 6 unique dongles, 85 unique routes, and 150 segments
Mismatches: 310 frames total, 39 segments (any frame)
Mismatches > 10: 28 frames total, 2 segments (any frame)
Segments with brake pressed: 46 segments (above 2 seconds of brake)
Segments with mismatches > 10: {'b186d6fbc83b99a8|2022-09-27--12-54-20--0', 'b186d6fbc83b99a8|2022-09-23--10-29-51--5'}
---
Platform: HYUNDAI TUCSON HYBRID 4TH GEN
Total: 1 unique dongles, 8 unique routes
Checked: 1 unique dongles, 1 unique routes, and 8 segments
Mismatches: 6 frames total, 2 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 3 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
---
Platform: HYUNDAI IONIQ 5 2022
Total: 4 unique dongles, 164 unique routes
Checked: 4 unique dongles, 83 unique routes, and 162 segments
Mismatches: 106 frames total, 28 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 42 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
I sampled the mismatches on update of the TCS message (since it's lower frequency), however the two segments with high mismatches had a lot of brake pressed events, so this is expected: https://imgur.com/a/DkCe9dP
Just to show what a comparison of two close, but not exactly the same signals looks like: https://imgur.com/a/pb60jr7
Platform: KIA EV6 2022
Total: 6 unique dongles, 150 unique routes
Checked: 6 unique dongles, 85 unique routes, and 150 segments
Mismatches: 16036 frames total, 53 segments (any frame)
Mismatches > 10: 7975 frames total, 44 segments (any frame)
Segments with brake pressed: 49 segments (above 2 seconds of brake)
Segments with mismatches > 10: {lots of segments}
---
Platform: HYUNDAI TUCSON HYBRID 4TH GEN
Total: 1 unique dongles, 8 unique routes
Checked: 1 unique dongles, 1 unique routes, and 8 segments
Mismatches: 0 frames total, 0 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 3 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
---
Platform: HYUNDAI IONIQ 5 2022
Total: 4 unique dongles, 164 unique routes
Checked: 4 unique dongles, 83 unique routes, and 162 segments
Mismatches: 6202 frames total, 46 segments (any frame)
Mismatches > 10: 3056 frames total, 37 segments (any frame)
Segments with brake pressed: 43 segments (above 2 seconds of brake)
Segments with mismatches > 10: {lots of segments}
This also looks good on the ICE cars, except the GV70 which doesn't set the bit:
@adeebshihadeh here are the most similar bits to the ALT signal sunny added on the GV70:
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=6, bit_idx=5
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=6, bit_idx=6
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=6, bit_idx=7
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=0
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=1
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=2
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=3
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=4
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=5
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=6
hex(msg)='0x4b9', bit_mismatches=25 of 210 (11.9%), byt_idx=7, bit_idx=7
hex(msg)='0x412', bit_mismatches=17 of 900 (1.89%), byt_idx=6, bit_idx=5
hex(msg)='0x418', bit_mismatches=17 of 900 (1.89%), byt_idx=5, bit_idx=1
hex(msg)='0x100', bit_mismatches=260 of 16150 (1.61%), byt_idx=4, bit_idx=7
hex(msg)='0x175', bit_mismatches=123 of 8074 (1.52%), byt_idx=10, bit_idx=6
Searched bus: 4
TCS->BRAKE_RELATED is nearly an exact match, but on the non-ICE cars, it's quite a bit different as you see above (https://imgur.com/a/pb60jr7). We'll have to verify that it's good to use for EVs, or just keep different signals for ICE and EV
Looks like BRAKE_RELATED on the ICE cars is good to use: https://imgur.com/a/HpnaOy2
The brake pressure signal in ESP_STATUS is oddly more noisy at the bottom end than our EV6, where it goes to 1 on occasion without BRAKE_RELATED rising (whereas on our EV6, if the pressure signal went to 1, BRAKE_RELATED would definitely rise). Each car must do their own filtering on the brake pressure signal in ESP_STATUS, which is good for us, as we can just use BRAKE_RELATED (which cancels cruise on the EV6).
This is the Kia Sportage cancelling on BRAKE_RELATED/DriverBraking (again, its brake pressure threshold is well above 1 vs. our EV6 which is 1): https://user-images.githubusercontent.com/25857203/197062825-a41cf508-71fc-4ae6-9719-dcd8a0746281.png
GV70 canceling on this bit too (shows ESP_STATUS->BRAKE_PRESSED is not to be trusted): https://user-images.githubusercontent.com/25857203/197063285-24f0e805-89da-4232-a213-072b7898d247.png
Santa Cruz also looks good: https://user-images.githubusercontent.com/25857203/197063593-393fdc35-2c95-436f-9003-30a21a4d2efb.png
Tucson Hybrid doesn't follow same convention. BRAKE_RELATED seems to depend on BRAKE->BRAKE_POSITION -- which is also now signed, rather than ESP_STATUS->BRAKE_PRESSURE, however the signal is identical to the current brake pressed signal: https://user-images.githubusercontent.com/25857203/197070203-be7297e7-30ed-473d-b3cd-0ab20d2c1123.png
EV6 and Ioniq 5 are the same in terms of behavior of BRAKE_PRESSURE, BRAKE_PRESSED, and the brake bits. Ioniq 5 BRAKE_RELATED rising as soon as BRAKE_PRESSURE is != 0 like EV6: https://user-images.githubusercontent.com/25857203/197075663-6d4adc3c-acaf-4a3b-a26e-db1b3bfffae1.png
Checked to make sure that TCS->BRAKE_RELATED always at least captures what the previous signal did (BRAKE->BRAKE_PRESSED:
Platform: HYUNDAI IONIQ 5 2022
Total: 4 unique dongles, 164 unique routes
Checked: 3 unique dongles, 40 unique routes, and 47 segments
Mismatches: 0 frames total, 0 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 42 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
---
Platform: KIA EV6 2022
Total: 6 unique dongles, 150 unique routes
Checked: 4 unique dongles, 35 unique routes, and 54 segments
Mismatches: 0 frames total, 0 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 46 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
---
Platform: HYUNDAI TUCSON HYBRID 4TH GEN
Total: 1 unique dongles, 8 unique routes
Checked: 1 unique dongles, 1 unique routes, and 3 segments
Mismatches: 2 frames total, 1 segments (any frame)
Mismatches > 10: 0 frames total, 0 segments (any frame)
Segments with brake pressed: 3 segments (above 2 seconds of brake)
Segments with mismatches > 10: set()
Final plots with high mismatches.
I was wrong about the BRAKE_PRESSURE != 0 condition (it's actually BRAKE_POSITION, but it often lines up with BRAKE_PRESSURE going non-zero), but nevertheless, this TCS->BRAKE_RELATED signal is just a more sensitive BRAKE->BRAKE_PRESSED for the EVs, and it's what the stock ACC uses to cancel.
EV6: https://imgur.com/a/ZHZGBXM IONIQ 5: https://imgur.com/a/cJdoKeB Tucscon Hybrid (no mismatches, behaves similarly to ICE cars): https://imgur.com/a/xEcIkRq
Sampled plots from these mismatches: https://pastebin.com/RWpkyXXn