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

Heli wizard added #92

Merged
merged 41 commits into from
Feb 23, 2023
Merged

Heli wizard added #92

merged 41 commits into from
Feb 23, 2023

Conversation

jrwieland
Copy link
Contributor

c480x272

  • Heli wizard added
  • Wizard loader updated to include heli
  • Helicopter images uploaded to corresponding folder

bw128x64

  • Heli wizard added
  • Wizard loader updated to include heli

bw212x64

  • Heli wizard added
  • Wizard loader updated to include heli
  • Wizard image folders modified to match color screen structure

@offer-shmuely
Copy link
Contributor

Hi
it is a very nice one :-)

some things to think about.

  1. do we really need to support the FLY-BAR? does it still popular? around me, all have flight-controller (aka: flybarless)
  2. the channel5 maybe better called "T.Gain" instead of GYRO, since on modern flight-controller you can have cyclic-gain as well
  3. the default of throttle can be "flat" for all 3 mode? since most friends use governor anyway, so you do not need a graph, but just weight=0 + offset=70%
  4. announce for bank0/bank1/back2 will be nice
  5. I put the T.Gain on trim T5, it is very useful, (including the appropriate voices)
  6. It will be nice to name the channel on the output page

Great work!

@pfeerick
Copy link
Member

do we really need to support the FLY-BAR? does it still popular? around me, all have flight-controller (aka: flybarless)

Most definitely. There are still a lot of heli flyers with older helis. Plus, there's the whole choice thing ;)

I put the T.Gain on trim T5

Might just have to be wary of transmitters that don't have a T5

I'll be back to look at this properly in a few days... please feel free to ping me if that is longer than a week ;)

@jrwieland
Copy link
Contributor Author

@offer-shmuely
Thank you for the feedback, I apologize for the delay in responding I had a critical project for work that was taking all of my attention/energy.

One thing to consider with all of the wizards. They are designed to be used by new users to EdgeTX and load a basic setup for that particular model type, and get them in the air. Most experienced users will utilize their own template or copy an existing model as a starting point rather than starting from scratch using a wizard.

I thought about making the initial selection for the curves" FLAT" for all ide ups (since that is what i run on all my FM's) and the modern ESC's spool much slower than the old word counterparts. as Peter said it comes down to choice. Do I put this one or that one first. Do I impose my preferences or make them decide by the order of the curves.

Utilizing "T.Gain" instead of GYRO is a good idea, I used gyro due to its common use with other radio systems.

Using the term "banks" tells me you use a "V" or kbar. Having announcements pre-programmed takes away from the "Customizable aspect". We all came to open source due to the fact that we could make it "Your radio , your way". The more that is added to the wizards makes it more and more like futaba, spectrum etc. Doing the simple tasks of setting announcements/warnings when something is changed is the initial steps to fully customizing it IMHO.

Adding the name to the outputs is another good idea. Thank you.
I'll make those changes this weekend

@offer-shmuely
Copy link
Contributor

Thanks for good work
I am using ikon2 & spirit2
and planning to give rotorflight a chance sometime

@jrwieland
Copy link
Contributor Author

@pfeerick
for uniformity should I add output names to the other wizards?

Copy link
Member

@pfeerick pfeerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Needed to make a minor change as the colorlcd version was altering system TEXT_COLOR, but otherwise looks good.

To make the wizard more robust, since setSwashRing is only available for HELI firmware, it might be good to somehow check if that is available so it doesn't crash if the wizard is used to create a FB model when setSwashRing is present. But that is food for a future revision.

edit: I just saw your question about output names, I'll do that as I need to check something else in the wizards anyway...

@pfeerick pfeerick merged commit 6373a8f into EdgeTX:master Feb 23, 2023
@pfeerick pfeerick linked an issue Feb 23, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wizard Heli EdtgeTx 2.8.0
3 participants