search

commaai / panda

code powering the comma.ai panda
MIT License
1.52k stars 757 forks source link

Panda aborts CAN send on NACK #1922

Open pd0wm opened 5 months ago

pd0wm commented 5 months ago

I noticed the panda no longer retries sending a message when it's not ACKed. I'm opening this issue to check if this is intentional.

I think this behavior was introduced in this PR: https://github.com/commaai/panda/pull/1067/files. An interrupt was added for CAN_IER_LECIE, which includes getting a NACK. This in turn triggers an abort of the send in llcan_clear_send.

If this is intentional behavior, it's probably cleaner to put the CAN controller in "Time Triggered CAN" mode where retries are disabled by default, and no manual cancellation is necessary. If this is intentional I would also like to suggest to return the message as "blocked/error" to the user, because now the message disappears into thin air.

Quick hack to bring back retry behavior on F4 pandas, but there are probably other errors that need to be retried as well (e.g. form error). https://github.com/pd0wm/panda/commit/d1fc60b79993428c6b109ab28a5dc1c107dd6cd9

gregjhogan commented 5 months ago

I believe we intend to infinitely retry sending messages that get into the TX ring buffer (since any device on the bus will ACK) as mentioned in #421 and #968. I think silently dropping a message that makes it into the TX ring buffer is always bad. It seems like we need a test that ensures we retry until ACK.