Use of tinyfsm may need to care about FreeRTOS tasks
hogthrob opened this issue · 8 comments
As part of #88 I looked a bit into the usage of TinyFSM in the code. TinyFSM by itself does not care about multitasking / multicore execution. We are calling send_event from main task and I2STask. send_event will execute the respective event reat code which includes state transitions and also running entry and exit functions in the context of two different tasks. The state´s run function always executes on the main task loop.
If task switch would always happen when outside code run on behalf of the StateMachine this would not be a problem. But as far as I understand how Arduino on the ESP32 runs, this is not guaranteed.
Not yet sure if there is currently a real impact of this as the code runs pretty stable but this kind of problem is very difficult to track down when it appears...
One way to solve this issue would be to make sure that all TinyFSM code is executed in the main task by not directly using send_event in its original form in the I2STask. Would need to send an event to the main loop which in turn then calls send_event when it gets the control back.
Hi there,
Interesting point indeed.
But as far as I understand how Arduino on the ESP32 runs, this is not guaranteed.
I have never checked into the matter and have never encountered issues with this. Maybe a switch of core is only done in some specific situations? I am currently working on some other projects so it is difficult to free up some time for this.
When I am picking up the local hotword and ui colorpicker for state colors again, I will have a look.
In the mean time if you are eager to solve this, please do, your work this far very much appreciated.
I am especially keen on getting the local hotword working, which will boost a good usage for the esp32-s3 in my personal situation :)
I looked a bit more into this issue and it is more serious than I thought.
There are at least 5 task which at some point may execute fsm code
- Main Task, sends events to FSM
- AsyncWebServer, Task runs web front end, changes device object and other config objects, saves config to NVM
- AsyncMqttClient Task, receives Rhasspy MQTT messages and sends events to FSM
- WifEvent Task, receives Wifi events, sends events to FSM
- I2S Task, receives MQTT Rhasspy MQTT messages, sends events to FSM
- IndicatorLight Task (if used by device), does not send events or interacts in any other way directly with the FSM
In addition there are a couple of places where same code could be potentially executed at the same time by two different tasks (save_configuration from WebServer and from AsyncMqttClient is an example).
This is not only a theoretical problem, the moment I replaced the WifiClient with the WifiClientSecure to stream the audio encrypted, hell broke loose and system was crashing. Analysis turned out that the WifiEvent code executed FSM code and also the main task tried the same and that crashed the SSL library and/or the TCPIP stack.
To cut a long story short, I changed send_event to queue events which are processed then exclusively by the main task.
Seems to work so far. Audio playback is just fine, audio recording works too.
Drawback: Lots of (rather small and simple) changes in many places. And of course some of the things may now happen in different order due to the event queuing.
I also changed the message handling of the onMqttMessage of the AsyncMqttClient, this is not directly related to the problem but gave me a better idea of what is going on.
See hogthrob@20f0bb0 to get an idea what was done.
Please note the code linked above is not "final" but work in progress.
You've busy, thanks for the good work!
I do not have a lot of coding experience in the areas you are working on, but I will study them to gain some.
The code is already way more feature-rich when I started it. Great work.
Worked a bit more and are now able to run upstream audio (microphone samples) to Rhasspy using encrypted MQTT communication via TLS. See branch here: https://github.com/hogthrob/ESP32-Rhasspy-Satellite/tree/fix89
Also small other changes (hopefully improvements), see log messages. Still work in progress, would be good if someone could tests this on their devices. It should behave (save for the small changes) identically to the master version.
I'll give it a try in my Matrix Voice and M5 Atom Echo
Maybe you can do a PR to my development branch? Otherwist I'll clone your branch
Works with the Matrix Voice, but my audio fixes are not in that branch so sound is crap.
I tried a couple of hotword - intent revolutions without issues.
Hi @hogthrob, are you still working on this at times?
If not I will close this issue