openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

pandad: refactor to single thread

Open deanlee opened this issue 1 year ago • 2 comments

To minimize changes and avoid introducing new issues, the original C-style functions have been retained.

Main Changes

  1. Renamed xxx_thread() functions to xxx().
  2. Changed variables in the original thread functions to static to retain values across calls.
  3. Moved send_peripheral_state from panda_state() to peripheral_state() for a better fit.
  4. Move 'safety_future' to a global variable to ensure the safety_setter_thread gracefully quits on exit.
  5. Prefix all global variables with 'g_'

Additional Note

A possible better approach to eliminate the static variables is to declare classes for PeripheralState, PandaState, etc., and move the static variables to their member variables. The final code might look something like this:

void PandaD::run() {
    ...
    panda_state.process();
    peripheral_state.process();
}

However, since the pandad process always restarts after failures, keeping these variables as static in this PR is harmless.

deanlee avatar Jun 10 '24 13:06 deanlee

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

github-actions[bot] avatar Jun 10 '24 13:06 github-actions[bot]

Nice work! I added to the 0.9.8 milestone and will try to get this merged soon.

adeebshihadeh avatar Jun 15 '24 23:06 adeebshihadeh

trigger-jenkins

adeebshihadeh avatar Jul 24 '24 17:07 adeebshihadeh

@Quantizr can you verify this in the testing closet and merge if it looks good?

adeebshihadeh avatar Aug 05 '24 23:08 adeebshihadeh

Looks good on the testing closet, significant reduction in the variability of the timing of pandaStates messages, going from a relative standard deviation of ~0.0175 to ~0.0049 and small improvements in various other service timings. This PR timings: testing comma life_revisions_6d2fed1745fb901f8b77d1086e27be8945e6c97d

Prior timings: testing comma life_revisions_d00a53983089be6bed0e037474abfea823f2fc3e

Quantizr avatar Aug 07 '24 03:08 Quantizr