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(radio) - Add options to disable radio and model menu tabs. #3484

Merged
merged 19 commits into from
May 22, 2023

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Apr 16, 2023

Resolves #1692, resolves #3016, resolves #3017 and #1249

Summary of changes:

  • Update radio and model data structs
  • UI for color LCD radios
  • Convert existing 'noGlobalFunctions' property.
  • Language strings.

TODO:

  • B&W radio UI
  • Companion support

Notes:

  • The existing 'noGlobalFunctions' model property is converted to the new model property and is reset. UI for this removed.

Screen Shot 2023-04-17 at 8 57 00 am
Screen Shot 2023-04-17 at 8 57 18 am

Screen Shot 2023-04-17 at 8 57 41 am
Screen Shot 2023-04-17 at 8 57 53 am

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 16, 2023

Before I start on the B&W and Companion changes, please let me know if this is what you were after.

@pfeerick
Copy link
Member

Oh, interesting, I was wondering if there was any benefit to extending it to the radio tabs... I'll have to try it out later today, but from the screenshots that looks perfect :)

@arthurson
Copy link

pls add heli option so there is no need to separate 2 firmware anymore

@gagarinlg
Copy link
Member

That is the main reason for this change 😉

@froqstar
Copy link

Would "visible Tabs" describe it better than "view options"?
Or is it planned to extend this page beyond tab visibility?

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 17, 2023

TBH I'm not a fan of 'View Options'.

It's possible the properties could control more than just the tab visibility.
It already disables mixer functionality for Global Functions.

Perhaps 'Features' might be a better name?

@pfeerick
Copy link
Member

pfeerick commented Apr 17, 2023

Well, since it now does more than configure view options, that probably doesn't fit any more. But "Features" is not very descriptive :-P What features? Why would I want to press it?

@rotorman
Copy link
Member

I wonder if numerous compile time options, such as e.g. HELI and GVARS, could be fully removed with this PR.

@pfeerick
Copy link
Member

Hopefully yes, but I think that will be a later task - as we may need them still right now for the smaller targets, especially as some of the translated builds can be slightly bigger without disabling some features. Hopefully, the savings from the removal of binary conversion has alleviated that pain for 2.9.

@gagarinlg
Copy link
Member

Those settings also cost RAM, so some people will complain, that there is less available RAM.

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 18, 2023

Added B&W radio support - that was fun ;)

Screen Shot 2023-04-18 at 7 05 23 pm

Screen Shot 2023-04-18 at 7 58 48 pm

Screen Shot 2023-04-18 at 9 01 31 pm

Screen Shot 2023-04-18 at 9 02 10 pm

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 18, 2023

Added code to collapse timer sections on model setup when timer is off. Fixes issue #3016

Screen Shot 2023-04-19 at 9 20 06 am

Screen Shot 2023-04-19 at 9 19 32 am

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 19, 2023

Added expand/collapse feature to the throttle, preflight checks and view options sections. Fixes issue #3017

Screen Shot 2023-04-19 at 10 18 27 am

Screen Shot 2023-04-19 at 10 55 57 am

Screen Shot 2023-04-19 at 11 20 17 am

@pfeerick pfeerick added this to the 2.9 milestone Apr 19, 2023
@philmoz
Copy link
Collaborator Author

philmoz commented Apr 19, 2023

Added Companion UI.

Note: this is only for updating the radio & model properties. It does not affect the tabs in the Companion radio or model editors. I.E. turning off Flight Modes does not hide the flight mode tab in the Companion Model editor. I'm not sure this is necessary (or how to do it).

Screen Shot 2023-04-19 at 5 25 07 pm

Screen Shot 2023-04-19 at 5 25 27 pm

@pfeerick
Copy link
Member

pfeerick commented Apr 19, 2023

It does not affect the tabs in the Companion radio or model editors. I.E. turning off Flight Modes does not hide the flight mode tab in the Companion Model editor. I'm not sure this is necessary (or how to do it).

Nope, not at all. Radio properties is more than enough. Damn, you're on fire with this one! :)

Can I ask what the justification was for being able to turn the Mixes tab off... Just curious as to the rationale, as without that, it pretty much makes it impossible to set a model up.

Perhaps a box around the View options controls, just to show they're all related, like with the sections above?

@gagarinlg
Copy link
Member

Can I ask what the justification was for being able to turn the Mixes tab off... Just curious as to the rationale, as without that, it pretty much makes it impossible to set a model up.

I was asking myself the same

@philmoz great work!

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 19, 2023

Can I ask what the justification was for being able to turn the Mixes tab off... Just curious as to the rationale, as without that, it pretty much makes it impossible to set a model up.

That would be me mis-interpreting what was written in issue #1692
Looking at it now 'Custom Mixers' probably means custom mix scripts, not mixes.

I'll take this one out :)

@elecpower
Copy link
Collaborator

@philmoz Companion suggestion to put new settings on their own tabs. If you agree I'm happy to do it for you.

Companion should take note of the settings as this has come up at times but less important since navigation is easier on a computer.

Best to leave any redesign work until after we get Qt 6 implemented and there are also other changes to support requested functionality.

@pfeerick
Copy link
Member

pfeerick commented Apr 19, 2023

Looking at it now 'Custom Mixers' probably means custom mix scripts, not mixes.

