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

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

Open
Harvie opened this issue Apr 21, 2020 · 14 comments
Open

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

Harvie opened this issue Apr 21, 2020 · 14 comments

Comments

@Harvie
Copy link

Harvie commented Apr 21, 2020

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
Copy link
Collaborator

rojer commented Apr 21, 2020

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

@Harvie
Copy link
Author

Harvie commented Apr 21, 2020

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
Copy link
Collaborator

rojer commented Apr 21, 2020

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
Copy link
Author

Harvie commented Apr 21, 2020

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
Copy link
Author

Harvie commented Apr 21, 2020

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
Copy link
Collaborator

rojer commented Apr 21, 2020

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
Copy link
Author

Harvie commented Apr 21, 2020

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
Copy link
Collaborator

rojer commented Apr 21, 2020

no, those should be fine.

@Harvie
Copy link
Author

Harvie commented Apr 21, 2020

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
Copy link
Collaborator

rojer commented Apr 21, 2020

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
Copy link
Author

Harvie commented Oct 3, 2020

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:

  • Can have multiple event loops with various priorities and yes, they will preempt each other even when callback is still running.
  • Each mongoose module can run its own event loop without blocking other modules with higher priority or being blocked by low-priority modules.
  • This is especialy useful when you have one queue per hardware resource, so every request to claim that resource is enqueued and asynchronously dispatched as soon as possible without blocking the main loop while waiting for that resource.
  • Can use invoke_cb() parameter to do xQueueSend "to FRONT" instead of "to BACK" for task with extreme priority to get even more priority than the other events in high-priority task (use this responsibly!)
  • If needed i can reimplement original mongoose event loop API using few single line #define macros. (features of original mongoose event loop are subset of this new event loop features)
  • Each queue can optionaly register IDLE callback with custom timeout. So when there was no event in the queue for defined time period, it will automaticaly run the idle callback to do some "housekeeping" tasks with extremely low priority.
  • Each callback has boolean return value. False means it should be enqueued/run again. True means it is done. In case of IDLE callback, the return value of true will suspend idling until next event.
  • Can possibly perform FreeRTOS task management (eg. pause one specific event loop and later restart it, etc...)
  • Currently the code is provided in form of mongoose module, which can be reused by other modules. but code can be easily integrated to mongoose core to work as both API for 3rd party modules and replacement for current mongoose os event loop.
  • Whole concept has been modeled on real-life scenarios that we runned into while developing several different modules
  • I am willing to provide the code once it will get sufficiently documented and battle-tested.

Drawbacks:

  • You have to store the queue handles somewhere, otherwise you will not know which queue you are sending events to. But for main mgos_invoke_cb you can have queue handle stored as global variable, so there would be no change in API.
  • You need to think about thread safety a bit, when using multiple tasks. But queues help with synchronization anyway if used properly... Eg.: use one event loop privately for your module and use main mongoose queue only for communication with other modules.
  • One has to make sure to feed watchdog when spending too much time in high priority tasks which preempt watchdog feeding in main event loop.
  • Currently there are no timers like mgos_set_timer(...) to generate events in individual queues, so legacy API has to be used yet...

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

@Harvie
Copy link
Author

Harvie commented Oct 30, 2020

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
Copy link
Author

Harvie commented Nov 9, 2020

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

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

@Harvie
Copy link
Author

Harvie commented Mar 18, 2021

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?

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