New LUA scripts & widgets

[jurgelenas]

No description provided.

[CapnBry]

I've only gotten through the VTX Admin widget and some of the base code. There's a lot to get through and so much awesome functionality!

VTX Admin

Fix?

• VTX Admin sending status poll despite never using any info from it
• VTXA If 6POS changes when the radio boots, and the VTX Admin and the 6POS disagree, 6POS does not push the change because it never technically changed.
  • To reproduce: TX16S Enable 6POS mode, set R1-R6. Select any position except 1 and the VTX changes. Reboot the radio. The 6POS will have returned to position 1 but the VTX stays on what was selected before the reboot. This also occurs on radios with programmable 6POS which have the "Group 1 startup" set to anything besides "=".
• Includes a presets.txt, which has settings that may not be appropriate
• VTXA 6POS preset option "Off" shares the same word as VTX Admin = Off, which is confusing because it isn't turning VTX Admin off, it is just an empty unused 6POS preset. Perhaps the editor UI could call it maybeeeeee "Unused". Optionally, it can just be left off when building the 6POS build6posLabels if Unusedseses make it too long, or just shown as a "--" instead.

Minor

• VTXA ui/topbar and portrait have redundant P- generator

Cosmetic

• VTXA Status bar: Layout to small "VTX" then R2 under and leave P- off if no power set. Make it look like the other StatusBar Value widgets that are label over value.
• VTXA P- seems either too terse or too much for what it is. Maybe just don't show it at all or "---"?
• VTXA ui/topbar should reuse VTXDisplay.showStatus() or bandChannel() for its status
• VTXA ui has several uses of Protocol.isActive() or VTX.state.band == 0. Candidate for code reuse in loadable since it also uses this as a condition
• VTXA ui/portrait's difference between its hasModule() and the other state checking functions like isActive(), is there a reason this one is different?

Future

• Can multiple instances of the widget share a singleton state rather than each one tracking the status themselves and polling? Not a big deal because I don't think users would have more than 2 (statusbar and main widget area), but it seems like a great opportunity to share the same data and just have unique UIs only rather than multiple instances running their own loops and polls.

[CapnBry]

I do love that new TOOLS script. Every bit of functionality there seems to do exactly what you want it to and looks good. I haven't had a chance to run through it on B&W yet, but will do that tomorrow.

My major concern on the rest of this is the duplication of code between the tool script and the widgets (which use the common CRSF). That and all the polling that is going on in Telemetry/VTX Admin needs to be removed-- I think it is unacceptable to waste packets for cosmetics. VTX Admin can poll just for the channel change, but should be gated behind like LQ > 90% or something. The rest I think needs to go.

CRSF

Minor

• onElrsStatus() doesn't use CRSF.fieldGetString()
• setMock() should be cleared setMock = nil after calling to free memory or make it anonymous like (function() ... end)()

TOOL

Fix?

• Why does the tool script duplicate crsf.lua and shim.lua?
• Comments from original parseParameterInfoMessage() removed as if the code there is super easy to follow why it does things :-D
• parseParameterInfoMessage() return value unused, remove (multiple places)
• Sort of mixed used of false/nil (e.g. elrsV1Detected, backgroundLoading). nil/true should be used for booleans because it takes no RAM when false. Same with functions. Functions should not return false, they should just return or skip the line entirely if it is the last line and returning false. Lua returns nil if there is no return value so return false is redundant.
• main.lua should also clear setMock() after init()

Telemetry Widget

Fix?

• The telemetry widget shouldn't be polling all the time. Polling eats channels frames! I know you've got it rate limited but there is no reason to degrade the link performance for telemetry display.
  • requestDeviceInfo() - Telemetry widget does not ever need to, EdgeTX does this for you, the info will just show up if you're poping CRSF.
  • requestElrsStatus() - hasTelemetry comes from RQly present and > 0. The "model mismatch" is nice but not worth throwing away channels updates for 100% of the time. Perhaps requestElrsStatus() for modelMismatch can be gated in some way to if RSSI > -70 and not modelMismatch and justConnected then updatModelMatch() or something along those lines. But for sure hasTelemetry should not rely on polling, and modelmatch should only be polled when appropriate

Minor

• Telemetry.cellCntCnt initialize to nil, not 0
• checkCellCount() comments removed from original
• Telemetry.readLink being called redundantly for every field instead of having one shared state
• hd/portrait/sd_tall/sd/small.lua duplicate a very slightly different RxBt for some reason?
• small.lua also duplicates getRfModeStr()
• Some places hasTelemetry takes precedence over modelMismatch, sometimes the opposite

Cosmetic

• The Range % maybe we can get rid of? But would be nice to see something comparing RSSI with the minRSSI
• Antenna 1 or 2 vs 0 or 1? Very confusing that in one place (Telem list) Ant 0/1 yet another (here) uses different numbering scheme. How about "Ant 1" "Ant 2" like the Lua has it to help distinguish?
• No TX model or version identifier, e.g. "RM Nomad X-Band (3.6.3)"
• "No telemetry" vs "No RX Connected"
• Is just 3 lines of text, LQ, RSS1, and Rate/Power (in tiny text), and battery pops in if it exists? It feels more like a proof of concept listing a few items and most of the font size is for ants

General

• You mentioned editing the bindphrase with a CRSF string editor. We have the ability to set the bindphrase / UID via MSP and we should not add a string bindphrase field to the parameter list. It should just use the direct MSP. A standalone bindphrase manager to easily swap between them would be cool, but I don't think many would use that. We can rejigger my test lua from the PR to use your backend in another PR.

[CapnBry]

And B&W works as well. Here's just a couple more though

• edgetx.yml - Incorrect repo url ExpressLRS-Lua-Scripts. As a side note, can we also change the repo name to just lua (lowercase, just lua). It is just easier to remember, and all lua scripts are scripts.
• README.md - States ExpressLRS v3.0+ but the script seems to require v3.5.4 or higher. On v3.5.1 it only shows the Exit line and that's it (on a Zorro)

This concludes my initial review, thank you for your patience!

EDIT: I should also say I love the lazy loading. It is so good I had to verify that it was working correctly with some debug logging on the TX module. Perfect execution to background load on the TX but not the RX for those 50Hz people.

[jurgelenas]

@CapnBry thank you for your excellent feedback. I will address it in the upcoming weeks.