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

fix: Don't allow select MULTI_TRAINER if no MULTI #2299

Merged
merged 2 commits into from
Sep 14, 2022
Merged

Conversation

pfeerick
Copy link
Member

@pfeerick pfeerick commented Sep 9, 2022

Fixes #1563 by ensuring Multi Trainer opt is not shown if no MPM
is enabled, as well as if MPM suport is not even compiled in.

It should only prevent TRAINER_MODE_MULTI being an selectable trainer mode if:

  • MULTIMODULE support not enabled
  • You don't have any modules enabled at all
  • Neither of the enabled modules are MPM

It seems that having two MPMs enabled is a valid case, so I've left that case alone.

Hardware tested on TX16S and TX12-MK2, and working as expected.

For me, this is both a bug (letting you select something that you can't) and an enhancement (don't show me stuff I can't use)... 🤷 😆

@pfeerick pfeerick added this to the 2.8 milestone Sep 9, 2022
@pfeerick pfeerick added bug 🪲 Something isn't working enhancement ✨ New feature or request labels Sep 9, 2022
@gagarinlg
Copy link
Member

gagarinlg commented Sep 9, 2022

I can confirm that having two MPMs is a thing.
One for the connection to the model and one for the connection to the trainee radio

@pfeerick
Copy link
Member Author

pfeerick commented Sep 9, 2022

@HThuren What's the equiv. of "OFF" (i.e Off, but uppercase) for Danish?
#define TR_VTRAINER_MASTER_OFF "Fra"

@ulfhedlund The same here, but for Swedish...
#define TR_VTRAINER_MASTER_OFF "Av"

I'm wanting to restore a change that as made in #1806 that looks like it got overwritten as part of rebasing the LVGL code - where we standardised on all uppercase for "OFF" for when trainer mode was, well, OFF! But it's also ok if it is left as it is since yours is translated - but though I should check before just making it uppercase incase that changes meaning ;)

@HThuren
Copy link
Contributor

HThuren commented Sep 9, 2022

Hi @pfeerick, I use OFF = "FRA" and Off = "Fra", when only the one word.
Thank you for asking :-)

@HThuren
Copy link
Contributor

HThuren commented Sep 9, 2022

Notice #2301, where changes are made to #2294

Fixes #1563 by ensuring Multi Trainer opt is not shown if no MPM
is enabled, as well as if MPM suport is not even compiled in.
@pfeerick pfeerick merged commit d54e656 into main Sep 14, 2022
@pfeerick pfeerick deleted the pfeerick/issue1563 branch September 14, 2022 02:34
@ulfhedlund
Copy link
Contributor

ulfhedlund commented Sep 16, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working enhancement ✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Displaying of "Master/Multi" Trainer Mode in X10s firmware and companion
4 participants