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.

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

Synchronize controls with the CAN bus #547

Closed
wants to merge 33 commits into from

Conversation

Gernby
Copy link

@Gernby Gernby commented Mar 4, 2019

This enhancement works way better than I hoped, and actually reduces the CPU utilization of boardd. The primary goal was to ensure that steering sensor data from the vehicle was neither skipped OR reused by controls due to mismatches in sample timing.

The current logic uses a static 5 ms sleep time between the end of one CAN sample and the start of the next CAN sample in boardd. This results in very irregular sample resolution due to varying traffic loads and USB thread contention. For cars with heavy CAN traffic, the sample period of a particular 100Hz CAN frame can be deteriorated to an effective alternating 50Hz / 200Hz rate. This irregularity causes a direct deterioration of controls.

non-syncid

The proposed logic has 2 synchronization modes. Both modes require the cp.update() method to wait for a packet from boardd. The difference is in the way boardd determines when to send the packet. If the new "syncID" value is set in interface for a vehicle, then boardd will synchronize itself on the receipt of that CAN frame. This ensures that controls will never outpace the CAN bus (even if it runs below 100Hz).

syncid

The second mode is the default mode, when there isn't a syncID defined in interface. In this mode, boardd will precisely synchronize on time. It will drain the Panda 3 times per 0.01 seconds, but it will only send the data 1 time per 0.01 seconds. To ensure precision, the first 2 calls to the Panda will occur at a "soft" 4.5 ms, with the sleep occurring outside of the USB lock. However, the 3rd call to the Panda will occur inside the USB lock, so that USB contention doesn't disrupt timing.

In order to prevent irregularity in control processing after the packet is received from boardd, the existing rate keeper is used to stage the data that will be sent back to the vehicle. However, there can be only 1 absolute rate keeper, so an optional offset was added to the rk.keep_time() method. This allows for occasional disruptions in the system without overrunning the secondary rate keeper.

@geohot
Copy link
Contributor

geohot commented May 31, 2019

This functionality will be in 0.5.13, thanks for the PR!

@Gernby
Copy link
Author

Gernby commented Jun 1, 2019

Awesome! I need to review the code, and make sure that it contains my "learnings"!

@Gernby
Copy link
Author

Gernby commented Jun 1, 2019

I found that synchronizing on CAN ID was too problematic without significant changes to the Panda logic, AND found that time synchronization alone provided almost all of the benefit. The code was also simplified.

