New LUA scripts & widgets - #3
Open
jurgelenas wants to merge 128 commits into
Open
Conversation
…ns for mk3 and 1 column for small screen radios
Member
|
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 AdminFix?
Minor
Cosmetic
Future
|
Member
|
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. CRSFMinor
TOOLFix?
Telemetry WidgetFix?
Minor
Cosmetic
General
|
Member
|
And B&W works as well. Here's just a couple more though
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. |
Member
Author
|
@CapnBry thank you for your excellent feedback. I will address it in the upcoming weeks. |
Member
|
Oops! One more thing. In v4.1.0, the name of the Band item has changed to be more descriptive so we need this or else the widget gets stuck on Loading diff --git a/src/WIDGETS/ELRSVTXAdmin/loadable.lua b/src/WIDGETS/ELRSVTXAdmin/loadable.lua
index 92829bf..0c3bb60 100644
--- a/src/WIDGETS/ELRSVTXAdmin/loadable.lua
+++ b/src/WIDGETS/ELRSVTXAdmin/loadable.lua
@@ -255,7 +255,7 @@ function Protocol.onSettingsEntry(data)
Protocol.statusText = "VTX Admin not found"
end
elseif st == Protocol.STATE_DISCOVER_VTX then
- if fieldName == "Band" then
+ if string.sub(fieldName, 1, 4) == "Band" then -- Band or Band/Enabled
VTX.ids.band = fieldId
elseif fieldName == "Channel" then
VTX.ids.channel = fieldId |
Member
|
crsf.lua defines addresses that I think could also be improved.
|
ExpressLRS PR #3608 renamed the Band field to Band/Enable and its Off option to Disabled. Match both labels during field discovery so the widget works on firmware before and after the rename, and update the CRSF simulator to mirror the new firmware parameters.
Member
Author
Fixed. Thanks for the tip! |
…, ADDRESS_HANDSET_ELRS
Member
Author
Tested on GX12. It does not work on 3.5.4, 3.5.5, 3.5.6. Started working with 3.6.0 and above. |
Member
Author
Additional notes: GX12 does not work with firmware below 3.6.0 at all. Created a PR to address min version requirement for GX12: Our new lua does not work with ExpressLRS below 3.5.4, because the previous versions were not fully CRSF spec compliant. This was addressed in this PR: ExpressLRS/ExpressLRS#3123 |
Member
Author
done |
|
Hello! When running the elrs-bindphrase script on a RadioMaster Boxer (B&W) with EdgeTX 2.11.6, the message "LVGL (EdgeTX 2.11+) required" appears at the top. |
The widget requested ELRS_STATUS every 2 seconds by writing field 0, but nothing under WIDGETS/ELRSVTXAdmin ever read the resulting hasTelemetry, modelMismatch or elrsFlagsInfo state. The UI is driven entirely by the folder PARAMETER_READ sent alongside it. On the TX side that write only triggers an ELRS_STATUS reply; it sets no mode, arms no timer and raises no device event, so dropping it changes no behaviour and halves the widget's steady-state traffic. It also bypassed the CRSF singleton's rate limiter, so with the Telemetry widget loaded the two sent 1.5 status writes per second instead of 1.
The five per-screen UI files each re-derived the same things locally: "not Protocol.isActive() or VTX.state.band == 0" appeared on 15 lines under ui/ and 8 more in loadable.lua, pitModeColor and pitModeText were defined identically in sd_tall, portrait and small, and the power string existed in four places. Add Protocol.hasModule() and VTX.isTuned() for the state checks, and move every power and pit-mode string into VTXDisplay as powerShort, powerLong, pitText, pitTextLong, pitShort and pitColor. detailLine and detailLong now compose those rather than rebuilding them. The screen files drop their local copies and reference the shared ones directly, which works because LVGL takes them as bare zero-arg callbacks. Protocol.hasModule() also names the condition the 6POS cheatsheet was already using inline: presets come from presets.txt on the radio, not from the VTX, so they are worth showing before discovery finishes. That is why it is weaker than isActive(), which was not obvious from the open-coded comparison portrait had given a local name. No behaviour change: statusText keeps its three-way branch so the loading state survives, detailLong keeps returning "VTX Disabled" on band 0, and pitShort and pitColor keep their guards.
VTX.parseFolderName tested parts[#parts] == "P", but the firmware only writes "P" when Pitmode is set to On. When it is bound to a switch, updateFolderName_VtxAdmin writes the aux label instead - "AUX1\192" through "AUX10\193", where \192 and \193 are the ExpressLRS up and down arrow glyphs. Every switch-bound configuration therefore displayed "Pit Mode Off". Presence of a fourth field, not its text, is what marks pit mode configured. Parse it as three states: "P" is confirmed on, anything else is a binding whose switch name is the label minus its one trailing arrow byte, and no fourth field is off. string.sub does that without a pattern match, which matters on a path that runs per folder read. pitmode now means "definitely on" and gains a pitmodeAux companion, so a binding renders "Pit Mode AUX1" and leaves the display its normal colour rather than turning red. Red asserts the VTX is muted, and the folder name carries only the binding, never the live switch position - the firmware derives that from the channel data, which is not worth a pattern match here for a value the name never promised. The CRSF simulator's Pitmode parameter only offered "Off;On" and so could not reach this path at all; it now carries the firmware's real option list.
ExpressLRS treats power level 0 as "no change" - luaVtxPwr's options are literally "-;1;2;3;4;5;6;7;8". updateFolderName_VtxAdmin omits both power and pit mode from the folder name in that case, and the module hides the Pitmode field outright: // Pit mode can only be sent as part of the power byte LUA_FIELD_VISIBLE(luaVtxPit, config.GetVtxPower() != 0); The widget showed "P-", "Power -" and "Pit Mode Off" anyway, which claims a pit mode state the protocol has no way to hold. Add VTX.hasPower() and gate every power and pit string on it, so the line is simply absent until a level is chosen, and hide the Pit Mode row in the full-screen editor to match. The row keys on the desired value, so it appears as soon as the user picks a level rather than waiting for a write to land. The "P-" and "Power -" fallbacks are then unreachable and are gone, which is what the redundant-generator review comment was really about. pitShort and pitColor lose their guards as redundant: after the previous commit pitmode can only be true when tuned and powered. VTX.isDisabled() names the remaining band-0 check that detailLong and pitTextLong need, since those report "VTX Disabled" rather than falling silent.
The top bar rendered one MIDSIZE line, "R2 P2", built from a local getStatusLine() that duplicated VTXDisplay.bandChannel() and a second copy of the power string. It also looked nothing like the status bar widgets beside it. Stack it instead: "VTX" over the band and channel, two centred SMLSIZE lines. That matches EdgeTX's own HeaderDateTime, which is the stock element with this shape, and the sibling ELRSTelemetry top bar. It is deliberately not ValueWidget, whose FONT(STD) caption over FONT(L) value comes to 43px of text in a 39px top-bar zone and clips the descenders - two SMLSIZE lines are 34px in the same space. Power no longer appears here at all, so the line does not recentre when a level is set or cleared; the larger tiers still carry it. Dropping both local helpers leaves the file needing only VTXDisplay, so the context table the five screen files pass in shrinks to that one key.
Hiding the power and pit mode line frees a row in the minimized tiers, so the preset cheatsheet no longer has to fit six labels across one line in a small font. Split it 1-3 over 4-6 and let each screen pick the font per tier through WidgetUI.fonts.<tier>.cheatsheet, alongside the hero and detail entries already there. The split is unconditional rather than tied to whether power is set, so the cheatsheet does not reflow every time the pilot changes power. portrait already had exactly this layout as a local buildCheatsheetNarrow for zones under 200px; promote it to VTXDisplay.buildCheatsheetRows and drop the copy. buildCheatsheet stays for the sixth and quarter tiers, which have no spare row. portrait's quarter tier keeps its existing width test, so that one still picks between the two by zone width. Fonts are STDSIZE where there is room and SMLSIZE where there is not - 480x272 at the third tier, and every tier on 320x240.
Presets.process() latched Presets.lastPos before calling Protocol.writeConfig(), which refuses to run outside STATE_READY. At widget create lastPos is -1, so the first debounced sample was consumed by the latch and its write discarded; every later tick then returned early on pos == lastPos. Nothing reached the module until the pilot physically moved the switch, so the VTX kept whatever band and channel were last sent to it. Whether that showed depended on a race. Discovery is round-trip limited, not timeout limited - onSettingsEntry resets fieldTimeout to 0 on every response, so the 500ms in fieldResponseTimeout() is only a lost-frame retry. Roughly sixteen reads can finish inside the 200ms debounce, which is why the original report did not reproduce on every radio. The same latch can also drop a switch move made during STATE_SENDING, though that window is narrower than the debounce and could not be reproduced. Hold the position until a write can land instead. Since lastPos is -1 on every create and is never persisted, the first tick after STATE_READY becomes the boot write, through the same path as any switch move; writeConfig()'s per-field diff means nothing is sent when the module already agrees. wgt.background() already runs poll, tick then process in that order, so syncDesiredFromState() on the READY transition lands before the preset is applied rather than wiping it. Nothing is needed for programmable 6POS radios. DEFAULT_FS_STARTUP_CONFIG is FS_START_PREVIOUS on every switch and setFSStartupPosition() leaves those alone, so a customizable-switch group restores its previous position across a reboot by itself. Verified under the CRSF simulator's slow_loading scenario: before, every widget instance logged "skipped - not ready" and no write reached the module; after, none do and the module follows the switch.
VTX.parseFolderName zeroes power and pitmode whenever ExpressLRS omits them from the folder name, which it does whenever power is "-". syncDesiredFromState copies those zeros into VTX.desired, and nothing re-syncs desired after a send completes. A later 6POS change then diffed the stale zeros against the module's real state and wrote power 0 and pit mode off, silently undoing both. Re-base desired on the module's current state before applying the preset so the 6POS path can only ever write band and channel. Asserting the position on create makes this reachable in one switch move rather than two, so it belongs with that change.
Band 0 rendered as "Off" in the preset editor and the cheatsheet, the same word the VTX Settings band selector uses for a disabled VTX. They mean different things: one turns the VTX off, the other is an empty preset slot that leaves the VTX alone. Show "--" for the preset slot in both places and leave the VTX Settings row alone, since "Off" is right there and matches VTXDisplay's "VTX Off" and "VTX Disabled". The two choice lists now differ only in their first entry, so they stay separate literals rather than being generated from one table with the label parameterised. BAND_NAMES existed only to map band 0 to that shared "Off", so it becomes BAND_LETTERS, a plain 1-based array, and the mixed_table selene pragma goes with the [0] entry. build6posLabels tests the numeric sentinel instead of comparing a rendered string. BAND_VALUES stays as it is: it parses protocol letters, it is not a UI list. "1:--" is four characters against "1:Off"'s five, so no tier reflows.
processPushSource seeded Presets.pushLastVal to -1 as a "no sample yet" sentinel, but the edge test was pushLastVal > 0, which turns "unknown" into "was low". A trigger source already high on the first sample therefore read as a low-to-high edge and pushed to the VTX with nobody having touched it. At widget create that was harmless - pushToVtx() bails as not ready - but the editor resets the sentinel the moment a source is picked, and the protocol is ready there. Assigning a two-position switch that happens to be on, or assigning one while holding a momentary button, sent an unrequested Send VTx immediately. Track the level as pushLastHigh, nil until sampled, and have the first sample adopt it without firing. The -1 was also serving as the getValue()-returned-nil fallback, which is a different condition; that now returns without sampling rather than being recorded as low. Verified with a trigger source that reads high from the first sample: eleven spurious pushes before, none after.
Send VTx Trigger carries a hint line explaining itself. Auto Push to VTX sits directly above it, decides whether that trigger does anything at all, and said nothing. createRow already renders a hint as a SMLSIZE sub-label, so createToggleRow only needed to forward one.
presets.txt is runtime state the widget writes, and the tracked copy held one developer's settings - 6POS enabled, auto push on, and raw source indices that mean different inputs on different radios. edgetx.yml already excludes it from both pkg install and dev sync so it never shipped, but it sat in the tree as if it were a default. The .gitignore entry meant to cover it still named color/WIDGETS, the path from before the unified-LCD migration, so it had stopped matching anything. Point it at src/WIDGETS and untrack the file; Presets.load() falls back to disabled with no source and R1-R6 when it is absent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.