Skip to content

Revert "enable cpplint"#2245

Merged
sshane merged 1 commit intomasterfrom
revert-2238-enable-cpplint
May 15, 2025
Merged

Revert "enable cpplint"#2245
sshane merged 1 commit intomasterfrom
revert-2238-enable-cpplint

Conversation

@sshane
Copy link
Copy Markdown
Contributor

@sshane sshane commented May 15, 2025

Reverts #2238

Breaks openpilot compilation

batman@workstation-shane:~/openpilot$ scons -j32
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
Retrieved `opendbc_repo/opendbc/can/common.os' from cache
Retrieved `opendbc_repo/opendbc/can/dbc.os' from cache
Retrieved `opendbc_repo/opendbc/can/parser.os' from cache
Retrieved `opendbc_repo/opendbc/can/packer.os' from cache
Retrieved `opendbc_repo/opendbc/safety/tests/libsafety/safety.os' from cache
Retrieved `opendbc_repo/opendbc/can/libdbc.so' from cache
Retrieved `opendbc_repo/opendbc/safety/tests/libsafety/libsafety.so' from cache
arm-none-eabi-gcc -o panda/board/jungle/main-panda_jungle.o -c -mcpu=cortex-m4 -mhard-float -DSTM32F4 -DSTM32F413xx -Iboard/stm32f4/inc -mfpu=fpv4-sp-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 -Tpanda/board/stm32f4/stm32f4_flash.ld -Ipanda/board/jungle -Ipanda/board -Ipanda -Ipanda/board -Iopendbc/safety panda/board/jungle/main.c
Retrieved `opendbc_repo/opendbc/can/packer_pyx.so' from cache
Retrieved `opendbc_repo/opendbc/can/parser_pyx.so' from cache
arm-none-eabi-gcc -o panda/board/jungle/main-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 -Tpanda/board/stm32h7/stm32h7x5_flash.ld -Ipanda/board/jungle -Ipanda/board -Ipanda -Ipanda/board -Iopendbc/safety panda/board/jungle/main.c
In file included from panda/board/jungle/main.c:4:
opendbc/safety/safety.h:3:10: fatal error: safety/safety_declarations.h: No such file or directory
    3 | #include "safety/safety_declarations.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [panda/board/jungle/main-panda_jungle.o] Error 1
arm-none-eabi-gcc -o panda/board/main-panda.o -c -mcpu=cortex-m4 -mhard-float -DSTM32F4 -DSTM32F413xx -Iboard/stm32f4/inc -mfpu=fpv4-sp-d16 -fsingle-precision-constant -Os -g -DPANDA -DALLOW_DEBUG -Wall -Wextra -Wstrict-prototypes -Werror -mlittle-endian -mthumb -nostdlib -fno-builtin -std=gnu11 -fmax-errors=1 -Tpanda/board/stm32f4/stm32f4_flash.ld -Ipanda/board -Ipanda -Ipanda -Ipanda/board -Iopendbc/safety panda/board/main.c
In file included from panda/board/jungle/main.c:4:
opendbc/safety/safety.h:3:10: fatal error: safety/safety_declarations.h: No such file or directory
    3 | #include "safety/safety_declarations.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [panda/board/jungle/main-panda_jungle_h7.o] Error 1
In file included from panda/board/main.c:13:
opendbc/safety/safety.h:3:10: fatal error: safety/safety_declarations.h: No such file or directory
    3 | #include "safety/safety_declarations.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [panda/board/main-panda.o] Error 1
scons: building terminated because of errors.

@github-actions github-actions bot added can related to CAN tools, aka opendbc/can/ car safety vehicle-specific safety code labels May 15, 2025
@sshane sshane merged commit f960080 into master May 15, 2025
10 checks passed
@sshane sshane deleted the revert-2238-enable-cpplint branch May 15, 2025 02:46
@adeebshihadeh
Copy link
Copy Markdown
Contributor

How can we be more robust to this?

@aubsw
Copy link
Copy Markdown
Contributor

aubsw commented May 15, 2025

Sorry about the breakage.

Since OP currently builds with opendbc@master, there are two options that I can think of:

A. peg opendbc to a specific version in OP repo
B. add a premerge step to opendbc PRs to check that OP compiles with new change

I assume you don't want to do A. for sake of development velocity, but let me know if you think otherwise.

aubsw added a commit to aubsw/opendbc that referenced this pull request May 15, 2025
@sshane
Copy link
Copy Markdown
Contributor Author

sshane commented May 15, 2025

I think the port from panda to opendbc was a bit hacky to get the paths working properly, we can probably refactor that a ton

@aubsw
Copy link
Copy Markdown
Contributor

aubsw commented May 15, 2025

we can probably refactor that a ton

Ahaha this is what I'm trying to do in commaai/panda#2171 (comment) 🫠

The main problem is that there's duplicated code in opendbc and panda, which makes it extremely are to refactor without compilation errors AND without breaking OP.

If you don't mind breaking OP for a few minutes, I have 3 PRs that will fix the duplicated code problem. This will in turn allow us to land the cpplint PR.

They need to be merged in this order:

  1. Forward-declare dlc_to_len #2248
  2. Delete duplicated code from panda panda#2186
  3. Get dlc_to_len from opendbc openpilot#35239

Please let me know if you see a different path... This is all I have after massaging this for a really long time.

@sshane
Copy link
Copy Markdown
Contributor Author

sshane commented May 15, 2025

Ideally we can do this piece wise without putting opendbc in a bad state for external projects. Revert one and suddenly something is broken

@aubsw
Copy link
Copy Markdown
Contributor

aubsw commented May 15, 2025

Just to be clear, the changes proposed above never put opendbc itself in a bad state–it does temporarily break panda and OP tho.

We have 3 options:

  1. keep Forward-declare dlc_to_len #2248 as-is
  2. Make dlc_to_len not static
  3. Revive can.h but still move the definition of dlc_to_len

Do you have a preference?

@aubsw
Copy link
Copy Markdown
Contributor

aubsw commented May 15, 2025

I thought about it some more. Option #2 is the least problematic option. We can rely on MISRA checks to prevent divergent declarations of dlc_to_len.

I updated the PR accordingly.

adeebshihadeh added a commit that referenced this pull request May 18, 2025
adeebshihadeh added a commit that referenced this pull request May 18, 2025
* Reapply "enable cpplint" (#2245)

This reverts commit f960080.

* include from root

* revert that

* clean that up

* seriously?
vanillagorillaa pushed a commit to vanillagorillaa/opendbc that referenced this pull request May 19, 2025
* Reapply "enable cpplint" (commaai#2245)

This reverts commit f960080.

* include from root

* revert that

* clean that up

* seriously?
eli-davis pushed a commit to eli-davis/opendbc_reference that referenced this pull request Jul 15, 2025
Revert "enable cpplint (commaai#2238)"

This reverts commit 7c4d6be.
eli-davis pushed a commit to eli-davis/opendbc_reference that referenced this pull request Jul 15, 2025
* Reapply "enable cpplint" (commaai#2245)

This reverts commit 84817b8.

* include from root

* revert that

* clean that up

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

Labels

can related to CAN tools, aka opendbc/can/ car safety vehicle-specific safety code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants