stm32duino / STM32FreeRTOS

Real Time Operating System implemented for STM32
312 stars 61 forks source link

STM32FreeRTOS breaks I2C #39

Closed andrey-gvrd closed 3 years ago

andrey-gvrd commented 3 years ago

Not a super detailed bug report, but just wanted to raise this issue.

Seems like if the FreeRTOS is included in the project (no FreeRTOS code is even in the binary), I2C or something lower-level than it breaks.

In particular, execution gets stuck in the first call to i2c_master_write() because HAL_GetTick() doesn’t get updated once inside the function, so:

delta = (HAL_GetTick() - tickstart) is stuck at 0.

Screenshot 2021-04-06 141235

I'm using platformio with Nucleo-64 G071 board. Simply removing stm32duino/STM32duino FreeRTOS@^10.2.1 from library dependencies in platformio.ini fixes the problem.

andrey-gvrd commented 3 years ago

Look like the problem is with the realloc() function that the STM32FreeRTOS uses.

Here's a simple program that crashes at realloc():

#include <Arduino.h>

void setup() {
  Serial.begin(115200);
  while (!Serial);
  Serial.println("Starting...");
  delay(500);

  int *p = (int *) malloc(1);
  p = (int *) realloc(p, 1);

  Serial.println("Ending...");
  for (;;);
}

void loop() {
}

By crashing I mean HAL_IncTick() isn't called anymore and Serial doesn't work.

Also I'm getting this warning: This wrapper was verified for newlib version 2.5.0; please ensure newlib's external requirements for malloc-family are unchanged!", the version of newlib that platformio provides is 3.1.

andrey-gvrd commented 3 years ago

Solved by uncommenting #define configMEMMANG_HEAP_NB 3 in FreeRTOSConfig_Default.h. Not sure why it is commented out by default?

fpistm commented 3 years ago

Hi, this is described by by default heap_useNewlib.c is used that's all because it is the best heap management to use as *alloc are used. I didn't saw that issue before I will not be able to check this soon. Anyway the heap_useNewlib.c have to be reviewed for newlib 3.x as it was updated, see: https://nadler.com/embedded/newlibAndFreeRTOS.html

Note that PIO is not supported here so maybe the config is not correct with pio.

andrey-gvrd commented 3 years ago

On the second look, these are the results that I'm getting:

#define configMEMMANG_HEAP_NB -1 kills Hal_IncTick() on malloc() and realloc() #define configMEMMANG_HEAP_NB 3 is fragile, hardfaults in some cases #define configMEMMANG_HEAP_NB 1, 2, 4 doesn't compile because:

.pio\libdeps\nucleo_g071rb\STM32duino FreeRTOS\src\../portable/MemMang/heap_1.c:61:17: error: variably modified 'ucHeap' at file scope
61 |  static uint8_t ucHeap[ configTOTAL_HEAP_SIZE ];

#define configMEMMANG_HEAP_NB 5 kills Hal_IncTick() some other way

andrey-gvrd commented 3 years ago

Ok, I was able to recreate the problem in the Arduino environment.

Simply adding #include <STM32FreeRTOS.h> to the VL53L3CX_Sat_HelloWorld example sketch hangs the program somewhere in sensor_vl53lx_sat.InitSensor(0x12);, presumably for the same reason as the OP.

Or even simpler, the same thing happens in the example of Wire -> master_writer. Adding #include <STM32FreeRTOS.h> breaks it, presumably after a call to realloc() is made inside Wire.requestFrom(2, 6);.

andrey-gvrd commented 3 years ago

Ran this test with 4 processors.

STM32F407 Discovery (Cortex-M4) — PASS STM32F103 Blue Pill (Cortex-M3) — PASS STM32F0308 Discovery (Cortex-M0) — FAIL STM32G071 Nucleo-64 (Cortex-M0+) — FAIL

All pass without #include <STM32FreeRTOS.h>.

#include <STM32FreeRTOS.h>

#define LED LED_BUILTIN

void setup() {
  pinMode(LED, OUTPUT);
}

void loop() {
  int *a = (int *) malloc(8);
  int *b = (int *) realloc(a, 16);
  free(b);

  digitalWrite(LED, !digitalRead(LED));

  delay(50);
}
Riffer commented 3 years ago

I must add another one - that may just be a problem of PIN interrupts, I suspect.

My current project uses IRMP library that depends on PIN interrupts to decode IR signals.

I wanted to apply RTOS tasks in favor of Arduino TaskScheduler (working very well, but is only a simple time slicer with no priority control at the end) in the future and included the library STM32FreeRTOS as lib dependency some weeks ago - to remember the idea.

Working on other parts of my project I did not test the IRMP decoding - because it just worked as expected before. Two days ago I tested the orchestration of all my routines and IRMP did not work any longer. Happy me, I was able to check different branches of my code back to the past and ensured my hardware was still ok.

The problem is - as soon as I include (no header files!) STM32FreeRTOS into the platformio configuration IRMP stops working. This is reproducible at any time.

andrey-gvrd commented 3 years ago

Update.

I thought that using configMEMMANG_HEAP_NB 1 – 5 means that malloc() and free() provided by the standard library get replaced by FreeRTOS's implementations, but they don't. Instead, pvPortMalloc() and pvPortFree() are added. So that had nothing to do with this.

The problem still remained with configMEMMANG_HEAP_NB -1 or using heap_useNewlib.c. It comes down to vTaskSuspendAll() / xTaskResumeAll() which contain taskENTER_CRITICAL() / taskEXIT_CRITICAL() which map to vPortEnterCritical() / vPortExitCritical():

port.c:

void vPortEnterCritical( void )
{
    portDISABLE_INTERRUPTS();
    uxCriticalNesting++;
    __asm volatile( "dsb" ::: "memory" );
    __asm volatile( "isb" );
}
/*-----------------------------------------------------------*/

void vPortExitCritical( void )
{
    configASSERT( uxCriticalNesting );
    uxCriticalNesting--;
    if( uxCriticalNesting == 0 )
    {
        portENABLE_INTERRUPTS();
    }
}

Since uxCriticalNesting is (intentionally) uninitialized (set to a large value), a call to vPortEnterCritical() followed by vPortExitCritical() doesn't re-enable the interrupts. Based on FAQ point 4:

If a FreeRTOS API function is called before the scheduler has been started then interrupts will deliberately be left disabled, and not re-enable again until the first task starts to execute. This is done to protect the system from crashes caused by interrupts attempting to use FreeRTOS API functions during system initialisation, before the scheduler has been started, and while the scheduler may be in an inconsistent state.

So yes, this code hanging up in the delay():

#include <Arduino.h>
#include <STM32FreeRTOS.h>

void setup() {
  volatile int *a = (int *) malloc(16);
  *a = 1;
  free((int *)a);

  delay(1);
}

void loop() {
}

Is things working as expected 🤔