openpilot icon indicating copy to clipboard operation
openpilot copied to clipboard

loggerd: add unit tests and fix identified sync issues

Open deanlee opened this issue 8 months ago • 1 comments

resolve: https://github.com/commaai/openpilot/issues/34839

This PR adds unit tests specifically aimed at resolving synchronization issues between encoderd and loggerd. While existing tests thoroughly cover logger rotation, these new test cases address the previously untested logic for synchronizing encoder segment numbers with logger segment numbers.

Key Changes:

  1. Refactored the code to make unit testing feasible. Due to the original design, it was nearly impossible to unit test the synchronization logic. To enable testing, LoggerdState and RemoteEncoder classes were moved to loggerd.h.
  2. The synchronization logic has been encapsulated into a new method RemoteEncoder::syncSegment, which can now be unit tested.

Future Improvements: As part of future work, loggerd.h can be split to separate configuration and logic. For instance, renaming loggerd.h to encoder_config.h and limiting loggerd.h to only contain declarations used by loggerd.cc. However, since this would introduce substantial changes, it will be addressed in a follow-up PR.

deanlee avatar Mar 11 '25 09:03 deanlee