-
Notifications
You must be signed in to change notification settings - Fork 886
Refactor to use source & headers structure #2302
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
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. |
2e74d6c to
63dfd6b
Compare
|
Ready for a review. @robbederks
|
63dfd6b to
45ff79f
Compare
0a89478 to
45c876b
Compare
|
@robbederks please take a look when you have a chance. |
|
trigger-jenkins |
|
@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 |
|
@robbederks: Unfortunately no, I don't have any hardware. |
|
@jakubberkop the STM hangs in the bootloader right after initializing the interrupts. I've loaded I see you've also had to add an Lmk once you debugged it and would like me to test on hw again |
|
@robbederks thanks for the help! Should be ready for tests on hw. |
|
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 |
|
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 |
|
@jakubberkop the RSA signature check was failing in the bootstub. Looks like an off-by-one bug was introduced in 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 |
|
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 |
c226f3c to
8651137
Compare
|
Figured out the fast LED breathing too: looks like this is the first time the |
|
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. 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. |
…red interrupt functions are not linked
79e8eab to
8b68ab8
Compare
|
Rebased after #2318 |
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.