Skip to content

Conversation

@jakubberkop
Copy link

@jakubberkop jakubberkop commented Nov 28, 2025

Work in progress for #2171.
Is this the refactor you have in mind? @robbederks

Building of jungle and body binaries are commented out of now, just to get a poc of the refactor.

@robbederks
Copy link
Contributor

Haven't reviewed in depth, but I think this is roughly in the same direction I'd start. Obviously still needs a lot of work to get the subprojects building and all tests passing again, after that I can give it a closer look.

@jakubberkop jakubberkop force-pushed the source_and_headers branch 4 times, most recently from 2e74d6c to 63dfd6b Compare December 5, 2025 22:46
@jakubberkop
Copy link
Author

jakubberkop commented Dec 5, 2025

Ready for a review. @robbederks

  • I have have suppressed few things in the tests/misra/test_misra.sh, most notably the opendbc includes, as they were causing issues and modifying them seemed out of scope for this task.
  • I also suppressed unused function warnings, for functions that were only used in board/bootstub.c, which is also excluded from the analysis.

@jakubberkop jakubberkop marked this pull request as ready for review December 5, 2025 23:01
@jakubberkop jakubberkop force-pushed the source_and_headers branch 2 times, most recently from 0a89478 to 45c876b Compare January 6, 2026 16:33
@jakubberkop
Copy link
Author

@robbederks please take a look when you have a chance.

@robbederks
Copy link
Contributor

robbederks commented Jan 7, 2026

trigger-jenkins

@robbederks
Copy link
Contributor

@jakubberkop: looks like it builds and flashes, but doesn't boot on the panda jungle and thus doesn't try it on the pandas either. do you have any hardware to test this on? does it boot on pandas? If not I can try to debug it too

@jakubberkop
Copy link
Author

@robbederks: Unfortunately no, I don't have any hardware.
I went once again through all of my changes related to panda jungle, and couldn't find anything, so help in debugging it would be greatly appreciated.

@robbederks
Copy link
Contributor

robbederks commented Jan 12, 2026

@jakubberkop the STM hangs in the bootloader right after initializing the interrupts. I've loaded bootloader.elf in ghidra, and all the interrupt handlers map to the same do {} while(true); loop, so this isn't surprising.

I see you've also had to add an unusedFunction exception to handle_interrupt, which it shouldn't need.

Lmk once you debugged it and would like me to test on hw again

@jakubberkop
Copy link
Author

@robbederks thanks for the help!
As suggested the issues was with the Interrupt handlers. I forgot to add the interrupt_handlers.c to the SConscript, and that didn't result in a undefined reference to a symbol errors, because those functions were marked as weakly linked.

Should be ready for tests on hw.

@robbederks
Copy link
Contributor

Great. I'll try again tomorrow.

Any way you can set linker flags to throw errors for mistakes like these? Failing silently doesn't seem like a great option going forward

@robbederks
Copy link
Contributor

In general we also try to avoid adding more misra suppressions and fixing the underlying issues instead. Haven't looked too extensively for other ones that have been added, but would be good to double check that

@robbederks
Copy link
Contributor

@jakubberkop the RSA signature check was failing in the bootstub. Looks like an off-by-one bug was introduced in rsa.c, I've fixed it.

Can you double check that there are no other changes like this introduced? It's a bit scary in a large PR like this to have functional changes like this since they're very hard to review due to the sheer diff size

@robbederks
Copy link
Contributor

fixed a CAN buffer bug where they weren't in the right memory section, that works now.

for some reason the main red LED breathes at like 2.5x speed with this PR though, didn't see anything obvious why that is the case (is the core running at a different frequency?). The uptime counter does look correct at 1Hz so the main PLLs should be fine

@robbederks
Copy link
Contributor

Figured out the fast LED breathing too: looks like this is the first time the delay function is aligned in memory causing it to run drastically faster. Fixed with #2318 which should be mergeable once HITL CI is back up, after that this PR needs to be rebased on that.

@jakubberkop
Copy link
Author

I went again through all of the files, and compared them the master branch. I have found a few small changes that I introduced by accident and fixed it here (commit b80bcd7). This also removed extraneous misra suppressions.
To prevent silent failures of the linking of interrupt handlers, I have commented the .weak definitions in the linker script (and left them as a reference if we ever remove them).

Interesting that we can get such a huge speedup by just alignment of the delay function. I compared the generated code yesterday, and that the only thing I noticed, but didn't know that it could have had such a big effect. I guess you learn something every day.

@jakubberkop
Copy link
Author

Rebased after #2318

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