EdgeTX
feat(radio): refactor of switch handling to better support customisable switches
This started out as a way to let users set the LED color for the SA & SD switches on the GX12 (see PR #6053); but it became clear that we really needed to be able to have these as fully customisable switches (like SW1 - SW6).
This PR is a big refactor of the switch handling to achieve this. All momentary switches with LED's can be set as customisable switches. Switch name ordering is maintained (SA & SD come before SW1-6).
The customisable switch settings in the model yaml file are now readable instead of being magic numbers.
I plan to extend this by also adding configuration of the customisable switches in the radio settings. Like we have with the enabled features, the custom switch settings in the model can be set to use the radio settings. So you will be able to have radio level settings for all customisable switches; but still override per model.
Companion still needs a lot of work. Basic read/write of the yaml files is done; but the model edit, radio edit and simulator will need fixing (@elecpower - I will need your help here).
WARNING: backup your radio and model files before playing with this. There are changes to the yaml files that are not backward compatible.
I have tested on GX12, T15, T20V2, Bumblebee and ST16. T20, TPro, and TProV2 are untested as I don't have these radios.
Todo:
- [ ] Companion support.
- [x] Radio settings for customisable switches.
- [x] Test radios with flex switches.
- [x] Investigate if flex switches can be unified.
It will be a few days before I even think of looking at this, but I can do the tests for T20, TPro, and TProV2. I expect the T20 would be fine as it is somewhat similar to the T20V2 in this regard. I take it #6053 will be updated on the back of this?
It will be a few days before I even think of looking at this, but I can do the tests for T20, TPro, and TProV2. I expect the T20 would be fine as it is somewhat similar to the T20V2 in this regard. I take it #6053 will be updated on the back of this?
No rush - I have more testing to do.
This supersedes 6053.
When the radio side is all but finished I'll tackle the Companion side. Working on the build toolchain upgrade at present.
I have added setup for custom switches to the radio hardware page (and radio data structure) so you can define type, name and startup position at the radio level.
Added a new type for model custom switch setup so you can choose to use the radio settings for each switch.
Note: there is no group setting for custom switches at the radio level. I thought it might be too confusing.
I think the core changes are done for the firmware (pending any bug fixes).
@elecpower for Companion I have yaml read / write working and have updated the model & radio edit to include the changes. I have added the custom switches to the simulator UI; but have not wired them to the running firmware. Todo for Companion:
- [x] Edit custom switch colors on the model setup edit page
- [x] Edit custom switch colors on the radio hardware edit page
- [x] Get the simulated switch type info for the simulator from the model or radio settings
- [x] Link the simulated switches to the running firmware in the simulator. This needs to be two way as the switch state can be changed by the user or by the firmware.
Any suggestions would be appreciated.
@philmoz thanks for the update. I'll start to have a look in the next few days when free time.
Use QColorDialog along the lines used for simulated radio case for the colour picker in Radio Profile settings.
Looks like we need to create new simulator widgets for six position buttons/crows feet with LED colours as an alternative to 6 position knob and LED colours for all switches (not just current SA/SD)
These are likely on your TODO list but just in case. Some initial feedback from testing.
GX12 libsim:
- after creating a new model, switches SW1-6 not initialised to Global type
Companion GX12 profile
- after creating a new model, all customisable switches should have Global type
- model setup switch warnings missing for A & D
- switch dropdown lists broken for A & D
- setting hardware SW1-6 to None and Global for model the switch dropdown lists contain the unavailable switch
The default for SW1-6 should be model setup, 2POS and group 1 for all of them to mimic the current way it works.
The default for SW1-6 should be model setup, 2POS and group 1 for all of them to mimic the current way it works.
I can understand not changing existing models but IMO it's counter intuitive especially since defaults are inconsistent across all customisable switches. How would we treat future customisable physical switches?
Retested after your last commit and all appear fixed except:
- model setup switch warning choice should be hidden in Companion and radio for ALL switches of type Toggle. Currently only non-customised Toggles are hidden.
Retested and switch warning visibility changes to reflect global and model level type.
Switch warnings are enabled by default however when a switch is changed from None or Toggle at global or model, thus becoming visible, the corresponding checkbox is not checked.
If a switch is not available for warning the corresponding warning flag is cleared (so the checkbox will not be set). If the availability changes this does not change the flag state so the checkbox remains unchecked.
IMO a switch becoming available should attract the default state of warning enabled. The state for all switches on model creation was changed some time ago from disabled to enabled due to user feedback. My two cents worth.
IMO a switch becoming available should attract the default state of warning enabled. The state for all switches on model creation was changed some time ago from disabled to enabled due to user feedback. My two cents worth.
That's not what is happening in companion for physical switches. E.G. on TX16S set SA to warning enabled (position does not matter). Change radio setting for SA to None. Open model settings - SA is not shown in warning. Change radio setting back to 3POS. Open model setting and switch warning is disabled for SA.
Companion works very differently to the radio in this regard. The radio does not alter the switch warning state when the switch type is changed. Companion splits the warning state into two variables (position and enabled) and changes the enabled state when the model settings is opened. Companion needs to be fixed to work the way the radio does.
B&W differed from colour in handling and Companion was stuck in the middle and likely ended up in the mess you fixed. Thanks.
I'm guessing a lot of the companion code has evolved over time ro handle difference and changes - a lot of it makes me scratch my head.
Have you considered a singleton for the radio settings to avoid passing the GeneralSettings& around everywhere?
Companion has been traditional an afterthought so lots of quick and dirty by one and all.
GeneralSettings like Firmware and Boards need to have multiple instances for data conversion. Also there needs to be one instance for each models and settings file open.
And likely some less used spots. I find these when have a bright idea to clean up what I expected would be unused code.
After the toolchain update is finished I'm back to clearing out binary limiting code then onto Boards and Firmware.
Added the ability for Lua scripts to override the CFS LED colors.
At the moment the Lua override will only apply when the switch LED color is off (0) for the current switch state. I.E. if the switch is off and the switch off color is 0 then a Lua script can override the color. If the switch is then pressed on, and the switch on color is not 0 then the switch on color is used.
How do you deal with numbering, and already available scripts that loop to LED_STRIP_COUNT to create ambiance ? Those will affect off led too which is likely unwanted ?
I start to wonder if we should have a regular call for cosmetics led that "pimp my ride" user will use, and another one for functional leds
How do you deal with numbering, and already available scripts that loop to LED_STRIP_COUNT to create ambiance ? Those will affect off led too which is likely unwanted ?
How do you know it's unwanted - it's just as likely that this is exactly what the end user intended.
Lets be realistic, there are already many apparence led scripts that mimic all kind of effects. It is quite unlikely that you want off functional leds to be part of those, or there needs to be at the very least a way to exclude those that doesn't require user to be a programmer. Many users use lua scripts,but only a handful actually author lua scripts
Bringing us back to what I said right from the start... rather than assume we know what the user wants, perhaps there needs to be a specific "lua" mode. Off/Red/White/Blue/Lua. Once you know that is the intent, there is no longer a clash between the functional switch led function/UI/UX, and it being controlled by a lua script. :facepalm:
Unfortunately, not quite true. You might have a cosmetic script (some radio in dev are even delivered with some running by default) but want to do something specific with one (or more) of the functional leds (like a 3 color arm like asked above). I'm personally in favour of two distinct lua call (or an option field on a single one)
You'll have to run that by me again, as I don't quite know what you are talking about (but might also be because brain is not working by this time of night). I was specifically referring to the options where you define for a functional switch what its on and off colours are. If you define either to be Lua, Lua is what sets the colours. Better perhaps is a seperate Lua checkbox for both the on and off states? As then you can have set the on and off colours as per normal, and then you can also specify Lua can also control the colours.
Ok, let me try again.
Most radio with cosmetics leds come with scripts that you run that loop from 0 to LED_STRIP_LENGHT and compute a color pattern for each led and set all the leds according to that pattern. Say you want a police like effect, you set all odd number led to red, all even to blue, and then next second, you do the opposite. That's for cosmetics
Now if you set your SW1 led to LUA, it will be reacting to the pattern above, when what you might want to do it something completely different.