feat(BLE): WSF-NVM Storage Full Error Handling #975
Conversation
|
/clang-format-run |
|
|
||
| /*If the entry is valid copy it into the defragmentation buffer, if its reserved we can defrag it*/ | ||
| if (header.id != WSF_NVM_RESERVED_FILECODE) { | ||
| PalFlashRead(©Buf[copyOffset], fileSize, currentOffset); |
There was a problem hiding this comment.
I am getting a HardFault on the memcpy call inside this call.
I have defined PAL_NVM_SIZE to be 0x2000. I also fixed the bug in this comment.
I filled up the DB, and now when this code executes, wsfNvmCb.availAddr's value is 0x2010, which is past the bounds of the DB. So the header actually has invalid data. header.len is 0x2000242c, which ultimately causes the HardFault in memcpy.
There was a problem hiding this comment.
Can you see where the address gets set to the out of bounds state? Once again the wsfHaveEnoughSpace function is intended to block against this condition. If it isn't, I need to fix that. A workaround that I don't like is just getting the min between the available address and the total size
There was a problem hiding this comment.
It's being set in WsfNvmInit on a subsequent boot, after the failed write due to insufficient space.
| @@ -232,7 +287,11 @@ bool_t WsfNvmWriteData(uint64_t id, const uint8_t *pData, uint16_t len, WsfNvmCo | |||
| } | |||
|
|
|||
| WSF_ASSERT(!((id == WSF_NVM_RESERVED_FILECODE) || (id == WSF_NVM_UNUSED_FILECODE))); | |||
| WSF_ASSERT((wsfNvmCb.availAddr - WSF_NVM_START_ADDR) <= wsfNvmCb.totalSize); | |||
|
|
|||
| if (!wsfNvmHaveEnoughSpace(len)) { | |||
There was a problem hiding this comment.
Should this be checking for wsfNvmHaveEnoughSpace(len+sizeof(header))? I am still seeing a HardFault, even with the additional fixes from today (which look great to me). I'm trying to figure out if it's a problem with my test setup or not. Noticed this while I was investigating.
| @@ -63,6 +65,9 @@ typedef struct { | |||
| uint32_t dataCrc; /*!< CRC of subsequent data. */ | |||
| } WsfNvmHeader_t; | |||
|
|
|||
| #define WSF_NVM_HEADER_SIZE sizeof(WsfNvmHeader_t) | |||
| #define WSF_NVM_FILE_SIZE(header_len) (WSF_NVM_HEADER_SIZE + (header_len)) | |||
There was a problem hiding this comment.
I think the cause of my HardFault is that the offset for reading chunks while defragging is calculated using this, without an alignment bump, whereas WsfNvmWriteData is writing files witha n alignment adjustment. See
When I made this change, I was able to get past that HardFault.
Presumably this macro should be doing the same thing (and maybe the write function should also use this macro?
There was a problem hiding this comment.
I forgot to add, I also had to change this line
to account for that alignment offset as well, in order to fix the HardFault
|
Hardfaults are gone and non word aligned de fragmentation is fixed |
There was a problem hiding this comment.
@sullivanmj if you can run a final test and give us confirmation your issue and findings have been resolved we'll merge it
@EricB-ADI in the meantime a very minor macro request below
| bool_t WsfNvmDefragment(uint8_t *copyBuf, uint32_t size) | ||
| { | ||
| #if WSF_ASSERT_ENABLED == 1 | ||
| WSF_ASSERT(copyBuf && size >= wsfNvmCb.totalSize); |
There was a problem hiding this comment.
In general I would suggest checking for WSF_ASSERT_ENABLED inside the WSF_ASSERT macro. It cleans up the code and is much easier to maintain
There was a problem hiding this comment.
@Jake-Carter Yes, with the changes provided by @EricB-ADI, the original issue can now be resolved on my end by detecting that the writing of pairing keys failed due to insufficient space, defragging, and then re-attempting to store the keys, which succeeds.
Thanks!
|
/clang-format-run |
This reverts commit 1644294.
Description