@pd0wm pd0wm closed this Jun 6, 2019
rav4kumar added a commit to rav4kumar/openpilot that referenced this pull request Feb 22, 2020
pd0wm pushed a commit that referenced this pull request May 31, 2020
49ffbe9 disable non-universal checks in hyundai safety
3a85f4c use whole route when running safety replay from CLI
098f47a Fix leaf brake rx check (#547)
b826734 Add pre commit checks + CI (#545)
339976c document tx message addresses better (#543)
a618e64 fix typing errors
9bece64 use mazda init
08db086 mazda cleanup
89658d0 Mazda: safety tests add missing safety checks (#525)
bdec139 Fix length of 0x20b in NISSAN_TX_MSGS, wasn't cancelling ACC (#544)
b48c74c Adding UNO to automated tests (#538)
a5802cd Hyundai: remove unused message from RX checks
9ebde25 Reset state on safety mode init (#542)
d4f3f15 Refactor addr check (#541)
5210e51 remove unused files
0657064 Hyundai checksum (#540)
07e668e Fast CI (#539)
5307bf7 Fix multi message iso tp requests
0610ed1 Hyundai wheel speed counter is actually 4 bits spread over two signals
0d581aa dockerfile optimization
eaefa2f fix docker file path
243a65f pull base image
0dd9470 only push to dockerhub from master
55b79b4 GitHub Actions (#535)
b2c720b Dos (#533)
01bf740 remove 0x1BE checksum test
0bd06c9 remove 0x1BE check (breaks some vehicles)
c31b899 honda bosch longitudinal safety
66250c4 Disable docker layer caching (#534)
6b19fa4 include nissan safety in release build
db31886 gate mazda safety behind debug flag
e4558c0 Safety: message length check on RX and TX (#529)

git-subtree-dir: panda
git-subtree-split: 49ffbe9
pd0wm pushed a commit that referenced this pull request May 31, 2020
9102c16 two spaces before inline comment
49ffbe9 disable non-universal checks in hyundai safety
3a85f4c use whole route when running safety replay from CLI
098f47a Fix leaf brake rx check (#547)
b826734 Add pre commit checks + CI (#545)
339976c document tx message addresses better (#543)
a618e64 fix typing errors
9bece64 use mazda init
08db086 mazda cleanup
89658d0 Mazda: safety tests add missing safety checks (#525)
bdec139 Fix length of 0x20b in NISSAN_TX_MSGS, wasn't cancelling ACC (#544)
b48c74c Adding UNO to automated tests (#538)
a5802cd Hyundai: remove unused message from RX checks
9ebde25 Reset state on safety mode init (#542)
d4f3f15 Refactor addr check (#541)
5210e51 remove unused files
0657064 Hyundai checksum (#540)
07e668e Fast CI (#539)
5307bf7 Fix multi message iso tp requests
0610ed1 Hyundai wheel speed counter is actually 4 bits spread over two signals
0d581aa dockerfile optimization
eaefa2f fix docker file path
243a65f pull base image
0dd9470 only push to dockerhub from master
55b79b4 GitHub Actions (#535)
b2c720b Dos (#533)
01bf740 remove 0x1BE checksum test
0bd06c9 remove 0x1BE check (breaks some vehicles)
c31b899 honda bosch longitudinal safety
66250c4 Disable docker layer caching (#534)
6b19fa4 include nissan safety in release build
db31886 gate mazda safety behind debug flag
e4558c0 Safety: message length check on RX and TX (#529)

git-subtree-dir: panda
git-subtree-split: 9102c16
pd0wm pushed a commit that referenced this pull request Jun 1, 2020
d7f7b14 Enable almost all Flake8 checks (#548)
3d5a717 fix whitespace after opening bracket
f70ef29 whitespace fix
eba113c backslash is redundant between brackets
8039638 fix linter complaint about unexpected spaces around keyword / parameter equals
275e76c fix linter complaint about too many blank lines
9102c16 two spaces before inline comment
49ffbe9 disable non-universal checks in hyundai safety
3a85f4c use whole route when running safety replay from CLI
098f47a Fix leaf brake rx check (#547)
b826734 Add pre commit checks + CI (#545)
339976c document tx message addresses better (#543)
a618e64 fix typing errors
9bece64 use mazda init
08db086 mazda cleanup
89658d0 Mazda: safety tests add missing safety checks (#525)
bdec139 Fix length of 0x20b in NISSAN_TX_MSGS, wasn't cancelling ACC (#544)
b48c74c Adding UNO to automated tests (#538)
a5802cd Hyundai: remove unused message from RX checks
9ebde25 Reset state on safety mode init (#542)
d4f3f15 Refactor addr check (#541)
5210e51 remove unused files
0657064 Hyundai checksum (#540)
07e668e Fast CI (#539)
5307bf7 Fix multi message iso tp requests
0610ed1 Hyundai wheel speed counter is actually 4 bits spread over two signals
0d581aa dockerfile optimization
eaefa2f fix docker file path
243a65f pull base image
0dd9470 only push to dockerhub from master
55b79b4 GitHub Actions (#535)
b2c720b Dos (#533)
01bf740 remove 0x1BE checksum test
0bd06c9 remove 0x1BE check (breaks some vehicles)
c31b899 honda bosch longitudinal safety
66250c4 Disable docker layer caching (#534)
6b19fa4 include nissan safety in release build
db31886 gate mazda safety behind debug flag
e4558c0 Safety: message length check on RX and TX (#529)

git-subtree-dir: panda
git-subtree-split: d7f7b14
pd0wm pushed a commit that referenced this pull request Jun 1, 2020
d7f7b14 Enable almost all Flake8 checks (#548)
3d5a717 fix whitespace after opening bracket
f70ef29 whitespace fix
eba113c backslash is redundant between brackets
8039638 fix linter complaint about unexpected spaces around keyword / parameter equals
275e76c fix linter complaint about too many blank lines
9102c16 two spaces before inline comment
49ffbe9 disable non-universal checks in hyundai safety
3a85f4c use whole route when running safety replay from CLI
098f47a Fix leaf brake rx check (#547)
b826734 Add pre commit checks + CI (#545)
339976c document tx message addresses better (#543)
a618e64 fix typing errors
9bece64 use mazda init
08db086 mazda cleanup
89658d0 Mazda: safety tests add missing safety checks (#525)
bdec139 Fix length of 0x20b in NISSAN_TX_MSGS, wasn't cancelling ACC (#544)
b48c74c Adding UNO to automated tests (#538)
a5802cd Hyundai: remove unused message from RX checks
9ebde25 Reset state on safety mode init (#542)
d4f3f15 Refactor addr check (#541)
5210e51 remove unused files
0657064 Hyundai checksum (#540)
07e668e Fast CI (#539)
5307bf7 Fix multi message iso tp requests
0610ed1 Hyundai wheel speed counter is actually 4 bits spread over two signals
0d581aa dockerfile optimization
eaefa2f fix docker file path
243a65f pull base image
0dd9470 only push to dockerhub from master
55b79b4 GitHub Actions (#535)
b2c720b Dos (#533)
01bf740 remove 0x1BE checksum test
0bd06c9 remove 0x1BE check (breaks some vehicles)
c31b899 honda bosch longitudinal safety
66250c4 Disable docker layer caching (#534)
6b19fa4 include nissan safety in release build
db31886 gate mazda safety behind debug flag
e4558c0 Safety: message length check on RX and TX (#529)

git-subtree-dir: panda
git-subtree-split: d7f7b14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants