-
-
Notifications
You must be signed in to change notification settings - Fork 333
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: customisable switches generated sources #5016
Conversation
Tested Companion and T20 libsim.
For consistency with other sources and switches and to keep lists short, groups should only be available for selection in lists when they are configured. The groups should optionally allow them to have user defined names assigned like all other sources and switches, and for the same reasons they have user defined names. I can imagine this would be the first enhancement request after the feature becomes available so why not do it as part of this implementation to save rework. |
@3djc 4722 WILL break this due to the index change from zero to one based |
I'll review this when #4722 gets merged, but I agree, we need to changes merged sooner rather than later |
76b59db
to
41ecf71
Compare
Ready for test |
Still needs to have unused/not configured GRx suppressed to be consistent with other types |
Radio side is ready to push, where should I look in companion code for cpn ? |
RawSource::isAvailable to show/hide in lists. |
Will add companion side show/not show once #4994 has been merged, as some functions I need are in there. Anohow, thats more icing o n the cake and can be merge as such imho |
IMHO it does not get merged until it is a completed feature as this is how the two get out of sync. |
452c315
to
aeacc9a
Compare
@elecpower , could you please take care of : Ensure group config changes trigger the lists to be refreshed. |
Thanks @elecpower , seems to work ok :) (obviously your change do, but I was referring here about the whole PR :)) |
33032fd
to
7f58f91
Compare
This is working better than I could have hoped when the customisable switches were first introduced, and the need for a traditional 6POS needed. Just one tiny detail... GR2 and GR3 don't actually appear to giving any output here? GR1 works, but GR2 and GR3 don't 😢 Unless I missed something in configuring this? Just a thought... can we get something for the GRx in the popup source lists also? Not an issue either way, as logical switches aren't there either, so maybe this is can be considered a switch also? |
7f58f91
to
a02dcc1
Compare
I believe I have fixed the computation issue (thanks to the just in time T15 merged that allowed me to work with debug (yeah !!)), but please test it thoroughly. Not sure I understand your other point, maybe ping me on discord ? |
I think that is probably topic for a followup PR... I was referring to this popup menu... Would be nice to have shortcuts to the customisable switch groups and/or LS... but it also needs to not show trainer when trainer is not actually configured. On colorlcd, probably needs to get added to the switches filter group also. And I thought you'd like being able to rebase to include the T15 ;) |
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.
LGTM on T-Pro and T15 :)
This add 3 sources (GR1 to GR3) on radio that have customisable switches as described in #4989
This PR is best when used in conjunction with #4994 (which prevents user misconfiguration)
This fixes #4989
@elecpower: I've done my best for companion support, but please check thoroughly.