feat(CMSIS): Adding mallinfo function and reworking Cordio memory management. #1179
Conversation
github-actions
bot
added
MAX32655
Need to call this multiple times to allow us to calculate the size requirements before requesting the heap memory.
This is a significant API change. We need to call WsfHeapAlloc with the required size before calling WsfHeapGetFreeStartAddress. This will allow us to do dynamic memory allocation, instead of allocating a large block of memory on initialization.
| void LlInitRunTimeCfg(const LlRtCfg_t *pCfg) | ||
| { | ||
| WSF_ASSERT(pLctrRtCfg == NULL); | ||
| // WSF_ASSERT(pLctrRtCfg == NULL); | ||
| WSF_ASSERT(pCfg); |
There was a problem hiding this comment.
Is the intention to entirely remove this commented line?
It makes sense to me that pLctrRtCfg may possibly be empty in this context -- given that it is not read and later assigned to pCfg.
There was a problem hiding this comment.
Yes we need to remove this check. The problem is that we will get an assertion if this function is called twice. We need to call this twice to calculate the length of dynamic memory needed.
| void BbInitRunTimeCfg(const BbRtCfg_t *pCfg) | ||
| { | ||
| WSF_ASSERT(pBbRtCfg == NULL); | ||
| // WSF_ASSERT(pBbRtCfg == NULL); |
There was a problem hiding this comment.
Ditto. Similar observation for pBbRtCfg in BbInitRunTimeCfg().
|
@crsz20 I was just thinking, it would be good to add a LL function for the size calculation. We're copying a lot of code that could be consolidated. |
Which part do you think could be consolidated? In the |
…solved hard-fault error.
Lines 127 - 133 in the file below. We could add a function with the same parameters as LlInit() that returns the memory requirement. |
|
/clang-format-run |
| * \return Memory used. | ||
| */ | ||
| /*************************************************************************************************/ | ||
| uint32_t LlInitMemConfig(LlInitRtCfg_t *pCfg) |
There was a problem hiding this comment.
I decided to name this function MemConfig because I see it is doing two things:
- Setting run-time configurations
- Computing the amount of heap memory used
Though, perhaps I could have simply named it LlInitSetRtCfg(). I think this is preferable. Let me know what you think.
There was a problem hiding this comment.
I like LlInitSetRtCfg(). Take the guts of 322 to 334 and replace those lines with a call to LlInitSetRtCfg().
There was a problem hiding this comment.
Observation
I noticed in LlInit() that the following lines are performed after calling both LlInitSetBbRtCfg() and LlInitSetLlRtCfg().
pCfg->pFreeMem += memUsed;
pCfg->freeMemAvail -= memUsed;In the updated examples, on the other hand, these are performed only once after LlInitSetBbRtCfg() and omitted after LlInitSetLlRtCfg(). Such as, here.
Conclusion
I am willing to bet that the examples needed that additional increment/decrement of memUsed. I will go ahead and add it to our new function LlInitSetRtCfg(). Is that correct?
There was a problem hiding this comment.
I don't think it's significant, but it can't hurt. I'm pretty sure that llCfg structure is only used to pass the initialization paramters. I don't think it's used once it returns from the init function.
|
Found a bug in the in BLE4_ctr example on ME17 that seems to date back to the following commit:
The BLE_datc and some other examples seem to run fine, however. I back-traced the error to I am not sure how this may be dependent on the recent changes to the heap memory management. |
|
Yes it sounds unrelated. I'm not sure why it would only fail on this example. That same procedure is done in most of the examples to get the Bluetooth address. Can you step through the procedure to see how it's failing? We've seen this before on pre-production parts where the USN isn't programmed properly. This would be a chip specific error that we would always see, not a software error. |
|
Seems to be failing at the checksum verification. |
This is a common problem with the pre-production parts. Please try a different board to see if we have the same problem. |
Description
Adding mallinfo function to give us information on how much heap space is being used and how much is available. Reworking the Cordio memory management so we don't have to request an arbitrarily large block of memory on initialization.
Checklist Before Requesting Review