Initial Rev 3 flash driver#261
Conversation
…etry/Flight-Computer-Firmware into feature/ES/r3-flash
NArmistead
left a comment
NArmistead
left a comment
There was a problem hiding this comment.
LGTM, but the code format in flash_validation_routine is nonstandard for FCF
| for(uint32_t i = 0; i < FLASH_MAX_ADDR; i+=FLASH_PAGE_SIZE) { | ||
| flash_handle.address = i; | ||
| flash_status = flash_read(&flash_handle, 2); | ||
| assert_fail_fast( flash_status == FLASH_OK, ERROR_FLASH_CMD_ERROR ); | ||
| assert_fail_fast( flash_buf[0] == 127, ERROR_FLASH_CMD_ERROR ); | ||
| assert_fail_fast( flash_buf[1] == 0xFF, ERROR_FLASH_CMD_ERROR ); | ||
| } |
There was a problem hiding this comment.
see code format here, for example
There was a problem hiding this comment.
This code is temporary and will most likely not be on the final product of rev 3. It will likely become a test utility after app integration on the new platform. Regardless, I will move to standardize the code seen here.
| flash_validation_routine(); | ||
|
|
||
|
|
||
| } /* run_hardware_validation */ No newline at end of file |
There was a problem hiding this comment.
I think we should add some indication that the routine is finished
There was a problem hiding this comment.
My intention is to eventually set this up to use the uart debug interface for driver development, which should come soon.
266-750Balloons
left a comment
266-750Balloons
left a comment
There was a problem hiding this comment.
This looks fine to me. Flash isn't my area of expertise, but this looks good besides my notes.
Sorry for taking forever to get to this.
|
|
||
| #ifdef DO_HARDWARE_VALIDATION | ||
| run_hardware_validation(); | ||
| while (1); |
There was a problem hiding this comment.
I feel like there's a good chance that while(1) and the way the validation is done her in general is accidentally going to cost someone a particularly frustrating 45 minutes one day when they're trying to figure out why the firmware is seemingly not working, only to find out they had forgotten to remove this flag during a previous debug session.
I feel like we need some warning for this; at the very least, we could have a well-documented LED sequence to signal that it's a hardware validation build, and I also wonder if we could do something to tell SDEC and have it warn a user that they're on a hardware validation build.
There was a problem hiding this comment.
This is temporary. If we keep these routines after the initial HW bring up, they'll live in test and need a different way to be compiled into the FW. Right now, this is the only way we'll be running rev 3 until we port the app. Gonna go NAT.
Also, I intend to have our debug logging indicate the results of HW validation once implemented for rev 3.
3 participants
Description
Adds flash support to rev3 test app
Issue Link
Parent: SunDevilRocketry/driver#30
Depends on: SunDevilRocketry/mod#127 (can be reviewed separately though, just make sure this branch is checked out if you run any tests)
Testing
Automated HW/SW integration test procedure written. Unit tests add support for the debug/assert macros.
Other
Leave any additional notes here
Reviewer Checklist
Standards
Error Handling
Memory
Performance