Yes, my apologies, it did mean custom mixer scripts. 😵‍💫 I was just thinking maybe there was a reason someone would want to turn the mixes off... but chop chop for now unless there is a good reason (at which point it can be added later).

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 19, 2023

Companion suggestion to put new settings on their own tabs. If you agree I'm happy to do it for you.

@elecpower - If you can make it prettier or easier to use go for it.

I've removed 'Mixes' from the list so this should be done (barring any bugs).

@pfeerick pfeerick added the enhancement ✨ New feature or request label Apr 19, 2023
@elecpower
Copy link
Collaborator

@philmoz see my branch https://github.com/elecpower/edgetx/tree/view-options for Companion Ui mods

In testing I noticed that Custom Mix Scripts is not shown on X9D+ or TX16S libsims. Should it also be removed from Companion for all radios?

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 20, 2023

see my branch https://github.com/elecpower/edgetx/tree/view-options for Companion Ui mods

Thanks, will take a look.

In testing I noticed that Custom Mix Scripts is not shown on X9D+ or TX16S libsims. Should it also be removed from Companion for all radios?

Custom mix scripts requires a custom radio build option; but I could not see any equivalent in Companion (like noheli or nogvars). I opted to leave it in; but I have no objection to removing it.

@robustini
Copy link
Contributor

It does not affect the tabs in the Companion radio or model editors. I.E. turning off Flight Modes does not hide the flight mode tab in the Companion Model editor. I'm not sure this is necessary (or how to do it).

Nope, not at all. Radio properties is more than enough. Damn, you're on fire with this one! :)

Can I ask what the justification was for being able to turn the Mixes tab off... Just curious as to the rationale, as without that, it pretty much makes it impossible to set a model up.

Perhaps a box around the View options controls, just to show they're all related, like with the sections above?

As far as I am concerned, a plausible reason might be to provide the tx to someone already well set up and not easily give him or her the chance to twist something that might, for example, cause problems in the flight of a model.
But in that case I would also take away their ability to be able to re-enable them via tx, I would leave it only in Companion.
For personal use I will never disable any tab.

@gagarinlg
Copy link
Member

Everything that has an impact should be visible or else you are searching for the cause of some strange behavior, but it is hidden away by a disabled tab.

@philmoz
Copy link
Collaborator Author

philmoz commented Apr 22, 2023

Latest commit uses the 'view options' flags to control other functionality:

  • Skips the 'Heli' mix logic is heli not enabled.
  • Excludes the Heli, GVAR and Telemetry values from source selection if not enabled.

(Really need a better name than 'View Options' now).

@rotorman
Copy link
Member

(Really need a better name than 'View Options' now).

How about "Visible tabs" or "Tabs visible:" ?

@gagarinlg
Copy link
Member

Since it now also disabled the functionalities, how about
"enabled features"?

@pfeerick pfeerick merged commit 5327c89 into EdgeTX:main May 22, 2023
37 checks passed
@ParkerEde
Copy link
Contributor

Can we also do the translations here?
DE:
#define TR_VIEW_OPTIONS "Aktivierte Menüpunkte"
#define TR_RADIO_MENU_TABS "Sender Menü-Punkte"
#define TR_MODEL_MENU_TABS "Modell Menü-Punkte"

@pfeerick
Copy link
Member

It's already merged, but I'll add any that are done here ;)

@ParkerEde
Copy link
Contributor

There is still a small problem in Companion. If I edit e.g. the fifth model in the list of models, click on the tab "Enabled Feature" and set e.g. "Heli" from "Global" to "On" under Model Menues, the focus jumps away from the selected model to the first model in the list.

@pfeerick
Copy link
Member

I don't believe that is related to this PR. If I go to Companion 2.8 (i.e. not even a recent 2.9), and make some change in the model settings there, focus is "lost" there also, so it seems to be a general Companion behaviour when you work in another modal dialog.

@ParkerEde
Copy link
Contributor

I don't believe that is related to this PR. If I go to Companion 2.8 (i.e. not even a recent 2.9), and make some change in the model settings there, focus is "lost" there also, so it seems to be a general Companion behaviour when you work in another modal dialog.

Ah, okay thank you.

@froqstar
Copy link

Can we also do the translations here?
DE:
#define TR_VIEW_OPTIONS "Aktivierte Menüpunkte"
#define TR_RADIO_MENU_TABS "Sender Menü-Punkte"
#define TR_MODEL_MENU_TABS "Modell Menü-Punkte"

Why the dashes in the last two, and no dashes in the first one? Including them looks wrong to me, so I prefer the first option.
Make some use of the famous arbitrary-length german words 😜

@ParkerEde
Copy link
Contributor

DE:
#define TR_VIEW_OPTIONS "Aktivierte Menüpunkte"
#define TR_RADIO_MENU_TABS "Sender Menüpunkte"
#define TR_MODEL_MENU_TABS "Modell Menüpunkte"

@ParkerEde
Copy link
Contributor

@pfeerick
The button text for TR_VIEW_OPTIONS is much too long. This is the best way I think:
DE:
#define TR_VIEW_OPTIONS "Menüpunkte"
#define TR_RADIO_MENU_TABS "Sender Menüpunkte"
#define TR_MODEL_MENU_TABS "Modell Menüpunkte"

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