Hello All, the following code is generating a HardFault. I'm sure I've missed something stupid:
void Thread_Debug2(void const *argument) { // locals Debug_Struct * pDbgStrct = 0; osStatus Status = osOK; uint32_t Placements2 = 0; // Start while(1) { // disable irq __disable_irq(); // get message, here the hardfault occurs pDbgStrct = (Debug_Struct *)osMessageGet(MsgQueue_Debug1, 0).value.v; // enable irq __enable_irq(); // free memory Status = osPoolFree(MemPool_Debug1, pDbgStrct); // check status if(Status == osOK) { Placements2++; } } }
In my opinion, the osMessageGet should be able to get a message from a message queue even when no interrupt is activated...
Hello Again,
Thanks to all of your replies. I've updated the thread/function:
void Thread_Debug2(void const *argument) { // locals Debug_Struct * pDbgStrct = 0; osEvent evt; uint32_t i = 0; volatile uint32_t Dummy = 0; // Start while(1) { // check for message evt = osMessageGet(MsgQueue_Debug1, 0); // message received? if(evt.status == osEventMessage) { // message received, get data pDbgStrct = (Debug_Struct *)evt.value.p; // do something with the data Dummy = pDbgStrct->Temp1 + pDbgStrct->Temp2 + pDbgStrct->Temp3; // free the memory block osPoolFree(MemPool_Debug1, pDbgStrct); } // do some other things (no os delay) for(i = 0; i < 123456;i++) { __NOP(); } } }
You might noticed, that I've posted another Question the other day about a mailbox overflow
http://www.keil.com/forum/60328
This is my main problem so I've started to search otherwise and found the problem above. I thought, that this might be some reason of my problem, but no it isn't...
Has anyone a Idea what can go wrong when receiving messages like above? Down here is the function where I put the messages:
void Thread_Debug1(void const *argument) { // locals Debug_Struct * pDbgStrct = 0; osStatus Status = osOK; uint32_t i = 0; // Start while(1) { // allocate memory pDbgStrct = (Debug_Struct *)osPoolCAlloc(MemPool_Debug1); // check for memory block if(pDbgStrct != 0) { // fill structure pDbgStrct->Temp1 = 0xAB; pDbgStrct->Temp2 = 0xCDEF; pDbgStrct->Temp3 = 0x01234567; // put it in queue Status = osMessagePut(MsgQueue_Debug1, (uint32_t)pDbgStrct, 0); // check status if(Status == osOK) { // message successful placed } else { // message not placed in queue, free memory osPoolFree(MemPool_Debug1, pDbgStrct); } } // do some other things (no os delay) for(i = 0; i < 123456;i++) { __NOP(); } } }
If you are looking for problems with the message queue, then you really should make sure you fully verify return codes.
The function can return:
osOK: no message is available in the queue and no timeout was specified. osEventTimeout: no message has arrived during the given timeout period. osEventMessage: message received, value.p contains the pointer to message. osErrorParameter: a parameter is invalid or outside of a permitted range.
You only check if "osEventMessage" or "not osEventMessage". Correct would be to check for "osOK" or "osEventMessage" - the two expected in your design - and notify you with some error message if you get anything else. Your code will not tell you anything if you get "osErrorParameter".
Next thing - in what way do you hope to test your code by having polled retrieval, and then do:
// do some other things (no os delay) i = 0; i < 123456;i++) { __NOP(); }
The above sequence is a busy-loop. And it will starve any thread with lower priority. It isn't even nice with other threads of same priority.
Notice that both your sender and your receiver wants to waste all the CPU capacity they can lay their hands on in your busy loops. That's not cooperative programming. Both threads are directly hostile to anything but themselves.
The goal with threads, is to have each thread perform as little work as possible before flagging to the OS that the processor capacity should be donated to something else. So a thread should only consume CPU if it has actual work that needs to be done. And it should perform this work at a priority that is reasonable compared to all other work your program may need to perform.
You can get lots of problems with your message queues if you don't write your program in a way that all consumers are given a chance to retrieve data at least as fast as the producers are able to insert data.
If you really do want to debug, you can make use of a separate counter where the producer increments the counter when it has added a message - and the consumer(s) decrements the counter when having retrieved a message. Possibly yet another counter for the allocation/deallocation of the actual message data. This obviously requires that the counters are volatile - and might require the use of critical sections since few processors has atomic increment/decrement of memory variables. But any code just has to verify the return codes of all function codes and appropriate "else" sections if you get a "bad" status code. You can't just code for "happy path". Code for the worst, but hope for the best.
Your code has no way to report if you fail to allocate a message etc.
One of the first design decisions when starting a project should be to decide how your program can inform you about deviations from "happy path". You can spend huge amounts of time debugging strange problems, if you haven't engineered in error detection and reporting in your program. Programming is both engineering and creativity. While the creative part might be fluffy and abstract, the engineering part is rule-based and a required rule is to care about errors. Best practices are called best practices because "best" is very good. And it's very good to do what helps you and at the same time helps your customers. Short-cuts saving your a couple of lines of code are seldom ending up being time savers.