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

code in the examples can crash on ESP32 #225

Open
txf- opened this issue Jun 20, 2020 · 8 comments
Open

code in the examples can crash on ESP32 #225

txf- opened this issue Jun 20, 2020 · 8 comments

Comments

@txf-
Copy link

txf- commented Jun 20, 2020

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()

@hallard
Copy link

hallard commented Aug 9, 2020

Hi,

Tried this one but looks like the code does not compile due to missing displayUpdateTaskHandle, not sure of what I need to do to get this working. would you mind help me to try ?

ok, self answer, declaration of displayUpdateTask should be as follow

void displayUpdateTask(void *)

@txf-
Copy link
Author

txf- commented Aug 9, 2020

Hi,

Tried this one but looks like the code does not compile due to missing displayUpdateTaskHandle, not sure of what I need to do to get this working. would you mind help me to try ?

ok, self answer, declaration of displayUpdateTask should be as follow

void displayUpdateTask(void *)

Whoops, sorry about that. You have to declare a task handle.

TaskHandle_t displayUpdateTaskHandle;

I updated my example above.

@hallard
Copy link

hallard commented Aug 9, 2020

sorry, it was not the source of the error, you declared it twice now, it was related to declaration of displayUpdateTask()

Working code is here (was also missing a parenthesis in if condition of if (ulTaskNotifyTake)

fixed code below, thanks for sharing

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

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(void *)
{
  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);
}

@txf-
Copy link
Author

txf- commented Aug 9, 2020

Sorry about that, that's what I get from trying write code from my phone

@dvmourik
Copy link

dvmourik commented Apr 11, 2021

I'am not getting this to work... i have problems with the old way of the ISR and display.display...
so i thought i give your code a shot it looks good.

For me it looks like it's not getting past the timer part in the setup.
Can you guide me the direction how to debug this?

Owww BTW

void IRAM_ATTR display_updater(){
{

There is a { too much there.

@txf-
Copy link
Author

txf- commented Apr 12, 2021

I'am not getting this to work... i have problems with the old way of the ISR and display.display...
so i thought i give your code a shot it looks good.

For me it looks like it's not getting past the timer part in the setup.
Can you guide me the direction how to debug this?

Owww BTW

void IRAM_ATTR display_updater(){
{

There is a { too much there.

Good catch, I edited my example

@dvmourik79
Copy link

I hope you can help me with this problem...

Sadly using this code gives me a panic much quicker than before.
I use a 128x64 screen and some buttons. When i press a button now it panic's directly

Guru Meditation Error: Core 1 panic'ed (LoadProhibited). Exception was unhandled.

When i do it the old fashioned way i can handle buttons but not other things like a wifimanager.

It does crash always at the display.display part.
Looks like the ESP is too busy keeping the screen alive and can't take other jobs next to it...

Hardware used:
4MB Lolin32
128x64 P2.5 screen with FM6124 chipset

ESP Decoding results:

PC: 0x4008c044: vTaskNotifyGiveFromISR at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/tasks.c line 5018
EXCVADDR: 0x00303188

Decoding stack results
0x4008c044: vTaskNotifyGiveFromISR at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/freertos/tasks.c line 5018
0x400810c0: display_updater() at C:\Code\Beta1.0.0/Beta1.0.0.ino line 176
0x40081429: __timerISR at ArduinoData\packages\esp32\hardware\esp32\1.0.6\cores\esp32\esp32-hal-timer.c line 88
0x400d1c9b: PxMATRIX::fillMatrixBuffer(short, short, unsigned char, unsigned char, unsigned char, bool) at C:\Users\danie\Documents\Arduino\libraries\PxMatrix_LED_MATRIX_library/PxMatrix.h line 791
0x400d1d6e: PxMATRIX::drawPixelRGB565(short, short, unsigned short) at C:\Users\danie\Documents\Arduino\libraries\PxMatrix_LED_MATRIX_library/PxMatrix.h line 817

@txf-
Copy link
Author

txf- commented May 13, 2021

I've not had issues with button interrupts and the code above. Maybe the code doesn't handle being interrupted? You could also play around in PxMatrix.h and change the SPI clock rates, Your screen is much bigger than the one I was using, thus takes much longer to fill.

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

4 participants