hd-zero / hdzero-goggle

MIT License
261 stars 76 forks source link

Quickswap sources using the Input menu options #423

Closed sanderpuh closed 3 months ago

sanderpuh commented 5 months ago

@Master92 did quite a lot of coding already, I took inspiration from his work which allows cycling through the source options. On top of that I've added the option to choose to cycle only between HDZero and the expansion module, which is most useful on races where people use both systems, and for people flying lots of whoops with both systems.

Added Toggle Mode I've added Toggle Mode to the Auto Scan screen, because this seemed the most logical to me. Open to suggestions! The Cycle setting does cycle through all source options, the Race setting switches between the expansion module and HDZero. In case Race is selected, and anything other than the expansion module or HDZero is active, it will switch to HDZero when the button is pressed to switch sources.

Added Toggle source The dropdown menu on the Input screen now has the option Toggle source added to the list, in this case I've set it on the Right long: button (not as default).

The code also takes into account that the expansion module can be switched off automatically, which is taken care of in the quickswap too.

sanderpuh commented 5 months ago

411 was already closed, but not implemented yet. This PR takes care of issue #411.

Master92 commented 5 months ago

Would've been nice if you left me as the author of the code I wrote by cherry picking from my branch properly.

The changes, however look nice, good job. A little fronted documentation of what race mode means would be appreciated by many users, I think.

Also, squashing equally named commits would serve comprehensibility for the future imho 😉

DarthCedius commented 5 months ago

Nice, thanks, how can I test it?

sanderpuh commented 5 months ago

Would've been nice if you left me as the author of the code I wrote by cherry picking from my branch properly.

The changes, however look nice, good job. A little fronted documentation of what race mode means would be appreciated by many users, I think.

Also, squashing equally named commits would serve comprehensibility for the future imho 😉

Thanks for the tip, still new to using GitHub, will keep them in mind! Frontend documentation in the goggles GUI you mean? Other option would be to drop the config in the Auto Scan and add 2 options in the drop-down Input, one for Cycle sources and one for Toggle HD - Analog.

sanderpuh commented 5 months ago

Nice, thanks, how can I test it?

GitHub does an automatic build to check the code. I think you could use that build to test, can be found under the Checks tab: https://github.com/hd-zero/hdzero-goggle/actions/runs/9419103738/artifacts/1579597591

sanderpuh commented 5 months ago

Nice, thanks, how can I test it?

@DarthCedius I've made a few more changes, settings weren't stored properly when changing the setting from Cycle to Race. Did you try to update using the link I provided? There's a new build made by Github based on the latest fixes: https://github.com/hd-zero/hdzero-goggle/actions/runs/9428376367/artifacts/1581709527

nerdCopter commented 5 months ago

@sanderpuh , you could attribute @Master92 with commit "co-author" as well: https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors. cherry-pick or branching from his branch could have both worked as well. I will hide this comment as off-subject after posting. :four_leaf_clover:

nerdCopter commented 5 months ago

Pilots may not associate the terminology Toggle Mode with Toggle Source function. wondering if they should be the same naming scheme. :man_shrugging:

sanderpuh commented 5 months ago

@nerdCopter Agreed, naming scheme should be in line.

Regarding the two options for implementation, I'm fine with either. Adding it to the inputs menu keeps everything in one page, which would keep things clean IMO. Cycle sources and Toggle sources could be the two added options.

Since the Inputs menu is new, we can add a description for each option, including these two.

Who can be the deciding factor in choosing which path to follow for implementation?

nerdCopter commented 5 months ago

[...] Cycle sources and Toggle sources could be the two added options.

i like that, albeit i'm "nobody" to decide.

Who can be the deciding factor in choosing which path to follow for implementation?

other interested community members or @ligenxxxx (or yourself :wink:) .

DarthCedius commented 5 months ago

Nice, thanks, how can I test it?

@DarthCedius I've made a few more changes, settings weren't stored properly when changing the setting from Cycle to Race. Did you try to update using the link I provided? There's a new build made by Github based on the latest fixes: https://github.com/hd-zero/hdzero-goggle/actions/runs/9428376367/artifacts/1581709527

it's working great thanks, maybe you should add a temporary OSD info to state which input you are on.

sanderpuh commented 5 months ago

Nice, thanks, how can I test it?

@DarthCedius I've made a few more changes, settings weren't stored properly when changing the setting from Cycle to Race. Did you try to update using the link I provided? There's a new build made by Github based on the latest fixes: https://github.com/hd-zero/hdzero-goggle/actions/runs/9428376367/artifacts/1581709527

it's working great thanks, maybe you should add a temporary OSD info to state which input you are on.

@DarthCedius Thank you for your constructive feedback! Temporary OSD info would be a nice to have when the cycle option is selected, I agree. I'm not sure if I can implement that myself though, I'll have a look.

sanderpuh commented 5 months ago

[...] Cycle sources and Toggle sources could be the two added options.

i like that, albeit i'm "nobody" to decide.

Who can be the deciding factor in choosing which path to follow for implementation?

other interested community members or @ligenxxxx (or yourself 😉) .

The more I think of it, the more I like that option too. Keeps the OSD menu cleaner and both options are integrated on one page. @ligenxxxx do you agree? If yes, I'll rework the code to remove the config in the Auto Scan page and add two options to the dropdown list on the Inputs page, Cycle sources and Toggle sources.

nerdCopter commented 5 months ago

@sanderpuh , i'm happy to test after commit/push on weekends. i don't have any hdmi to test that part though.

sanderpuh commented 5 months ago

@sanderpuh , i'm happy to test after commit/push on weekends. i don't have any hdmi to test that part though.

Current commit seems to be working well, been using it for a while now on my own goggles without any issues. I've also implemented a beep on my end when sources are swapped as an audible indicator.

That being said, I'll rework the code to have 2 more options on the inputs page, one will be 'cycle sources' and the other will be 'toggle sources'. The Toggle mode menu item is no longer necessary, and can be dropped. The function to swap sources will stay the same. I'll timeblock some time next week to rework the code.

After implementation of these two items, we could further enhance by implementing #395 from @qvasic - Add a beep option to the OSD menu with setting Off, Boot and Source - which will enable the end user to not use the beep at all, use it to indicate booting has finished, or use it to indicate the source is switched and booting has finished.

sanderpuh commented 3 months ago

This PR can be closed when PR #433 is merged.