You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
The text was updated successfully, but these errors were encountered:
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.
The text was updated successfully, but these errors were encountered: