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

feat(color): Run widget 'background' function while in setup pages. #4393

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Dec 9, 2023

Fixes #4279

All setup pages inherit from the Page or TabsGroup (via PageTab) classes. These block the main view so no main view updates are processed while in the setup pages.

This PR adds a function to call just the 'background' function for all active widgets when using the setup pages. It does not call any of the UI objects 'checkEvents' functions so the only overhead is the processing time each widget 'background' call takes. So long as the widgets are well behaved, and there is not an excessive number of them it should not significantly affect performance.

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

@pfeerick pfeerick added this to the 2.11 milestone Dec 9, 2023
@mha1
Copy link
Contributor

mha1 commented Dec 9, 2023

Thanks Phil. I built and ran your PR with a script that has some load in background(). Works fine.

@pfeerick
Copy link
Member

Please verify that your special functions are still enabled... i.e. with the default theme there will be a yellow filled square on the right side of the sf overview screen if a function is enabled.

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

Have lost all Special functions (notable by no switch calls except in global functions) and background still does not work on the X12S. Loaded latest 2.10 nightly and all returned back (still with the ongoing sound problem). Back to 2.9.2!!

I don't have a X12S but built Companion based on #4393. Running my test widget script (https://github.com/EdgeTX/edgetx/files/13624506/bckgrnd.zip - see #4279 (comment)) works fine. Background calculation results (approximating Pi) and debug output confirms this PR is working fine.

image

image

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

@Kevltan your script is wrong. try this: main.lua.txt (remove the .txt)

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

I don't understand what you're saying. Your main.lua doesn't work in the background because you don't call the part that is required to run in the background in the background function. I have changed your script, it should work now.

So try the version I uploaded. It's called main.lua.txt because github doesn't allow .lua files to be uploaded. Just click here https://github.com/EdgeTX/edgetx/files/13637615/main.lua.txt, download the file, then rename it from main.lua.txt to main.lua

Then copy main.lua to your widget script folder (WIDGETS/TD-RDT)

@Kevltan
Copy link

Kevltan commented Dec 11, 2023

Ok thanks for that 😊👍Will try that🤞. Have looked at the change you made can see where I went wrong!!

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

how does it fail?

@Kevltan
Copy link

Kevltan commented Dec 11, 2023

My bad forgot about the *.text🤷‍♂️🤷‍♂️
Will load 2.10 PR#4395 and test👍

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

I hope you are aware to flash this PR: https://github.com/EdgeTX/edgetx/suites/18891839239/artifacts/1102977052

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

Do you have a receiver connected? FCAL will remain red if there is no live telemetry. Works fine on my NV14 FCAL is updating but red as long as there is no receiver connected

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

yes it runs.

ok, strange. working different on my NV14.

@mha1
Copy link
Contributor

mha1 commented Dec 11, 2023

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

@Kevltan
Copy link

Kevltan commented Dec 11, 2023 via email

mha1 added a commit to mha1/edgetx that referenced this pull request Dec 13, 2023
@robustini
Copy link
Contributor

robustini commented Dec 16, 2023

Tested on Yaapu in background which generates 8 virtual sensors with Ardupilot and ELRS, all continue to work just fine in setup=>telemetry screen, without this PR they would obviously stop working.
And the feeling is that everything is more streamlined, faster even when browsing through the various pages of the widget, i've 5 telemetry screen.
So TOP pull request, thanks!

Senza titolo

@Kevltan
Copy link

Kevltan commented Dec 17, 2023

Mine works fine too in both 2.10 and 2.9.2 builds using 9 sensors one of which is generated from a widget calculation which is then put into a telemetry field for reading in log files. 2.9.2 was built for me by Michael for testing at my own risk. FCAL is the widget generated telemetry field.

screen-2023-12-17-142829

@robustini
Copy link
Contributor

robustini commented Dec 19, 2023

@philmoz I tried recompiling the firmware today from the main branch with your PR but it generates an annoying screen flicker on my X10S Express, could you check if it does this to you too?
On December 16 it was ok.

@philmoz
Copy link
Collaborator Author

philmoz commented Dec 19, 2023

I tried recompiling the firmware today from the main branch with your PR but it generates an annoying screen flicker on my X10S Express, could you check if it does this to you too? On December 16 it was ok.

There's nothing in this PR that should affect screen display - can you post a video showing the issue.

@robustini
Copy link
Contributor

I tried recompiling the firmware today from the main branch with your PR but it generates an annoying screen flicker on my X10S Express, could you check if it does this to you too? On December 16 it was ok.

There's nothing in this PR that should affect screen display - can you post a video showing the issue.

I have not tried for now to recompile the firmware without your PR, so I do not rule out that it may also be in one of the latest commits the problem.

