Skip to content

Dedupe opendbc code#2181

Closed
aubsw wants to merge 6 commits intocommaai:masterfrom
aubsw:dedupe-code
Closed

Dedupe opendbc code#2181
aubsw wants to merge 6 commits intocommaai:masterfrom
aubsw:dedupe-code

Conversation

@aubsw
Copy link
Copy Markdown
Contributor

@aubsw aubsw commented Apr 4, 2025

Needed for #2171

Dedupes the code in panda that was cribbed from opendbc.

details:

  1. As a convention, we now include headers from opendbc using with the relative path from opendbc.
    E.g. #include "safety.h" now becomes #include "safety/safety.h"
  2. Delete duplicated code.
  3. Fix MISRA test invocation (now it can see opendbc headers).

why?

  1. Conflicts

    The duplicated code was causing issues during compilation when trying to set up the source/header model for [$500 Bounty] Refactor panda to use source + header file structure #2171.

    The duplicated macros defined at opendbc/safety/board/utils.h and board/utils.h were confusing the compiler during preprocessing, e.g.:

panda % arm-none-eabi-gcc -o board/bootstubx-safety-panda.o -c -mcpu=cortex-m4 -mhard-float -DSTM32F4 -DSTM32F413xx -Iboard/stm32f4/inc # [... truncated for brevity]
In file included from /Users/aub/programming/panda-cpy/opendbc/safety/safety.h:6,
                 from /Users/aub/programming/panda-cpy/opendbc/safety/main.c:1:
/Users/aub/programming/panda-cpy/opendbc/safety/board/utils.h:38:21: error: expected identifier or '(' before 'void'
   38 | #define UNUSED(x) ((void)(x))
      |                     ^~~~
/Users/aub/programming/panda-cpy/opendbc/safety/main.c:6:1: note: in expansion of macro 'UNUSED'
    6 | UNUSED(heartbeat_engaged);
      | ^~~~~~

I don't think there's a sensible work-around aside from deleting the duplicated macros outright.

  1. Comprehensibility

    The change in import paths for headers clarifies where the code is coming from. May help protect us against namespace collisions in the future.

@aubsw aubsw changed the title Dedup opendbc code Dedupe opendbc code Apr 4, 2025
@aubsw aubsw marked this pull request as ready for review April 4, 2025 23:47
@aubsw
Copy link
Copy Markdown
Contributor Author

aubsw commented Apr 4, 2025

@robbederks WDYT of this approach as a first step? This help shrink the diff in subsequent PRs and also makes things a bit easier to reason about, IMO.

I still need to figure out the MISRA checks.

I figured out the issue with the MISRA checks.

@robbederks
Copy link
Copy Markdown
Contributor

Splitting up the code like this is pretty ugly (especially things like the FAULT list being split over the two repos). The duplicated code we have now in panda / opendbc is obviously bad already, but this seems like a step in the wrong direction to me. Requires a bit more thought / refactoring to abstract away cleanly I think (@sshane any ideas?)

Any way we can finish your refactor without these changes in the meantime?

@aubsw
Copy link
Copy Markdown
Contributor Author

aubsw commented Apr 7, 2025

@robbederks I will give it a shot. Closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants