-
-
Notifications
You must be signed in to change notification settings - Fork 357
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
Add "Enable" button to all CF (#3024) #3038
Conversation
Correct me if I'm wrong; but doesn't this also require a change to the internal data storage for custom functions? AFAIK the same 8 bit property ('active') in the CustomFunctionData struct is used to store both the 'Active' and 'Repeat' values. So at the moment a CF can have one or the other; but not both. |
You are right! I have completely missed it. :-( |
Testing continued... Companion does not set to enabled by default on new function |
Thank you for testing! Will look into it. |
@eshifri not sure if you are finished with fixing read/write but just in case Companion is now reading/writing repeat but not TX16S libsim as always 1s on startup |
@elecpower Thank you again! I'm almost sure it worked for me last night. :-( Are going into simulator from companion or starting it it as standalone? |
@eshifri both methods. Haptic is the function I have been testing. |
Tested on TX16S libsim and appears all the functions are being interpreted by the radio correctly. Side point: currently in the CF grid (this maybe fixed in ui PRs) the enabled button does not show in single line CFs. |
@eshifri still some Companion things to do:
Do you need any assistance? |
Latest testing
|
@elecpower Thank you! I cannot reproduce your second item: for me all changes are saved. |
It depends on the before and after action. Steps to reproduce:
refer void CustomFunctionsPanel::functionEdited() |
Fix "disabled" when swicthing functions.
@eshifri thinking further about the change of action maybe since enabled is the proposed default then changing an action should also change to the default of enabled rather than keeping old value? The current bug is that it is still applying the old default of disabled. |
Isn't it what happens? At least in the simulator? |
@eshifri retested and same fix required for B&W radios |
@eshifri retested X9D+ libsim and works as expected |
@elecpower Thank you for investing your time into this! Your help is much appreciated. |
@@ -1609,26 +1612,24 @@ static bool w_customFn(void* user, uint8_t* data, uint32_t bitoffs, | |||
break; | |||
|
|||
default: | |||
add_comma = false; | |||
//add_comma = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please remove this
if (add_comma) { | ||
// "," | ||
if (!wf(opaque, ",", 1)) return false; | ||
} | ||
|
||
// "0/1" | ||
if (!wf(opaque, CFN_ACTIVE(cfn) ? "1" : "0", 1)) return false; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the indentation
if (!wf(opaque, "\"", 1)) return false; | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No part of this PR, but this type of formatting should be omitted. The risk of confusing the next person is too high.
I generally like that. It reduces complexity by having all SFs behave the same. |
Squash rebased in #3601 |
This PR adds "Enable" switch to all CF/GF for all targets.
By default newly created function is enabled.
Disabled functions are shown with different color.
Companion is supported as well.
Fixes #3024
Summary of changes: add "enable" switch unconditionally, preserve switch status when the function is changed, set special color for disabled function. Split "active" filed filed on the CF structure into "active" and "repeat".