Issues on nissan_rx_checks on file safety_nissan.h
- Dominant language
- Python
- Stars
- 3.4k
- Forks
- 2.3k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 39
Description
The following analysis applies only for NISSAN LEAF.
nissan.safety.h (https://github.com/commaai/panda/blob/master/board/safety/safety_nissan.h#L22) defines 10Hz for the expected reception of CRUISE_STATE msg, while on OpenPilot that frequency is at 50Hz (https://github.com/commaai/openpilot/blob/master/selfdrive/car/nissan/carstate.py#L202).
It is not fully clear which is the correct, because OP CANParser object "checks" (see carstate.py "checks" list) are checking for 50Hz to create timeouts and if the actual case is 10Hz (which is slower), no timeout from OP side looks eminent because from what I have seen I think we need to miss roughly 10 messages to create a timeout.
From Panda FW side, this frequency is used again to verify for possible timeouts from the car, but no matter 10 or 50Hz being the actual case, the relevant consistency checks from the receiving side of the Panda FW are exaggerated quite a bit to define a max of 1s lagging messages from the car (see safety.h , safety_tick function). This means that it doesn't matter if we receive at 10 or 50 Hz since the verification will happen at 1Hz. It should be though corrected on all cases to the actual frequency in order to avoid potential future problems and weird timeouts, if additional verifications are added.
And then again on nissan.safety.h line 29, BRAKE message (hex: 1cc) is defined as having a DLC of 4 bytes, but the nissan dbc defines the same msg with 8 bytes (https://github.com/commaai/opendbc/blob/d597599ebe9fcccf87f42fd588e3b4a4ec39e00a/nissan_leaf_2018.dbc#L70). The real case should be 4, otherwise I believe OP shouldn’t work at all since we should get "controls_allowed" = 0 from Panda FW side. So most likely just a dbc problem here. The actual dbc is used to create data structures from CANParser/CANPacker objects on OP and potential impact on that side of the SW is not clear. Since CANParser will mostly just verify the frequency (which is correct) and CANPacker will pack only specific messages and not this particular one, there probably isnt any major problem from this issue if indeed the correct case is 4. If the correct case is 8 bytes then I would definitely expect Panda Safety to through some error message.
If someone wants to verify these problems, I would be happy to further explain and support potential corrections.
Contributor guide
No contributing guide indexed for this repository
Research direction
Compare board/safety/safety_nissan.h with openpilot selfdrive/car/nissan/carstate.py and opendbc's nissan_leaf_2018.dbc. First verify the Nissan LEAF CRUISE_STATE frequency and BRAKE DLC against the referenced definitions and safety_tick behavior. Done means the correct values are established and the affected definitions are made consistent, with the relevant behavior checked for regressions.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100