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

VW PQ: Message updates, checksum and counter support #633

Merged
merged 20 commits into from
Jul 15, 2022
Merged

VW PQ: Message updates, checksum and counter support #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
fd82dcb
refactor MQB checksum/counter to prep for PQ
jyoung8607 Jun 9, 2022
cb6d980
first pass at PQ checksums and counters
jyoung8607 Jun 9, 2022
0d30ee9
remove checksum and counter alignment checks
jyoung8607 Jun 9, 2022
bbb67a9
forgot HCA_1
jyoung8607 Jun 9, 2022
1701440
Revert "remove checksum and counter alignment checks"
jyoung8607 Jun 13, 2022
579f317
don't check counter offset
jyoung8607 Jun 13, 2022
c987ed0
tweak comment
jyoung8607 Jun 13, 2022
eef6620
handle checksum not starting at zero
jyoung8607 Jun 13, 2022
06f8c58
remove unused argument
jyoung8607 Jun 14, 2022
fac85f7
add LDW_Textbits to LDW_1
jyoung8607 Jun 9, 2022
838fa4e
Update can/common.cc
jyoung8607 Jun 19, 2022
944a024
Merge branch 'master' of https://github.com/commaai/opendbc into pq-c…
jyoung8607 Jul 2, 2022
02571d4
fix counter again; minimize diff
jyoung8607 Jul 2, 2022
7342679
additional door-open signals
jyoung8607 Jul 3, 2022
629169a
add trunk lid state
jyoung8607 Jul 3, 2022
a712a5b
update LDW_Status message and signals
jyoung8607 Jul 3, 2022
a78c458
typo
jyoung8607 Jul 3, 2022
6eecf6c
Merge branch 'master' into pq-checksum
jyoung8607 Jul 5, 2022
433ea88
Merge branch 'master' into pq-checksum
jyoung8607 Jul 11, 2022
2aae662
make XOR checksum generic
jyoung8607 Jul 11, 2022
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
14 changes: 14 additions & 0 deletions can/common.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,20 @@ unsigned int volkswagen_mqb_checksum(uint32_t address, const Signal &sig, const
return crc ^ 0xFF; // Return after standard final XOR for CRC8 8H2F/AUTOSAR
}

unsigned int xor_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d) {
uint8_t checksum = 0;
int checksum_byte = sig.start_bit / 8;

// Simple XOR over the payload, except for the byte where the checksum lives.
for (int i = 0; i < d.size(); i++) {
if (i != checksum_byte) {
checksum ^= d[i];
}
}

return checksum;
}

unsigned int pedal_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d) {
uint8_t crc = 0xFF;
uint8_t poly = 0xD5; // standard crc8
Expand Down
1 change: 1 addition & 0 deletions can/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ unsigned int toyota_checksum(uint32_t address, const Signal &sig, const std::vec
unsigned int subaru_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
unsigned int chrysler_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
unsigned int volkswagen_mqb_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
unsigned int xor_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
unsigned int hkg_can_fd_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);
unsigned int pedal_checksum(uint32_t address, const Signal &sig, const std::vector<uint8_t> &d);

Expand Down
1 change: 1 addition & 0 deletions can/common.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ cdef extern from "common_dbc.h":
TOYOTA_CHECKSUM,
PEDAL_CHECKSUM,
VOLKSWAGEN_MQB_CHECKSUM,
XOR_CHECKSUM,
SUBARU_CHECKSUM,
CHRYSLER_CHECKSUM
HKG_CAN_FD_CHECKSUM,
Expand Down
1 change: 1 addition & 0 deletions can/common_dbc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ enum SignalType {
TOYOTA_CHECKSUM,
PEDAL_CHECKSUM,
VOLKSWAGEN_MQB_CHECKSUM,
XOR_CHECKSUM,
SUBARU_CHECKSUM,
CHRYSLER_CHECKSUM,
HKG_CAN_FD_CHECKSUM,
Expand Down
2 changes: 2 additions & 0 deletions can/dbc.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ ChecksumState* get_checksum(const std::string& dbc_name) {
s = new ChecksumState({16, 8, 0, 0, true, HKG_CAN_FD_CHECKSUM, COUNTER, &hkg_can_fd_checksum});
} else if (startswith(dbc_name, {"vw_mqb_2010"})) {
s = new ChecksumState({8, 4, 0, 0, true, VOLKSWAGEN_MQB_CHECKSUM, COUNTER, &volkswagen_mqb_checksum});
} else if (startswith(dbc_name, {"vw_golf_mk4"})) {
s = new ChecksumState({8, 4, 0, -1, true, XOR_CHECKSUM, COUNTER, &xor_checksum});
} else if (startswith(dbc_name, "subaru_global_")) {
s = new ChecksumState({8, -1, 0, -1, true, SUBARU_CHECKSUM, DEFAULT, &subaru_checksum});
} else if (startswith(dbc_name, "chrysler_")) {
Expand Down
68 changes: 42 additions & 26 deletions vw_golf_mk4.dbc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ BO_ 1504 Klima_1: 8 XXX
SG_ Drehzahlanhebung : 0|1@1+ (1,0) [0|0] "" XXX

BO_ 906 GRA_Neu: 4 XXX
SG_ GRA_Checksum : 0|8@1+ (1,0) [0|255] "" XXX
SG_ CHECKSUM : 0|8@1+ (1,0) [0|255] "" XXX
SG_ GRA_Hauptschalt : 8|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Abbrechen : 9|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Down_kurz : 10|1@1+ (1,0) [0|1] "" XXX
Expand All @@ -628,7 +628,7 @@ BO_ 906 GRA_Neu: 4 XXX
SG_ GRA_Neu_Setzen : 16|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Recall : 17|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Sender : 18|2@1+ (1,0) [0|3] "" XXX
SG_ GRA_Neu_Zaehler : 20|4@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 20|4@1+ (1,0) [0|15] "" XXX
SG_ GRA_Tip_Down : 24|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Tip_Up : 25|1@1+ (1,0) [0|1] "" XXX
SG_ GRA_Zeitluecke : 26|2@1+ (1,0) [0|3] "" XXX
Expand Down Expand Up @@ -683,7 +683,7 @@ BO_ 1344 Getriebe_2: 8 XXX
BO_ 1088 Getriebe_1: 8 XXX
SG_ Wandlerverlustmoment : 56|8@1+ (0.39,0) [0|99.06] "MDI" XXX
SG_ Fehlerspeichereintrag__Getriebe : 55|1@1+ (1,0) [0|0] "" XXX
SG_ Zaehler_Getriebe_1 : 51|4@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 51|4@1+ (1,0) [0|15] "" XXX
SG_ Gang_eingelegt : 50|1@1+ (1,0) [0|0] "" XXX
SG_ Schaltabsicht : 49|1@1+ (1,0) [0|0] "" XXX
SG_ Motor_aus : 48|1@1+ (1,0) [0|0] "" XXX
Expand Down Expand Up @@ -722,6 +722,8 @@ BO_ 912 Gate_Komf_1: 8 XXX
SG_ GK1_ParkFrontWi : 22|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_KW_Warm : 23|1@1+ (1,0) [0|1] "" XXX
SG_ BCM_Remotestart_Betrieb : 24|1@1+ (1,0) [0|1] "" XXX
SG_ BSK_HL_geoeffnet : 26|1@1+ (1,0) [0|1] "" XXX
SG_ BSK_HR_geoeffnet : 27|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_Rueckfahr : 28|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_BrLi_links : 29|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_BrLi_rechts : 30|1@1+ (1,0) [0|1] "" XXX
Expand All @@ -735,6 +737,8 @@ BO_ 912 Gate_Komf_1: 8 XXX
SG_ GK1_Sta_Licht2 : 38|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_Sta_LSM : 39|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_Count_Anhaen : 40|4@1+ (1,0) [0|15] "" XXX
SG_ BSK_BT_geoeffnet : 41|1@1+ (1,0) [0|1] "" XXX
SG_ BSK_HD_Hauptraste : 43|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_BLS_AAG : 44|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_EDC_AAG : 45|1@1+ (1,0) [0|1] "" XXX
SG_ GK1_Anhaenger : 46|1@1+ (1,0) [0|1] "" XXX
Expand Down Expand Up @@ -857,8 +861,8 @@ BO_ 424 Bremse_6: 3 XXX
SG_ Bremsdruck__Bremse_6_ : 0|10@1+ (0.3255,-40) [-40|293] "bar" XXX

BO_ 1192 Bremse_5: 8 XXX
SG_ Checksumme_Bremse_5 : 56|8@1+ (1,0) [0|0] "" XXX
SG_ Zaehler_Bremse_5 : 52|4@1+ (1,0) [0|15] "" XXX
SG_ CHECKSUM : 56|8@1+ (1,0) [0|0] "" XXX
SG_ COUNTER : 52|4@1+ (1,0) [0|15] "" XXX
SG_ Bremslicht_ECD : 51|1@1+ (1,0) [0|0] "" XXX
SG_ Bremsentemperatur_vorn : 48|3@1+ (125,125) [125|1000] "C" XXX
SG_ Frei_Bremse_5_5 : 40|8@1+ (1,0) [0|0] "" XXX
Expand Down Expand Up @@ -909,7 +913,7 @@ BO_ 416 Bremse_1: 8 XXX
SG_ ESP_Systemstatus_4_1 : 62|1@1+ (1,0) [0|0] "" XXX
SG_ ESP_Passiv_getastet : 61|1@1+ (1,0) [0|0] "" XXX
SG_ ASR_Steuerger_t : 60|1@1+ (1,0) [0|0] "" XXX
SG_ Zaehler_Bremse_1 : 56|4@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 56|4@1+ (1,0) [0|15] "" XXX
SG_ MSR_Eingriffsmoment : 48|8@1+ (0.39,0) [0|99.06] "MDI" XXX
SG_ ASR_Eingriffsmoment_schnell : 40|8@1+ (0.39,0) [0|99.06] "MDI" XXX
SG_ ASR_Eingriffsmoment_langsam : 32|8@1+ (0.39,0) [0|99.06] "MDI" XXX
Expand Down Expand Up @@ -981,8 +985,8 @@ BO_ 1360 Airbag_2: 2 XXX
SG_ Checksumme_Airbag_2__reserviert : 0|8@1+ (1,0) [0|0] "" XXX

BO_ 80 Airbag_1: 4 XXX
SG_ Checksumme_Airbag_1 : 24|8@1+ (1,0) [0|0] "" XXX
SG_ Zaehler_Airbag_1 : 20|4@1+ (1,0) [0|15] "" XXX
SG_ CHECKSUM : 24|8@1+ (1,0) [0|0] "" XXX
SG_ COUNTER : 20|4@1+ (1,0) [0|15] "" XXX
SG_ Fehlerspeichereintrag : 19|1@1+ (1,0) [0|0] "" XXX
SG_ Frei_Airbag_1_2 : 18|1@1+ (1,0) [0|0] "" XXX
SG_ Airbag_im_Stellgliedtest : 17|1@1+ (1,0) [0|0] "" XXX
Expand Down Expand Up @@ -1084,7 +1088,7 @@ BO_ 872 ACC_System: 8 XXX
SG_ ACS_max_AendGrad : 48|8@1+ (1,0.02) [0.02|5.06] "Unit_MeterPerSeconSquar" XXX

BO_ 1386 ACC_GRA_Anziege: 8 XXX
SG_ ACA_Checksum : 0|8@1+ (1,0) [0|255] "" XXX
Copy link
Contributor

Choose a reason for hiding this comment

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

think it would be nice to keep these names. how do you feel about using the unit field for this instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This has come up in discussion before; my first cut at MQB back in the day kept the signal names intact. I'm amenable to any good solution that lets us tag an existing canonically-named signal as "the checksum signal" or "the counter signal". I think last time I looked at it, there was some other means within the DBC file spec I wanted to use, but (ab)using units is possible too. I will dig up my old conversations and think about it.

Copy link
Collaborator Author

@jyoung8607 jyoung8607 Jun 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For DBCs where we want to keep the canonical signal names, I think the best thing is a DBC attribute that maps those signal names to their special usage. For DBCs that were created by hand and use the conventional CHECKSUM and COUNTER naming, we could fall through to using the name when the signal type attribute is entirely absent.

BA_DEF_ SG_ "opendbc_SignalType" ENUM "DEFAULT", "CHECKSUM", "COUNTER";
...
BA_DEF_DEF_ "opendbc_SignalType" "DEFAULT";
...
BA_ "opendbc_SignalType" SG_ 906 GRA_Checksum 1;
BA_ "opendbc_SignalType" SG_ 906 GRA_Neu_Zaehler 2;
(etc)

We could also get rid of the filename-based magic for selecting the checksum algorithm, even tweaking the algorithm per-message if we need to for situations like GM. I've seen MQB DBCs with our annoying pad byte lookup tables stored in the DBC in a similar way, which is something we could consider.

BA_DEF_ BO_ "opendbc_ChecksumAlgorithm" STRING;
...
BA_DEF_DEF_ "opendbc_ChecksumAlgorithm" "VOLKSWAGEN_PQ";

Something like this is a worthy goal, and I'm even willing to work on it at some point, but I'd rather not add a complex blocker for Volkswagen PQ dashcam upstream if that's okay. If you agree with the approach, I'd want to go back and unwind the signal names we changed years ago for MQB, and I can fix PQ at the same time.

SG_ CHECKSUM : 0|8@1+ (1,0) [0|255] "" XXX
SG_ ACA_StaACC : 8|3@1+ (1,0) [0|7] "" XXX
SG_ ACA_ID_StaACC : 11|5@1+ (1,0) [0|31] "" XXX
SG_ ACA_Fahrerhinw : 16|1@1+ (1,0) [0|1] "" XXX
Expand All @@ -1102,12 +1106,12 @@ BO_ 1386 ACC_GRA_Anziege: 8 XXX
SG_ ACA_Codierung : 56|1@1+ (1,0) [0|1] "" XXX
SG_ ACA_Tachokranz : 57|1@1+ (1,0) [0|1] "" XXX
SG_ ACA_Aend_Zeitluecke : 58|1@1+ (1,0) [0|1] "" XXX
SG_ ACA_Zaehler : 60|4@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 60|4@1+ (1,0) [0|15] "" XXX

BO_ 208 Lenkhilfe_3: 6 XXX
SG_ LH3_Checksumme : 0|8@1+ (1,0) [0|255] "" XXX
SG_ CHECKSUM : 0|8@1+ (1,0) [0|255] "" XXX
SG_ LH3_BS_Spiegel : 8|4@1+ (1,0) [0|15] "" XXX
SG_ LH3_Zaehler : 12|4@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 12|4@1+ (1,0) [0|15] "" XXX
SG_ LH3_LM : 16|10@1+ (1,0) [0|1023] "" XXX
SG_ LH3_LMSign : 26|1@1+ (1,0) [0|1] "" XXX
SG_ LH3_LMValid : 27|1@1+ (1,0) [0|1] "" XXX
Expand All @@ -1118,8 +1122,8 @@ BO_ 208 Lenkhilfe_3: 6 XXX
SG_ LH3_Lenkungstyp : 46|2@1+ (1,0) [0|3] "" XXX

BO_ 978 Lenkhilfe_2: 8 XXX
SG_ LH2_Checksumme : 0|8@1+ (1,0) [0|255] "" XXX
SG_ LH2_Zaehler : 8|4@1+ (1,0) [0|15] "" XXX
SG_ CHECKSUM : 0|8@1+ (1,0) [0|255] "" XXX
SG_ COUNTER : 8|4@1+ (1,0) [0|15] "" XXX
SG_ LH2_Geradeaus : 12|1@1+ (1,0) [0|1] "" XXX
SG_ LH2_Sta_Charisma : 13|3@1+ (1,0) [0|7] "" XXX
SG_ LH2_Sta_HCA : 16|4@1+ (1,0) [0|15] "" XXX
Expand All @@ -1131,8 +1135,8 @@ BO_ 978 Lenkhilfe_2: 8 XXX
SG_ LH2_PLA_Abbr : 52|4@1+ (1,0) [0|15] "" XXX

BO_ 210 HCA_1: 5 XXX
SG_ HCA_Checksumme : 0|8@1+ (1,0) [0|15] "" XXX
SG_ HCA_Zaehler : 8|4@1+ (1,0) [0|15] "" XXX
SG_ CHECKSUM : 0|8@1+ (1,0) [0|15] "" XXX
SG_ COUNTER : 8|4@1+ (1,0) [0|15] "" XXX
SG_ HCA_Status : 12|4@1+ (1,0) [0|15] "" XXX
SG_ LM_Offset : 16|15@1+ (0.03125,0) [0|300] "cNm" XXX
SG_ LM_OffSign : 31|1@1+ (1,0) [0|1] "" XXX
Expand Down Expand Up @@ -1172,16 +1176,22 @@ BO_ 870 AWV: 5 XXX
SG_ AWV_Zaehler : 8|4@1+ (1,0) [0|15] "" Bremsbooster
SG_ AWV_Checksumme : 0|8@1+ (1,0) [0|255] "" Bremsbooster

BO_ 1470 LDW_1: 8 XXX
SG_ Right_Lane_Status : 0|2@1+ (1,0) [0|3] "" XXX
SG_ Left_Lane_Status : 2|2@1+ (1,0) [0|3] "" XXX
SG_ LDW_Direction : 14|1@0+ (1,0) [0|1] "" XXX
SG_ SET_ME_X1 : 18|1@0+ (1,0) [0|1] "" XXX
SG_ Kombi_Lamp_Orange : 19|1@0+ (1,0) [0|1] "" XXX
SG_ Kombi_Lamp_Green : 20|1@0+ (1,0) [0|1] "" XXX
SG_ XX_LDW_Maybe_Warning : 16|1@0+ (1,0) [0|1] "" XXX
SG_ XX_DLCORTLC1 : 24|8@1+ (1,0) [0|255] "" XXX
SG_ XX_DLCORTLC2 : 32|8@1+ (1,0) [0|255] "" XXX
BO_ 1470 LDW_Status: 8 XXX
SG_ LDW_Lernmodus_rechts : 0|2@1+ (1,0) [0|3] "" XXX
SG_ LDW_Lernmodus_links : 2|2@1+ (1,0) [0|3] "" XXX
SG_ LDW_Lernmodus : 9|3@1+ (1,0) [0|3] "" XXX
SG_ LDW_Textbits : 12|4@1+ (1,0) [0|15] "" XXX
SG_ LDW_Gong : 16|2@1+ (1,0) [0|3] "" XXX
SG_ LDW_Kameratyp : 18|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_Lampe_gelb : 19|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_Lampe_gruen : 20|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_SW_Warnung_links : 21|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_SW_Warnung_rechts : 22|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_KD_Fehler : 23|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_DLC : 24|8@1+ (0.01,-1.25) [-1.25|1.25] "" XXX
SG_ LDW_TLC : 32|5@1+ (0.1,0) [0|3] "" XXX
SG_ LDW_Seite_DLCTLC : 37|1@1+ (1,0) [0|1] "" XXX
SG_ LDW_Frueh_Spaet : 38|2@1+ (1,0) [0|3] "" XXX

BO_ 428 Bremse_8: 8 XXX
SG_ BR8_Checksumme : 0|8@1+ (1,0) [0|255] "" XXX
Expand Down Expand Up @@ -1461,6 +1471,12 @@ CM_ SG_ 896 Drosselklappenpoti "Throttle Position";
CM_ SG_ 896 Fahrpedal_Rohsignal "Accelerator Pedal Position";
CM_ SG_ 896 Ansauglufttemperatur "Intake Air Temperature";

CM_ SG_ 912 GK1_Fa_Tuerkont "Status of the driver's door rotary latch";
CM_ SG_ 912 BSK_HL_geoeffnet "Status of the rear left door rotary latch";
CM_ SG_ 912 BSK_HR_geoeffnet "Status of the rear right door rotary latch";
CM_ SG_ 912 BSK_BT_geoeffnet "Status of the passenger door rotary latch";
CM_ SG_ 912 BSK_HD_Hauptraste "Status of trunk lid main detent";

CM_ SG_ 1088 Zaehler_Getriebe_1 "Counter Getriebe_1";
CM_ SG_ 1088 Waehlhebelposition__Getriebe_1_ "Gear Selector Position";

Expand Down