cesanta / mongoose-os

Mongoose OS - an IoT Firmware Development Framework. Supported microcontrollers: ESP32, ESP8266, CC3220, CC3200, STM32F4, STM32L4, STM32F7. Amazon AWS IoT, Microsoft Azure, Google IoT Core integrated. Code in C or JavaScript.
https://mongoose-os.com
Other
2.51k stars 429 forks source link

Add mgos_invoke_cb() option to use xQueueSendToFront...() #537

Open Harvie opened 4 years ago

Harvie commented 4 years ago

I need to use relatively complex code in ESP32 GPIO ISR (to handle complex protocol in time). But it turned out, i can't just tag everything IRAM_ATTR... It panics and i don't know why.

What is working for me is using mgos_invoke_cb to switch from ISR context to main context, where i can use complex libraries... But i feel like it's slow and other events and timers can mess me up, so i can't handle the communicaton on time. Is there way around this?

mgos_invoke_cb() currently uses xQueueSendToBackFromISR(). What if we used xQueueSendToFrontFromISR() instead? That way i probably can get more priority over other stuff to be able to handle my communication on time... And get executed ASAP after return from ISR...

https://www.freertos.org/xQueueSendToFront.html

IRAM bool mgos_invoke_cb(mgos_cb_t cb, void *arg, bool from_isr) {
  struct mgos_event e = {.cb = cb, .arg = arg};
  if (from_isr) {
    BaseType_t should_yield = false;
    if (!xQueueSendToBackFromISR(s_main_queue, &e, &should_yield)) {
      return false;
    }
    YIELD_FROM_ISR(should_yield);
    return true;
  } else {
    return xQueueSendToBack(s_main_queue, &e, 10);
  }
}
rojer commented 4 years ago

we can add a variant that sends to front instead - mgos_invoke_cb_front, perhaps. feel free to send a PR.

Harvie commented 4 years ago

I can do the PR, but i am not FreeRTOS expert... Can you please confirm this will actualy work in the way i expect?

So my callback will get executed immediately instead of waiting for other jobs that might have been queued before that?

rojer commented 4 years ago

you can add elements to queue front, yes. it will not get executed immediately, but it will be executed as soon as previosu callback currently executing on the main task finished executing. how about you try it, see if it makes a difference for you and if it does, send a PR. i don't want to add something that is not used, so far i have not had the need to do it.

Harvie commented 4 years ago

Sure. I will have to first check what causes more trouble in my case. Events already being executed or the events that are queued but not executed yet.

BTW is there something else that you would reccomend to tackle my issue with having to do immediate yet complex communication in ISR? I was thinking about using ISR to create new RTOS task instead of mgos event. To handle the communication immediately (while not running in ISR context). That way the slow/blocking mgos event might get preempted and my code might run fast enough. But i am not sure about how fast/realtime this would be...

Harvie commented 4 years ago

Is there way i can check if there's something currently executing and how many events are waiting in mgos queue? So i can use it in ISR to check if that's the case?

rojer commented 4 years ago

creating a task is possible and might be warranted in your case. if you create it with priority higher than MGOS_TASK_PRIORITY (e.g. MGOS_TASK_PRIORITY+1), it will preempt mgos if you send a queue item to it. just be careful invoking mgos APIs from it, as mgos is not thread-safe. you'll need to use mgos_invoke_cb from your task to interact with mgos.

Harvie commented 4 years ago

mgos is not thread-safe. you'll need to use mgos_invoke_cb from your task to interact with mgos.

I probably only need the following calls to mgos. Is there some reason why it should cause problems?

mgos_uptime_micros(); mgos_ints_disable(); mgos_msleep(); and mgos_gpio_*

rojer commented 4 years ago

no, those should be fine.

Harvie commented 4 years ago

Maybe i can create high priority RTOS task which would run similar event queue/loop to what mgos uses, but i will only use it for realtime stuff. Maybe this idea might be even generalized and implemented directly in mongoose as some kind of "multiqueue" support.

rojer commented 4 years ago

Maybe i can create high priority RTOS task which would run similar event queue/loop to what mgos uses, but i will only use it for realtime stuff

yes, that's exactly what i'm suggesting. let's hold off generalizing for now.

Harvie commented 4 years ago

let's hold off generalizing for now.

Too baaad, i've already generalized the heck out of it :-) I've came up with relatively elegant and simple (~120 LoC) solution, which offers way to handle tasks with both extremely high and extremely low priority. (extremely medium priority is possible as well of course :-)

It looks like this from user perspective:

  //Configure some basic parameters
  pq_handle pqh; //PQH = Parallel Queue Handle
    pq_set_defaults(&pqh);
    pqh.idle_after_ms = 6000;
    pqh.idle_cb = idle_cb;
    pqh.idle_cb_arg = "IDLE ARG";
    pqh.prio = MGOS_TASK_PRIORITY+1;

  //Create new FreeRTOS task running new FreeRTOS queue based event loop very similar to mongoose os
  pq_start(&pqh);

  //Invoke callback same way the mgos_invoke_cb() does
  pq_invoke_cb(&pqh, pq_test_cb, NULL, (void *)"CB ARG", false, false);

Benefits when compared to original mongoose event loop:

Drawbacks:

Can i expect some will to adopt this code on the upstream side? Be it as module or integral part of os core...

Harvie commented 3 years ago

I would need to move few mongoose defines (eg. MGOS_TASK_PRIORITY) to header files so i don't have to redefine them in my parallel event loop module, to keep the code compatible with upstream changes. Would you mind looking at it? #554

Harvie commented 3 years ago

So here is the parallel event loop module i've been talking about:

https://github.com/Harvie/pq https://github.com/mongoose-os-libs/freertos/issues/3

Harvie commented 3 years ago

What do you think about that pq project? Would you accept PR that would integrate it to mongoose os to replace currently used event loop without breaking current API?