PxMatrix icon indicating copy to clipboard operation
PxMatrix copied to clipboard

code in the examples can crash on ESP32

Open txf- opened this issue 4 years ago • 8 comments

Using pxMatrix 1.8.2 on an ESP32, as part of a larger program.

void IRAM_ATTR display_updater(){
  // Increment the counter and set the time of ISR
  portENTER_CRITICAL_ISR(&timerMux);
  display.display(display_draw_time);
  portEXIT_CRITICAL_ISR(&timerMux);
}

I was building an application that used plenty of other sensors on different cores and thread. Effectively I was getting core crashing when this ISR callback was being called. Then I took a look at display.display() and saw that it was a reasonably large block of code being called from an ISR.

surely doing something like this would be safer?

#define CORE_1 1
static TaskHandle_t displayUpdateTaskHandle = NULL;
hw_timer_t * timer = NULL;
portMUX_TYPE timerMux = portMUX_INITIALIZER_UNLOCKED;
TaskHandle_t displayUpdateTaskHandle;

void IRAM_ATTR display_updater()
{
  portENTER_CRITICAL_ISR(&timerMux);
  BaseType_t xHigherPriorityTaskWoken = pdFALSE;
  //notify task to unblock it 
  vTaskNotifyGiveFromISR(displayUpdateTaskHandle, &xHigherPriorityTaskWoken );

  //display task will be unblocked
  if(xHigherPriorityTaskWoken){
    //force context switch
    portYIELD_FROM_ISR( );
  }
  portEXIT_CRITICAL_ISR(&timerMux);
}
 
void displayUpdateTask( )
{
  for(;;){
    //block here untill timer ISR unblocks task
    if (ulTaskNotifyTake( pdTRUE, portMAX_DELAY ){
        display.display();
    }
  }
}

void setup()
{
  Serial.begin(112500);
  display.begin(16);
  /* we create a new task here */
  xTaskCreatePinnedToCore(
    displayUpdateTask, /* where display() actually runs. */
    "displayUpdateTask", /* name of task. */
    2048, /* Stack size of task */
    NULL, /* parameter of the task */
    3, /* Highest priority so it is immediately launched on context switch after the ISR */
    &displayUpdateTaskHandle, /* Task handle to use for task notification */
    CORE_1);

  timer = timerBegin(0, 80, true);
  timerAttachInterrupt(timer, &display_updater, true);
  timerAlarmWrite(timer, 2000, true);
  timerAlarmEnable(timer);
}

The examples run fine on their own, problems may arise when others may try to use them as the basis for a more complex program.

Edit:added taskhandle declaration. Edit2: removed extraneous bracket in display_updater()

txf- avatar Jun 20 '20 22:06 txf-