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.

Sign up for GitHub

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

Jump to bottom

fix: enforce customisable switches rules when configuring #4994

Merged
merged 11 commits into from
Jun 1, 2024
Merged

fix: enforce customisable switches rules when configuring #4994

merged 11 commits into from
Jun 1, 2024

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented May 12, 2024

Fixes #4990

Changes to the UI for setting up custom function switches to prevent invalid setup.

@3djc
Copy link
Collaborator

3djc commented May 12, 2024

Overall very nice (i realize this is WIP)

  • I think we can hide Groups that have no switches in them
  • There is a functional issue, where selecting a switch for the group doesn't delete the '=' for other switch in group
  • I think most, if not all, of the functions in model_setup should move to a common place, like switches.cpp

I will wait for that change to be merged to address #4989, since both need similar functions added

@philmoz
Copy link
Collaborator Author

philmoz commented May 12, 2024

  • I think we can hide Groups that have no switches in them

Done.

  • There is a functional issue, where selecting a switch for the group doesn't delete the '=' for other switch in group

I think this is fixed now.

  • I think most, if not all, of the functions in model_setup should move to a common place, like switches.cpp

Moved the ones that aren't UI specific.

I've also renamed FS_START_UP to FS_START_ON and FS_START_DOWN to FS_START_OFF to avoid confusion and because the previous names were backwards. E.G. if you use one of the custom switches in a SF then the ON state shows as SW DOWN, not UP which is consistent with usage for normal switches.

@3djc
Copy link
Collaborator

3djc commented May 13, 2024

Perfect !

@philmoz
Copy link
Collaborator Author

philmoz commented May 14, 2024

For completeness, I've added an option to set the startup state of a group to 'Off'.
This will force the group to be all off on startup.
Only applies to groups that are not set to always on.

@3djc
Copy link
Collaborator

3djc commented May 14, 2024

Successfully tested on T20V2

@philmoz
Copy link
Collaborator Author

philmoz commented May 14, 2024

Changed the diag page to use arrows and OFF/ON labels.

screen-2024-05-14-223649

@pfeerick pfeerick added this to the 2.11 milestone May 14, 2024
@3djc 3djc mentioned this pull request May 16, 2024
Open
@elecpower
Copy link
Collaborator

Screenshot from 2024-05-17 13-17-40

Add space character between Group and number ie Group1 -> Group 1 for labels and drop down lists

Would it not be more appropriate for the 'Start Switch' label to be 'Start' as the option per group is not always a switch eg Restore or Off

@pfeerick pfeerick self-requested a review May 30, 2024 01:44
@pfeerick pfeerick changed the title fix(radio): enforce rules for custom switches when setting up in the UI. fix: enforce custom switches rules when configuring Jun 1, 2024
@pfeerick pfeerick added the bug 🪲 Something isn't working label Jun 1, 2024
pfeerick
pfeerick approved these changes Jun 1, 2024
View reviewed changes
Copy link
Member

@pfeerick pfeerick left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firmware side seems to work great, applies the rules as laid out and all makes sense.

Companion side needs a little more work as it while it seems to apply the rules just like the firmware, it does it differently... i.e. if you set SW6 to toggle, no group, set a group to always on, you can't assign SW6 to that group (whereas the firmware would let you, but force it to 2POS). And if you do this in reverse, i.e. set SW6 to be momentary, assign it to a group, and then set the group always on, it disables the group combobox, and the type field flashes but doesn't update until mouse-over and is now unselected rather than 2POS.

image

I would prefer to merge this as is, and continue any Companion work in #5016 since that is the next one in line for this three PR chain... sound ok? (Assuming, of course, this is isn't related to a limitation of what we can do with Companion UI/OS specific).

@philmoz
Copy link
Collaborator Author

philmoz commented Jun 1, 2024

I would prefer to merge this as is, and continue any Companion work in #5016 since that is the next one in line for this three PR chain... sound ok?

Give me a little while to check - I would prefer to have this PR clean if possible.

@philmoz
Copy link
Collaborator Author

philmoz commented Jun 1, 2024

Should be fixed now.
Also squashed another bug where startup state of all switches could get reset when group was set to always on.

@pfeerick
Copy link
Member

pfeerick commented Jun 1, 2024

Yes, looks like you got it... thanks! :)

@pfeerick pfeerick merged commit 9a7e1db into EdgeTX:main Jun 1, 2024
47 checks passed
@pfeerick pfeerick changed the title fix: enforce custom switches rules when configuring fix: enforce customisable switches rules when configuring Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pfeerick pfeerick pfeerick approved these changes

Assignees
No one assigned
Labels
bug 🪲 Something isn't working
Projects
None yet
Milestone
2.11
Development

Successfully merging this pull request may close these issues.

Quirks in behaviour of customisable function switches.
4 participants
@philmoz @3djc @elecpower @pfeerick