Add zp_assert #12
Add zp_assert #12
Conversation
Common/Inc/assert.h
Outdated
| @@ -0,0 +1,6 @@ | |||
| #ifndef ASSERT_H | |||
There was a problem hiding this comment.
Use a different file name, assert is part of the standard library and we don't want to cause confusion between C standard assert.h and our own assert.
Common/Src/assert.c
Outdated
| if (expr) { | ||
| #warning This should call the los hardfault handler, or something more appropriate | ||
| while (1) { | ||
| } | ||
| } |
There was a problem hiding this comment.
Add a stack trace printout so that we know where the assert was called, as well as printing out the location of where the assert failed to aid debugging
| @@ -0,0 +1,29 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
Pragma once is the same as the include guards directly below. We use include guards throughout our project so I would stick to that
| #pragma once |
|
|
||
| #define GET_LR() __builtin_return_address(0) | ||
| //#define GET_PC(_a) __asm volatile ("mov %0, pc" : "=r" (_a)) // Comment out this for ARM | ||
| #define GET_PC(_a) __asm volatile("1: lea 1b(%%rip), %0;": "=a"(pc)); // Comment this for x86 |
There was a problem hiding this comment.
I think it would be preferable to use some form of #ifdef to determine what platform we are building for and use that to choose the line that is defined.
Not sure but there might be a compiler extension that will give us this info for free.
| typedef struct sAssertInfo { | ||
| uint64_t pc; | ||
| uint64_t lr; | ||
| } sAssertInfo; |
There was a problem hiding this comment.
What do PC and lr stand for? Can we expand their abbreviations?
| if (!expr) { | ||
| //#warning This should call the los hardfault handler, or something more appropriate | ||
| while (1) { | ||
| MY_ASSERT_RECORD(); |
There was a problem hiding this comment.
Why are we using macros instead of normal functions?
|
|
||
| #define GET_LR() __builtin_return_address(0) | ||
| //#define GET_PC(_a) __asm volatile ("mov %0, pc" : "=r" (_a)) // Comment out this for ARM | ||
| #define GET_PC(_a) __asm volatile("1: lea 1b(%%rip), %0;": "=a"(pc)); // Comment this for x86 |
There was a problem hiding this comment.
What does this assembly do? A comment could be useful to explain it here
|
|
||
| void zp_assert(bool expr) { | ||
| if (!expr) { | ||
| //#warning This should call the los hardfault handler, or something more appropriate |
There was a problem hiding this comment.
If we assert, we should probably also set all motors to 0 throttle. Can you think of any other safety related functions that should be performed in the event if an assert?
| @@ -0,0 +1,11 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
I think we want to avoid commiting .project files
* re-named AM * updated to sync with AM changes * reading data from IMU * SF testing architecture from ZP-SW * Fixed linker errors, and properly named pins * LOS_Pos Is the instance getting correct? * Bring Template AM into Main (#6) * re-named AM * updated to sync with AM changes * BMX160 Driver (#7) * Safer Singleton * Datatype changes * gitignore * Changes to include SF, no vApplicationStackOverflowHook Should be setup but not sure why it no work * vApplicationStackOverflowHook * change to SF import * updates * Modified freeRTOS for SF testing * Test material, removed obsolete code * Testing AM validity * Removing obsolete code Co-authored-by: Anthony Luo <[email protected]>
3 participants
Description
Adds an assert for use in ZP. This is to ensure we have a assertion function that we can use safely tear down in the event of a programming error. This assertion function will need to be properly defined with desired functionality (e.g. call the hardfault handler, stop motors, etc.) which will likely depend on LOS
#10 (comment)
Testing
No testing has been done. Implementation still needs to be defined
Documentation
No documentation created
Merge Checklist:
- [ ] The code has been tested on hardware, either by me or someone else.Implementation not yet defined- [ ] Comprehensive unit tests have been made for this changeUnit tests not functional- [ ] Corresponding changes to documentation have been created and links to this documentation are provided within the pull request.No documentation required- [ ] The changes generate no new warnings and compile and run; A screenshot of a successful compile message is attached to the bottom of this PR.Builds are not functioning