Hey Gaurav. Are you sure this statement is accurate? I just changed one of my tasks back to “unprivileged” and it now works with the call to xPortIsInsideInterrupt.
The following is from the ARM Architecture Reference Manual -
As you can see unprivileged access is RAZ (Read As Zero), you will always get zero i.e. Thread Mode, when you call it from an unprivileged task.
Thanks. Makes sense.
Using SCB->ICSR for ISR context detection has similar restrictions.
What about adding an isISR flag to the callback API if you still want to stick to the generic callback API (maybe using a default parameter = false for the task context) ?
Yes, that’s exactly what I was thinking when I said I could implement something outside the RTOS support. It’s similar to Gaurav’s suggestion. Even though his suggestion is more efficient in terms of code execution, it’s less readable or more confusing to another developer. I’d like to have one generic call for this and set a flag as you suggested, Hartmut, to distinguish between the contexts. It just would have been much better if this could have been done automatically with support from FreeRTOS.
The biggest problems with implementing this sort of thing inside FreeRTOS is first, that it adds execution cost to all code using the API, as even if you know the context, inside the API it will still need to check. A second problem is portability, in some ports, it isn’t easy to determine if you are in an ISR or not, so if the API was defined with a single API, then FreeRTOS couldn’t be used on such a processor.
The second part of the issue is that ISR in real-time programming (which is FreeRTOS core target) are supposed to be short. Sending “Debug Logs” inside an ISR isn’t in the scope of normal programming.
Using FromISR ending on all routines defined to be used inside an ISR, doesn’t, in my opinion, reduce readability, but actual helps make things clearer.
1 Like
Again, these logs are NOT being printed at the time of the ISR. The data containing the logs are being queued. If I cannot queue data, then what is the purpose of having a from ISR for queues? BTW, I can shut off logging or select certain items to be logged.
I only need it for ARM processors (one in particular). I believe FreeRTOS documentation already states that reporting context is not available on all platforms (buyer beware). Your point about adding to the load of the API is correct. I wouldn’t have to do this if it were supported by FreeRTOS. Hence my question…why?
I’m looking for consistency in the API. Therefore, readability of the code is only part of the answer.
Gaurav, one more question on this. If I’m running inside an unprivileged task and then pre-empted by an ISR, wouldn’t the ISR switch to privileged mode? If this is the case, it is fine if xPortIsInsideInterrupt reports in thread while inside an unprivileged task because that’s where I’d be anyway.
Just out of curiosity: This function can apparently be called from both ISR and application contexts, but what do you expect to be in the handle member of log_msg when this is called in an isr context? I don’t believe it could cause much harm (unless you later on reuse the member under false assumptions), but it looks a little fishy to obtain a task handle in an isr.
I’ve had the same thought. The task handle is quite meaningless coming from an ISR. I can put an ISR handle in there and be done with it. Good suggestion.
Ok, got confused by the term “Print” which normally implies to send out.
But again, it seems you are trying to do something a bit to complicated inside the ISR.
Perhaps if you had to label a version of the functions as “FromISR” you would notice how much you were doing inside an ISR.
Doing something of the complexity of sscanf or sprintf inside an ISR raises flags to me. Personally, I would have probably had the ISR just send a notification to a task or a Pended function with the message to be processed as a task, and get around all the problems.
One BIG question, is have you confirmed that the code for these functions IS ISR safe in your system? (Some will malloc buffers).
Small note, you don’t check for success in sending in the FromISR case, maybe not much you can do, but you may want to record it somehow.
Not sure I agree with your statement. It’s basically a copy operation.
malloc is not used for logging. The log message structure contains a 1024-byte buffer for writing the log into. The log message structure is declared as a local variable inside the logger function and is therefore on the stack. This makes it thread-safe. The buffer length is what I’m more nervous about as my stack is not that much larger. I have a reason for making it that large. The Queue function copies the contents of this structure into it’s own buffers. Yes, it’s a lot of copying, but it is thread/ISR safe. Also, there is mention of real-time operations inside an ISR. I’d rather not get into a lecture on this, but it depends on your definition of real-time or more precisely the circumstance in this case. In this particular instance, there is no precise timing involved nor hard deadline. The action by the modem has already taken place and this is just a resulting report back from it. In fact, I’m not sure why they (3rd party) placed this inside of an ISR at all, but I have no control over that.
Your reference to using an event to signal that this call back had occurred doesn’t help much in my opinion. How would I transfer the content of the data that is sent to this callback? This data is basically the log that I’m sending via the queue mechanism.
For some reason, I thought the xQueueSendFromISR function returned a void. Good catch!
Scanf and sprintf are NOT just “copy operations”, there is a LOT of code to parse the format string, and they often make significant use of the stack for temporaries.
Note, I didn’t say by your logging routines, but by sscanf and sprintf.
Also, if you log message structure contains a 1024-byte buffer, then every call to you log is going to involve copying OVER 1024 bytes from that buffer into the queue. That in itself is a major operation. Even if the processor/library is well optimized for this sort of copy, that is many hundreds of computer cycles just for that operation, hopefully you aren’t blocking any other important operations while this is happening.
I was suggesting sending the incoming message, which looks to be quite short. Since it is coming from a single source (the ISR) to a single task, a StreamBuffer or a MessageBuffer should be quite efficient, and only need to copy the length of the actual string received, not a fixed 1024 byte message. The processing can be done at an appropriate priority task allowing more urgent things (and especially ISRs) to complete while this is being done.
Now thinking about it, yes, you are right. You should be able to use it for your use case.
Excellent!! Thanks, Gaurav.
I realize snprintf outputs to a buffer. What that does, at least in theory, is eliminate dynamic allocation of space that might happen behind the interface, and makes the print string operation atomic.
If each piece of the format were processed in turn to try to eliminate allocation of a large buffer, that might be done by sending that piece on to be printed, breaking the “atomic” feature of the operation. Alternatively, extending the output buffer with additional mallocs runs the risk of fragmentation or running out of memory.
I have seen with printf some items get printed with unexpectedly long outputs. Those are pathological cases. Using snprintf and then sending the output buffer to a “print string” function saves you from worrying about the buffer overflow hazard.
I’ll try out your implementation!
Thanks!