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: Add "Enable" button to all SF/GF (#3024) #3601

Merged
merged 9 commits into from
Dec 3, 2023
Merged

feat: Add "Enable" button to all SF/GF (#3024) #3601

merged 9 commits into from
Dec 3, 2023

Conversation

pfeerick
Copy link
Member

@pfeerick pfeerick commented May 16, 2023

Resolves #3024
Resolves #1165
Squashed rebase of #3038 for inclusion in v2.9

@pfeerick pfeerick added enhancement ✨ New feature or request color Related generally to color LCD radios labels May 16, 2023
@pfeerick pfeerick added this to the 2.9 milestone May 16, 2023
@pfeerick
Copy link
Member Author

@philmoz I've finally attempted to bring #3038 back up to a mergeable state. Would you mind looking over it, and seeing if there is anything that went horribly wrong during the rebase, or could be improved on. I've only had a quick run-through to make sure TX16S, TX12 and X9D+2019 all compile, and appear to be functioning in the simulator.

The B&W 128x64 interface would be nicer if the checkboxes only showed on configured SFs, but as far as I can tell I ported that code over correctly so may be an issue in the original implementation.

@philmoz
Copy link
Collaborator

philmoz commented May 16, 2023

Line 218 of radio/src/gui/128x64/model_special_functions.cpp should be 'j = 5;'

The fields no longer fit on 128x64 for the 'Lua Script' function - script name overlaps repeat value. Not sure how to fix this.
Screen Shot 2023-05-16 at 8 11 00 pm

@pfeerick
Copy link
Member Author

pfeerick commented May 16, 2023

Thanks :)

Yeah, the 128x64 interface is just cramped for space. I guess there will probably be an issue with the play track function also. And don't really want to go to having edit screens just because of an active/inactive checkbox.

@philmoz
Copy link
Collaborator

philmoz commented May 16, 2023

The 'enable' flag is not being read in from the YAML file for some functions.

  • Override works
  • Instant Trim and Lua Script fail (always start disabled).

Tested in the simulator by re-loading the radio data button.

@philmoz
Copy link
Collaborator

philmoz commented May 16, 2023

PR #3602 fixes a few problems.

@pfeerick
Copy link
Member Author

Merged in, thanks. Nice thinking... removing the macro since it's no longer needed/used would have conveniently highlighted any other cleanup that could be done. And looks like I did mangle some of the rebase in yaml_datastructs_funcs.cpp ... 😇

@pfeerick pfeerick self-assigned this Jun 2, 2023
@pfeerick
Copy link
Member Author

pfeerick commented Jun 8, 2023

@3djc Do you have any thoughts on this re: UI? Are there any other tricks we can use?

@3djc
Copy link
Collaborator

3djc commented Jun 8, 2023

This feature (the checkbox) was introduced to be able to disable 'more complex' SF without loosing the configuration. I'm really unsure of the value of saving say 'instant' trim.... and there is not much magic that can be done without resorting to 'one' page, but what a step back...

@pfeerick
Copy link
Member Author

pfeerick commented Jun 8, 2023

Need to think a little more outside the box when thinking of functions like instant trim, screenshot, etc - it makes more sense when a global function, as you can leave it in the list, with the switch allocated, and then only turn it on when needed.

The only catch here is the 128x64 screen... it's not an issue for 212x64 or colorlcd, and adds some much-needed consistency to the UI. Looks like this moves to 2.10 until we can figure out to squeeze it in.

@pfeerick pfeerick modified the milestones: 2.9, 2.10 Jun 8, 2023
@philmoz
Copy link
Collaborator

philmoz commented Jul 28, 2023

The only thing I can think of for the 128x64 LCD is to reduce the length of the affected function names (in all translations) and then move the parameter to the left.

Examples:

Screenshot 2023-07-28 at 4 42 03 pm

Screenshot 2023-07-28 at 4 45 31 pm

@raphaelcoeffic raphaelcoeffic marked this pull request as draft August 12, 2023 05:51
@raphaelcoeffic raphaelcoeffic added the needs: rebase A git rebase on top of the latest destination branch version is required label Aug 12, 2023
@pfeerick
Copy link
Member Author

Now that I've rebased #2145, this actually looks a lot neater to do (shorten affected names just for the 128x64... was a bit of a mess before how the SF strings were defined.

@philmoz
Copy link
Collaborator

philmoz commented Oct 28, 2023

I've rebased this to current main.

I changed the default for the 'active' flag to disabled as there is some risk when setting the other parameters of having the function enabled first.

Also discovered a problem - the YAML reader assumes the new format where all SF/GF have the active flag. It does not handle reading older model files where only one of the active/repeat settings was ever set. Depending on the function used this can cause the repeat value to be read incorrectly. Have not tested Companion yet; but I suspect it will have issues as well.
Fixed YAML reading.

@pfeerick pfeerick removed the needs: rebase A git rebase on top of the latest destination branch version is required label Oct 28, 2023
@philmoz
Copy link
Collaborator

philmoz commented Oct 28, 2023

I've added the changes to shorten the function names and adjust the parameter position for 128x64 LCD screens.

@pfeerick pfeerick removed their assignment Nov 12, 2023
@pfeerick pfeerick marked this pull request as ready for review November 12, 2023 22:58
eshifri and others added 8 commits December 3, 2023 12:12
Formatting.

chore(ui): Convert GVAR tab to LVGL (#2945)

fix(ui): Adjust model setup button layout to better fit portrait screen (#2956)

* Adjust model setup layout to better fix on portrait LCD screen.

* Set buttons to fixed width and allow to grow vertically to fit text.

* Cleanup layout logic - buttons stretched horizontally, centered vertically.

fix(color): Invalid custom curves + layout updates (#2973)

* Layout updates and bug fixes for curve editor.
- Fix issue 2972 where an invalid curve could be created
- Use vertical layout or list of points
- Add border to preview to better show end points
- Fix issue where preview was redrawn very frequently
- Better organisation of curve edit classes

* Fix to refresh curve preview on input edit page when parameters changed.

chore: freeRTOS task stack diagnostics (#2976)

* migrated stack diagnostics (available, free) to using built in FreeRTOS builtin function

Note: main stack must be and is treated differently as it is not under FreeRTOS control. main stack is painted by the STM32 startup routine. free main stack is calculated checking the remaining painted stack area.

* corrected my merge error

* missed a blank

fix(lua): fix drawHudRectangle for roll = 0 (#2978)

- fixed an extra line at the bottom of the hud rectangle
- fixed overflowing hud rectangle for horizon outside viewport (pitch > 0)

fix(lua): Remove faulty carryTrim, replace with trimSource (#2995)

* Fix carryTrim in LUA

* Remove carryTrim

feat(diags): Red LED before RTC data, blue LED after (#3006)

* Added turning the red LED before RTC data and blue LED afterwards

With the LED turned RED if the RTC data retrieval fails it will stay red.
This will at least be an indication that the RTC startup encountered
a problem. For now there is no indication that this problem occurred.

* GH-3005 Moved LED manipulation to conditionally compiled when RTC operation happens

fix(color): Update top bar date & time when in setup pages (#3037)

fix(translation): JP had overlapping strings due to line breaks (#3004)

fix(cpn): Translate OFF in external module setup (#2998)

* Translate OFF in external module setup

* Update moduledata.cpp

Add B/W and CP support

Init enabled to 1

delete extra file

Fix Save/Read operations

Cleanup and fix R/W

Cleanup based on review

Change the fields order to match YAML

change the fields order to match the YAML

Add headers, set new CF to enabled

Remove "ON" label from "Enabled" field.
Fix "disabled" when swicthing functions.

Set enable==1 when changing the function.

Set "enabled" if function is changed on B/W radios
Fix loading and saving of 'active' flag and 'repeat' param in YAML file.
Add missing logic for 'Lua Script' repeat param.
Remove redundant HAS_ENABLE_PARAM macro.
Remove empty column from Companion special/custom functions page.
@pfeerick pfeerick removed the color Related generally to color LCD radios label Dec 3, 2023
@pfeerick pfeerick changed the title rebase: Add "Enable" button to all CF (#3024) feat: Add "Enable" button to all SF/GF (#3024) Dec 3, 2023
@pfeerick pfeerick merged commit ee73fa4 into main Dec 3, 2023
40 checks passed
@pfeerick pfeerick deleted the 3038-rebase branch December 3, 2023 02:23
ulfhedlund added a commit to ulfhedlund/edgetx that referenced this pull request Dec 8, 2023
ulfhedlund added a commit to ulfhedlund/edgetx that referenced this pull request Dec 8, 2023
pfeerick pushed a commit to ulfhedlund/edgetx that referenced this pull request Dec 12, 2023
pfeerick pushed a commit to ulfhedlund/edgetx that referenced this pull request Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add missing Enable switch to SF/GF functions add enable Toggle for all Special/Global Functions
5 participants