Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

osTimerNew does not really support static memory allocation #48

Closed
flotter opened this issue Apr 11, 2021 · 3 comments
Closed

osTimerNew does not really support static memory allocation #48

flotter opened this issue Apr 11, 2021 · 3 comments

Comments

@flotter
Copy link

flotter commented Apr 11, 2021

Hi,

The osTimerNew requires pvPortMalloc/pvPortFree, even with only static allocation is selected.

I migrated to CMSIS OS v2 because of explicit support in all the APIs for using static only memory allocation (i.e. owner module allocated memory resources at compile time).

It seems the osTimer API does not so easily allow for this on FreeRTOS as the internal timer construct provides no place to save the function pointer and argument in the Timer Control Block.

My solution was to statically allocate the additional space after the Timer Control Block, and simply pass in a larger cb and size. However, this CMSIS OS v2 FreeRTOS implementation needs to do some magic.

My implementation is a quick and dirty one, but could this idea be useful for you?

So the idea is that the owner statically allocate additional space for the Timer Control Block before calling osTimerNew(...)

*MACRO*
PRIVATE U32 os_timer_cb_##iname[((sizeof(StaticTimer_t) + 3U) / 4U) * 4U + 8U];					
PRIVATE CONST osTimerAttr_t os_timer_attr_##iname =
{															
	#iname,															
	0U,															
	(&os_timer_cb_##iname),
	((sizeof(StaticTimer_t) + 3U) / 4U) * 4U + 8U
}

And then ...

diff --git a/common/rtos/freertos/cmsis-rtos/cmsis_os2.c b/common/rtos/freertos/cmsis-rtos/cmsis_os2.c
index 783191d1..7823ee2e 100755
--- a/common/rtos/freertos/cmsis-rtos/cmsis_os2.c
+++ b/common/rtos/freertos/cmsis-rtos/cmsis_os2.c
@@ -1090,15 +1090,25 @@ static void TimerCallback (TimerHandle_t hTimer) {
 osTimerId_t osTimerNew (osTimerFunc_t func, osTimerType_t type, void *argument, const osTimerAttr_t *attr) {
   const char *name;
   TimerHandle_t hTimer;
-  TimerCallback_t *callb;
+  TimerCallback_t *callb = NULL;
   UBaseType_t reload;
   int32_t mem;

   hTimer = NULL;

   if ((IRQ_Context() == 0U) && (func != NULL)) {
-    /* Allocate memory to store callback function and argument */
-    callb = pvPortMalloc (sizeof(TimerCallback_t));
+
+    #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
+      /* Allocate memory to store callback function and argument */
+      callb = pvPortMalloc (sizeof(TimerCallback_t));
+    #else
+      if (attr != NULL) {
+        if ((attr->cb_mem != NULL) && (attr->cb_size >= (((sizeof(StaticTimer_t) + (sizeof(void *) - 1)) / sizeof(void *)) * sizeof(void *) + sizeof(TimerCallback_t)))) {
+          uint8_t * callb_ptr = (uint8_t *)attr->cb_mem;
+          callb = (TimerCallback_t *)&callb_ptr[((sizeof(StaticTimer_t) + (sizeof(void *) - 1)) / sizeof(void *)) * sizeof(void *)];
+        }
+      }
+    #endif

     if (callb != NULL) {
       callb->func = func;
@@ -1149,10 +1159,12 @@ osTimerId_t osTimerNew (osTimerFunc_t func, osTimerType_t type, void *argument,
         }
       }

-      if ((hTimer == NULL) && (callb != NULL)) {
-        /* Failed to create a timer, release allocated resources */
-        vPortFree (callb);
-      }
+      #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
+        if ((hTimer == NULL) && (callb != NULL)) {
+          /* Failed to create a timer, release allocated resources */
+          vPortFree (callb);
+        }
+      #endif
     }
   }

@@ -1256,7 +1268,9 @@ osStatus_t osTimerDelete (osTimerId_t timer_id) {
   TimerHandle_t hTimer = (TimerHandle_t)timer_id;
   osStatus_t stat;
 #ifndef USE_FreeRTOS_HEAP_1
-  TimerCallback_t *callb;
+  #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
+    TimerCallback_t *callb;
+  #endif

   if (IRQ_Context() != 0U) {
     stat = osErrorISR;
@@ -1265,10 +1279,11 @@ osStatus_t osTimerDelete (osTimerId_t timer_id) {
     stat = osErrorParameter;
   }
   else {
-    callb = (TimerCallback_t *)pvTimerGetTimerID (hTimer);
-
     if (xTimerDelete (hTimer, 0) == pdPASS) {
-      vPortFree (callb);
+      #if (configSUPPORT_DYNAMIC_ALLOCATION == 1)
+        callb = (TimerCallback_t *)pvTimerGetTimerID (hTimer);
+        vPortFree (callb);
+      #endif
       stat = osOK;
     } else {
       stat = osErrorResource;
@VladimirUmek
Copy link
Collaborator

Hi,
many thanks for your proposal! It is a very good idea to enable support when only static allocation is required. User application needs to provide a bit larger chunk of memory and therefore typedef StaticTimer_t cannot be used directly but I would say that this might be only a minor drawback.

I'll see if we can find a clean solution.

@VladimirUmek
Copy link
Collaborator

VladimirUmek commented May 20, 2021

Hi,
please see the referenced commit. Static and dynamic memory allocation variants are supported, one can either:

  • provide all memory statically
  • provide only memory for control block and callback memory will be allocated from heap
  • all memory will be allocated from heap

@flotter
Copy link
Author

flotter commented May 27, 2021

Thank you so much! I will update my setup in the near future.

@flotter flotter closed this as completed May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants