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

LUA Widgets: background() not called if menu is active #4279

Closed
1 task done
mha1 opened this issue Nov 3, 2023 · 14 comments · Fixed by #4393
Closed
1 task done

LUA Widgets: background() not called if menu is active #4279

mha1 opened this issue Nov 3, 2023 · 14 comments · Fixed by #4393
Labels
bug 🪲 Something isn't working

Comments

@mha1
Copy link
Contributor

mha1 commented Nov 3, 2023

Is there an existing issue for this problem?

  • I have searched the existing issues

What part of EdgeTX is the focus of this bug?

Transmitter firmware

Current Behavior

LUA widgets can be updated even if they are currently not on screen, e.g. by having a different main view on screen. 2.7 also called the widget's background() function in case a menu like model settings was selected. 2.8, 2.9 and current main don't call background() if a menu is selected. This causes problems for some LUA scripts relying on continuous foreground and background processing.

Expected Behavior

As in 2.7 LUA needs to call the LUA widget's background() function in all cases where the widget is not on screen which is also the case for active menus.

Steps To Reproduce

  1. use 2.9 firmware simulator and set up the attached test widget bckgrnd.zip
  2. observe debug output "in refresh()" if the widget is visible
  3. switch to a different main view and observe debug output "in background()"
  4. enter model settings menu (or any other menu) and observe no debug outout
  5. repeat steps 1..5 using 2.7 firmware simulator and observe debug output "in background()" for step 4 too

Version

2.9.1

Transmitter

Radiomaster TX16S / TX16SMK2

Operating System (OS)

No response

OS Version

No response

Anything else?

@philmoz I think you might be able to help. The LUA background() function is invoked by void LuaWidget::checkEvents() which is called if the widget is visible or not visible and in a different main view but not called if a menu is active.

@mha1 mha1 added bug 🪲 Something isn't working triage Bug report awaiting review / sorting labels Nov 3, 2023
@philmoz
Copy link
Collaborator

philmoz commented Nov 4, 2023

I'm not sure there is an easy way to do this.

The menus (system, model and screen setup) take over and everything on the main view stops.
Nothing on the main view will get updated or have the checkEvents function called until the menu is closed.

@mha1
Copy link
Contributor Author

mha1 commented Nov 4, 2023

It worked in 2.7. Was that a deliberate change in 2.8?

@philmoz
Copy link
Collaborator

philmoz commented Nov 4, 2023

2.7 did not use LVGL.

@mha1
Copy link
Contributor Author

mha1 commented Nov 4, 2023

2.7 also called LUA widget background processing through luawidget::checkEvents() which hasn't changed since. Where was luawidget::checkEvents() called from in 2.7?

@philmoz
Copy link
Collaborator

philmoz commented Nov 4, 2023

2.7 also called LUA widget background processing through luawidget::checkEvents() which hasn't changed since. Where was luawidget::checkEvents() called from in 2.7?

That was before my time. I guess that opening a menu did not block the underlying views and they continued to run.

@mha1
Copy link
Contributor Author

mha1 commented Nov 4, 2023

We need to call luawidget::background() directly if the menus are running. At what rate is checkEvents() called if the main view is active?

@philmoz
Copy link
Collaborator

philmoz commented Nov 4, 2023

We need to call luawidget::background() directly if the menus are running. At what rate is checkEvents() called if the main view is active?

That's potentially difficult (have to iterate all screens and all widgets, including top bar, on all screens).
It's could also have unintended, and difficult to diagnose, side effects.

In my opinion the current behaviour is correct.

@pfeerick
Copy link
Member

pfeerick commented Nov 4, 2023

I have a niggling feeling this was deliberate, but can immediately place a finger on the issue/PR this was discussed in. IIRC it was to do with slowdowns in the menus, due to widget background behaviour? I get the desire to have a widget working in the background all the time, but with the current architecture, I can't see that ending well in the long term.

@pfeerick pfeerick removed the triage Bug report awaiting review / sorting label Nov 4, 2023
@JimB40
Copy link
Collaborator

JimB40 commented Nov 4, 2023

Well AFAIR there was discussion once about that with 2.8
Thing is that background is not background anymore. So giving simple example when background function reads voltage sesnsor and plays warning if below certain treshold it's no more reliable. TBH I can't find practical use case for background functon if it is only called when displaying other main view.

@philmoz
Copy link
Collaborator

philmoz commented Nov 4, 2023

TBH I can't find practical use case for background functon if it is only called when displaying other main view.

I have the opposite problem - I struggle to think of a scenario where this actually matters.

I mean, how much time would/should you actually spend looking at a setup screen when a model is connected and active?

@mha1
Copy link
Contributor Author

mha1 commented Nov 4, 2023

IIRC it was to do with slowdowns in the menus, due to widget background behaviour?

If this is a concern we have a problem. Function scripts run continuously in the background, hence also while menus are active. So I think this can't be a point against running LUA widget code in the background.

I get the desire to have a widget working in the background all the time, but with the current architecture, I can't see that ending well in the long term.

It's not about running a widget in the background all the time. It's about allowing widgets to continue processing (like continuing to integrate a parameter) even if not on display. This is why I agree with Robert. Starting with 2.8 LUA widgets don't work like advertised. The background processing feature of LUA widgets got mutilated with 2.8.

Architectural concerns are valid as in does calling checkEvents() while menus are active fit into the menus concept. As I understood, no.

Idea: If there was a way to detect if menus are active we could add calling luawidget::background() outside the menus code to cover the other half of background processing.

@robustini
Copy link
Contributor

robustini commented Nov 6, 2023

@mha1 100% agree.
In my case I use Yaapu very often, and having voice alerts in the background for me is imperative, in any case.
I remember being one of those who raised the issue that initially with 2.8 noticing that Yaapu was no longer working in the background with LVGL.
I honestly don't know how it can be fixed but I agree, lua scripts should continue to work in the background in any case, except of course in cases like "writing firmware" and things like that, provided of course that they do not affect the control features at the model (switches and sticks).
But to safeguard the execution of basic functionality there should be a watchdog, if lua scripts overload the system something should slow them down.

@philmoz
Copy link
Collaborator

philmoz commented Dec 9, 2023

PR #4393 adds logic to call the widget background functions when in the setup pages.

It needs extensive testing before being considered for merging into main.

@mha1
Copy link
Contributor Author

mha1 commented Dec 9, 2023

However it needs extensive testing on a variety of radios especially with complex widget setups (which I don't use).

@philmoz Hi Phil, I hacked a little widget script iteratively calculating Pi using the Nilakantha series in the widget's background function. With the number of iterations set by the widget's settings you can adjust the load background() will put on the system. Once you cross a certain boundary of iterations (about 1450 on my NV14) the script will be killed due to hitting the allowed max CPU limit. This happens as expected with no side effects on the main system just like it should. Being close to the CPU limit (1425 iterations) the GUI is still responsive with no major lag or other side effects. I think you PR works fine.

To use: unzip bckgrnd.zip and put the folder backgrnd into the WIDGETS folder and assign it to a screen. You'll see a counter running (calculated in the foreground) and PI initially at 3.0. Once you enter a GUI menu, e.g. model setup, background() is called continuously and calculates Pi in every background() cycle with an accuracy according to the number of iterations ("Reps") in the widget's settings. Going back to the main screen the widget will show the approximated Pi. Doing this close to the kill point for CPU limit is the GUI's worst case for evaluation.

image

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants