[$500 Bounty] Refactor panda to use source + header file structure

[robbederks]

It's time we move the repo from the current style (everything is a header file) to having separate source and header files. This should clean up all the import orders and *_definitions.h files, as well as make it a lot easier to add new drivers etc.

Bounty for a clean PR with minimal changes.

[robbederks]

Locked to #2175 for now.

[aubsw]

I spent some time trying to break things down into smaller PRs, but now I'm not so sure how feasible thi will be because I'm struggling with the MISRA checks.

Here's what we have right now:

1. https://github.com/commaai/opendbc/pull/2076 which the pre-factor to opendbc to make our changes to panda happy
2. https://github.com/commaai/panda/pull/2175 the whole panda change in one PR

I need to write up some more notes on the PR description themselves.

@robbederks do you have any guidance on how much I should care about he MISRA checks? Are there any kinds of checks I can fudge?

[aubsw]

@robbederks Here's where we stand:

• I was able to make do the refactor without https://github.com/commaai/panda/pull/2181, but it did require changes to opendbc. We are forced to address the namespace conflict in these declarations as long as we need to link the safety.h headers.

  Crucially, we add matching guards in can.h in both opendbc and panda, as these files have conflicting definitions at compile time. I.e.,

diff --git a/opendbc/safety/board/can_declarations.h b/opendbc/safety/board/can_declarations.h
index 3824f01..589d466 100644
--- a/opendbc/safety/board/can_declarations.h
+++ b/opendbc/safety/board/can_declarations.h
@@ -1,4 +1,5 @@
-#pragma once
+#ifndef CAN_DEFINITION_H
+#define CAN_DEFINITION_H
 
 #define CANPACKET_HEAD_SIZE 6U
 
@@ -24,3 +25,6 @@ typedef struct {
 
 #define GET_BUS(msg) ((msg)->bus)
 #define GET_ADDR(msg) ((msg)->addr)
+
+#endif // CAN_DEFINITION_H
+

• The refactor is implemented at https://github.com/commaai/panda/pull/2175
• But we need to land opendbc changes first in order for the refactor to compile https://github.com/commaai/opendbc/pull/2076

@robbederks Are you happy with this approach?

[robbederks]

@robbederks Are you happy with this approach?

Yes, this looks more sensible to me until we can completely dedupe the code. I'll start reviewing the main PR superficially already since it's so big, and then once the opendbc PR is merged we can hopefully finalize the refactor asap.

[aubsw]

Sounds good. Also, I think there's one opportunity for a pre-factor in panda: refactor the various "config"/definition headers. This would basically be a no-op change to clarify the config dependency chain before the final PR. This change is already implemented in the big PR.

Could reduce the diff size in the main PR by ~300 lines give or take.

[robbederks]

anything to reduce that diff is welcome! will hold off on reviewing further

[aubsw]

@robbederks prefactor PR is ready for you https://github.com/commaai/panda/pull/2180

[aubsw]

@robbederks some updates:

I have made the changes you requested locally, and it seems to work, but a new change merged to opendbc has introduced a blocker.

https://github.com/commaai/opendbc/pull/2088 just broke our refactor. In particular, the static re-declarations cause problems, e.g.:

arm-none-eabi-gcc -o board/jungle/safety-panda_jungle_h7.o -c -mcpu=cortex-m7 -mhard-float -DSTM32H7 -DSTM32H725xx -Iboard/stm32h7/inc -mfpu=fpv5-d16 -fsingle-precision-constant -Os -g -DPANDA_JUNGLE -DALLOW_DEBUG -Wall -Wextra -Wstrict-prototypes -Werror -mlittle-endian -mthumb -nostdlib -fno-builtin -std=gnu11 -fmax-errors=1 -Tboard/stm32h7/stm32h7x5_flash.ld -Iboard/jungle -Iboard -I. -I/Users/aub/programming/panda-cpy/opendbc -I/Users/aub/programming/panda-cpy/opendbc/safety -Iboard -Iinclude -Iinclude/board board/safety.c
In file included from board/jungle/main.c:4:
/Users/aub/programming/panda-cpy/opendbc/safety/safety_declarations.h:230:13: error: 'generic_rx_checks' declared 'static' but never defined [-Werror=unused-function]
  230 | static void generic_rx_checks(void);
      |             ^~~~~~~~~~~~~~~~~

Source/header model in panda is incompatible with this style of static function forward declaration like this.

~I opened https://github.com/commaai/opendbc/pull/2101 to unblock us. Otherwise, we need to completely refactor safety.h, etc as a prereq for our refactor.~ Actually made a cleaner seperate PR for this.

~How do you want to proceed?~

[aubsw]

@robbederks OK I think I've finally figured out how to make this CLEAN 🍋 ✨

1. https://github.com/commaai/opendbc/pull/2076 minimal no-op refactor of opendbc
2. https://github.com/commaai/panda/pull/2188
3. https://github.com/commaai/panda/pull/2189
4. https://github.com/commaai/panda/pull/2186 delete duplicated utils.h and can.h from panda
5. https://github.com/commaai/panda/pull/2185 prefactor (mostly just adding forward decl.)
6. https://github.com/aubsw/panda/pull/2/files the actual refactor (CLEAN EDITION)We need to merge in that order. Please take a look when you have some time.

[jordandiazdiaz]

robbederks ready for you https://github.com/commaai/panda/pull/2237

[jordandiazdiaz]

@adeebshihadeh ready for you

[Dr-Drone]

Seems like there are some commits already, is this still open?

[ReyNeill]

Is the bounty expecting zero functional diff (bit‑for‑bit firmware), or is a small ELF layout change acceptable?