20231219_084815_0.mp4

@philmoz
Copy link
Collaborator Author

philmoz commented Dec 19, 2023

I have not tried for now to recompile the firmware without your PR, so I do not rule out that it may also be in one of the latest commits the problem.

I can't think of any reason why this PR would cause that to happen.

@robustini
Copy link
Contributor

I have not tried for now to recompile the firmware without your PR, so I do not rule out that it may also be in one of the latest commits the problem.

I can't think of any reason why this PR would cause that to happen.

Yes, the problem is elsewhere, i'm sorry, I report that.

@philmoz philmoz force-pushed the run-widget--background branch from 6631745 to bdbf8b0 Compare March 2, 2024 23:53
@ParkerEde
Copy link
Contributor

This pr works great on my tx16s and three different lua widgets in the background

@philmoz philmoz force-pushed the run-widget--background branch from bdbf8b0 to 71648d4 Compare March 8, 2024 09:06
@ParkerEde
Copy link
Contributor

It would be great if this pr came with 2.10. Is there anything against it?

@pfeerick
Copy link
Member

pfeerick commented Mar 9, 2024

I have no views either way... this was simply not slated as an intended feature for 2.10 right from the start.

Given users like @robustini have been using with Lua widgets such as Yaapu it since it was released @Kevltan with builds of 2.9 is reasonable to consider it, since it can be scrapped in 2.10.1 if something goes wrong. So I see reports of X10, X12 and TX16S? Yappu, TD-RDT... any other widgets like iNav Lua?

@pfeerick pfeerick modified the milestones: 2.11, 2.10 Mar 9, 2024
@Kevltan
Copy link

Kevltan commented Mar 9, 2024 via email

@pfeerick
Copy link
Member

pfeerick commented Mar 9, 2024 via email

@Kevltan
Copy link

Kevltan commented Mar 9, 2024

Nightly is coming up on my radio as 2.11.0 now !? I have version number come up at the bottom of the screen (I added version update in my widget). My widget is seeing 2.11. 0 since the new 2.10 RC1.

@3djc
Copy link
Collaborator

3djc commented Mar 9, 2024

Nightly is coming up on my radio as 2.11.0 now !? I have version number come up at the bottom of the screen (I added version update in my widget). My widget is seeing 2.11. 0 since the new 2.10 RC1.

Yes, 2.10 line as been created with RC1 and splitted from main. Thats the normal process

@Kevltan
Copy link

Kevltan commented Mar 9, 2024

So I am correct in saying and my radio widget is saying Nightly 2.11.0?

@3djc
Copy link
Collaborator

3djc commented Mar 9, 2024

Yes, correct. Current nightly are 2.11 as they include now features that will/might not be in 2.10, but 2.11

2.10 evolution is now made of 2.10 branch, not main anymore

@Kevltan
Copy link

Kevltan commented Mar 9, 2024

👍👍👍

@Kevltan
Copy link

Kevltan commented Mar 9, 2024

Don't you mean 2.10? 😂

I think I am right in saying that @philmoz must have pushed it over as branch off the latest 2.11.0 nightly. I built it off his latest force - pushed run-widget--background branch from bdbf8b0 to 71648d4 adding a few changes of my own to the firmware.

@pfeerick
Copy link
Member

pfeerick commented Mar 9, 2024

That wasn't the point I as making... why

Possibly 2.11.0 then?

when the discussion is that it might be included into 2.10 ;)

main is indeed reporting 2.11 now, and Phil did rebase this recently to bring it up to date with main. But for all intents and purposes, this PR is just 2.10 with these changes and a different version number at the present... there is no different between main and 2.10 branches... yet.

@gagarinlg
Copy link
Member

My 2 cents
When it has been running good for a couple of people for weeks now, we can add it to 2.10 RC2

@mha1
Copy link
Contributor

mha1 commented Mar 9, 2024

I made a LUA script some time ago to be able to test this under various load and overload conditions, see #4279. Works as expected. I am convinced this is 2.10 worthy.

@robustini
Copy link
Contributor

I made a LUA script some time ago to be able to test this under various load and overload conditions, see #4279. Works as expected. I am convinced this is 2.10 worthy.

Absolutely, also because for those who use LUA in the background it is essential.

@ParkerEde
Copy link
Contributor

great job. Thx to all of you

@pfeerick
Copy link
Member

I'm not see any issues either on TX16S w/ iNav, Yappu widgets, or Michael's load test widget... UI and radio were no less responsive or usable than normal... time to more eyes on on ;)

@pfeerick pfeerick merged commit a396eff into EdgeTX:main Mar 11, 2024
43 checks passed
@Kevltan
Copy link

Kevltan commented Mar 11, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LUA Widgets: background() not called if menu is active
8 participants