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(el18): Use trim hats as keys for navigation #3894

Merged
merged 35 commits into from
Sep 29, 2023

Conversation

mha1
Copy link
Contributor

@mha1 mha1 commented Aug 2, 2023

Fixes #3202

Summary of changes:

  • implemented navigation by trims (hats) on NV14/EL18
  • specification by @JimB40

NV14-EL18_hats_modes

@elecpower
Copy link
Collaborator

Hi @mha1 thankyou for coding the Companion side and it works however you have followed a method for the ui I have slowly been replacing.

So rather than me try to explain and point you at multiple code snippets to cover your change, do you have any issue with me knocking up a branch that you can review and merge back into yours?

@mha1
Copy link
Contributor Author

mha1 commented Aug 2, 2023

nope, have at it.

@mha1
Copy link
Contributor Author

mha1 commented Aug 3, 2023

Neil, you might want to wait a bit. There was some UI discussion started, see #3202

@elecpower
Copy link
Collaborator

Michael, noted though I'll do the common parts and pause until the discussion reaches a conclusion

@elecpower
Copy link
Collaborator

@mha1 here is my version of cpn side for consideration and merging onto yours. Note: mine is branched from yours.
https://github.com/EdgeTX/edgetx/tree/elecpower/cpn-nv14-trims-as-keys

@mha1
Copy link
Contributor Author

mha1 commented Aug 3, 2023

First go at model level hats mode selection (overriding radio setting).

@mha1
Copy link
Contributor Author

mha1 commented Aug 3, 2023

@elecpower Thank you very much Neil. I merged your changes, checks are running. Depending on how well accepted the model level selection of hats mode in the Trims sub menu is I might ask for the favor to implement this also in Companion. Under the Setup tab just before the Trim step field:
Hats mode: <options: Trims only, Keys only, Switchable, Global> with yaml representation (0..3).

@elecpower
Copy link
Collaborator

@mha1 I updated my branch with your latest and added model level setting code on my branch again.
Note: to be consistent with the new way of encoding and decoding yaml values I have mapped the enum to/from text. Companion side only. Feel free to change the text I chose but ensure identical on both sides.

@mha1
Copy link
Contributor Author

mha1 commented Aug 4, 2023

@elecpower Thanks, merged and harmonized radio and companion yaml tags and id's

@pfeerick @JimB40 please let me know if you came to an agreement were best to put the hats mode model level settings. after ADC filter or in Trims doesn't make a difference for me. I believe it fits the Trims head line pretty well but don't mind shifting it one level up. Just let me know.

@JimB40
Copy link
Collaborator

JimB40 commented Aug 4, 2023

For me either Peter decides.

@elecpower
Copy link
Collaborator

@mha1 Michael I would strongly recommend adding back the HATS_ prefix to then enums as there are other MODEs already and likely in the future. Having an enum of MODE_GLOBAL when it is not global is sure to mislead and catch someone out. The yaml text is in context of the label so I have no issue with hatsMode: GLOBAL

@philmoz
Copy link
Collaborator

philmoz commented Aug 5, 2023

PR #3903 adds the bubble popup you were asking about on Discord.

@pfeerick
Copy link
Member

pfeerick commented Aug 5, 2023

in Trims

Is fine... I hadn't considered the "trims off means navigation only" path Robert mentioned earlier, so if trims get disabled as a radio or model level feature, that option will implicitly get forced to "keys only", so you don't need access to the option anyway.

@mha1
Copy link
Contributor Author

mha1 commented Aug 5, 2023

@philmoz This is so much better. Thank you very much! I merged your PR and unfortunately pushed. Do you want me to revert this commit and wait for your PR to be merged?

@philmoz
Copy link
Collaborator

philmoz commented Aug 5, 2023

Thank you very much! I merged your PR and unfortunately pushed. Do you want me to revert this commit and wait for your PR to be merged?

No problem - better to get it tested anyway. We can sort it out when the PR's get merged.

@mha1
Copy link
Contributor Author

mha1 commented Aug 5, 2023

tried it on your el18?

@philmoz
Copy link
Collaborator

philmoz commented Aug 5, 2023

tried it on your el18?

The build from this PR works on my EL18.

I would suggest that the default for the model level setting should be Global (like enabled features).

If you look at the 'Enabled Features' model settings, I display the current radio level setting when the model is set to global to let users know the state. Might be nice to add this as well.

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

Thank you for testing and your suggestion, makes sense. Will discuss with @JimB40

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

Added hats mode defaults:

  • radio settings: Trims only (inherently set to Trims only by memclear)
  • model settings: Global

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

Dear Translators,

I'd very much appreciate your support for a new NV14/EL18 feature.

FlySky NV14/EL18 radios have two little joy sticks usually used as trims. Those joysticks are called "hats". This PR adds logic to make the hats available as keys for navigating the GUI. The hats can be moved up, down, left, right and pressed. "Hats mode" refers to the state of the hats. In "Trims mode" the hats act as trims, in "Keys mode" they act as Keys according to the key map shown in #3894 (comment). Switching between the two hats modes is possible. Option "Switchable" refers to a users setting that allows switching between trims mode and hats mode.

Don't hesitate to ask if you have any question.

ENG original text:
#define TR_HATSMODE "Hats mode"
#define TR_HATSOPT "Trims only","Keys only","Switchable","Global"
#define TR_HATSMODE_TRIMS "Hats mode: Trims"
#define TR_HATSMODE_KEYS "Hats mode: Keys"

Thanks,
Michael

@Pat6874
Copy link
Contributor

Pat6874 commented Aug 6, 2023 via email

@HThuren
Copy link
Contributor

HThuren commented Aug 6, 2023

Hi Michael, Can we understand 'hat' as a joystik ?
Then translation will be "Joystik only" or will that mix up with the 'Pins'

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

@HThuren Well, they are little joysticks and I suppose the are called hats because the sticks grow wider at the top. To me the little joystick sticks look a bit like tiny mushrooms. I'll leave it up to you. Call them whatever makes sense in your language. Maybe calling them joystick might collide with the USB mode where the radio can act as Joystick when connected to a PC.

@Eldenroot
Copy link
Contributor

I am thinking about to use “joysticks” word for these "hats".

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

I think calling the hats Joystick will be confusing. There is a USB mode Joystick and the Hats mode settings in radio settings is right below the USB mode settings.

image
image

@Eldenroot
Copy link
Contributor

Well, maybe joystick USB should be renamed then... because hats mode is more confusing :)

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

We don't want to open this can of worms, do we? Hats is a NV14/EL18 thing only. By calling the hats hats (I understand this is a real term for the NV14/EL18) you confuse NV14/EL18 owners only (if at all).

@mha1
Copy link
Contributor Author

mha1 commented Aug 6, 2023

Oh, and don't worry to much, the Hats mode setting is visible on NV14/EL18 radios only.

pfeerick pushed a commit that referenced this pull request Nov 7, 2023
pfeerick pushed a commit that referenced this pull request Nov 7, 2023
@pfeerick pfeerick mentioned this pull request Mar 11, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ✨ New feature or request rn: feature Feature to be highlighted in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OS wide key emulation for NV14/EL18