EdgeTX / edgetx

EdgeTX is the cutting edge open source firmware for your R/C radio
https://edgetx.org
GNU General Public License v2.0
1.58k stars 333 forks source link

"Outputs" tab changes: missing trims => subtrims, copy/add subtrims phrasing #354

Closed weixelgeist closed 2 years ago

weixelgeist commented 3 years ago

EdgeTX 2.4.0 RC3 Jumper T18

copy trims to subtrims does not work

raphaelcoeffic commented 3 years ago

@weixelgeist what do you mean by that? If works exactly the same way as OTX does, as the exact same function is used (b/w targets and Color LCD). The menu item "Copy trims to subtrim" is really the same.

Or is that the button at the end of all outputs that you are missing?

weixelgeist commented 3 years ago

at my T18 it does nothing if I go back to the main screen the trim indicators are at the same place like bevor. they are not in the middle position

pfeerick commented 3 years ago

How do you get to it on the colour screens? I have it at the bottom of the outputs tab on both the X9D+ and T-Lite but not on the TX16S

Edit: Scratch that, I see it's in the individual outputs entry menus. In that case, it applied the subtrim, and left the trims where they were, rather than re-centre the tims

But it doesn't seem to work the first time it's done. I could be imagining things though, need a second opinion ;) I think I was managing to do the wrong channels, it seems to be working. Just that the trim doesn't reset like with other txs

raphaelcoeffic commented 3 years ago

How do you get to it on the colour screens? I have it at the bottom of the outputs tab on both the X9D+ and T-Lite but not on the TX16S

The button at the end of all outputs is not there, that's true, but there is a menu item which does it "channel by channel", without reseting the trim for that channel itself, which is what OTX has (I think). So what is actually missing is the button at the end. Or we can add it in the menu with "Copy All Trims to Subtrims".

weixelgeist commented 3 years ago

at the T18 it's missing at the bottom of the screen you will get a menu in each column if you press enter but the "Copy trims to subtrim" does nothing

raphaelcoeffic commented 3 years ago

at the T18 it's missing at the bottom of the screen you will get a menu in each column if you press enter but the "Copy trims to subtrim" does nothing

It does! Try it out: set trim for aileron to max in one direction, then go to the channel which outputs aileron, select the menu, you will see. It does not reset the trim position, but this is normal and intended (don't know why though, but OTX has always done it this way).

So what you miss is really just the button at the bottom of the page.

weixelgeist commented 3 years ago

It does! Try it out: set trim for aileron to max in one direction, then go to the channel which outputs aileron, select the menu, you will see.

Okay, this works.

It does not reset the trim position, but this is normal and intended (don't know why though, but OTX has always done it this way).

No, in OTX the trim position is reset to the middle position. It should be so, because that is the reason to copy the trims to subtrims

weixelgeist commented 3 years ago

Now I got it how it works in Edge: If you make Copy Trims to subtrims it writes the value into the subtrim the trim position I must reset by hand to the middle if I now do a new trim and Copy Trims to subtrims the new value is added to the value from bevor

But it would be much better if the trims are set automatically to the middle position

raphaelcoeffic commented 3 years ago

It does! Try it out: set trim for aileron to max in one direction, then go to the channel which outputs aileron, select the menu, you will see.

Okay, this works.

It does not reset the trim position, but this is normal and intended (don't know why though, but OTX has always done it this way).

No, in OTX the trim position is reset to the middle position. It should be so, because that is the reason to copy the trims to subtrims

This is only the case with the "All-in-one" button at the bottom of the page. Not saying we should not change it to be more intuitive, but really, this is the way OTX works. The "single channel copy trims to subtrims" function does not reset anything. This is best seen on the b/w targets where the UI has not changed.

weixelgeist commented 3 years ago

This is only the case with the "All-in-one" button at the bottom of the page. Not saying we should not change it to be more intuitive, but really, this is the way OTX works. The "single channel copy trims to subtrims" function does not reset anything. This is best seen on the b/w targets where the UI has not changed.

Okay, I never did the "single channel copy trims to subtrims" in OTX I always do the all channel menu But it would be great to get a menu for all channels copy trim to subtrims with automatically reset the trim position

raphaelcoeffic commented 3 years ago

This is only the case with the "All-in-one" button at the bottom of the page. Not saying we should not change it to be more intuitive, but really, this is the way OTX works. The "single channel copy trims to subtrims" function does not reset anything. This is best seen on the b/w targets where the UI has not changed.

Okay, I never did the "single channel copy trims to subtrims" in OTX But it would be great to get a menu for all channels copy trim to subtrims with automatically reset the trim position

I'm with you ;-) And frankly, I never understood why the function for single channel should not reset the trim, 100% agreed. But we cannot change everything at once ;-)

Now the question is: should we really have a button at the bottom of the page, or just add a menu item for "all channels", so that you don't have to scroll to the bottom of the page.

weixelgeist commented 3 years ago

Now the question is: should we really have a button at the bottom of the page, or just add a menu item for "all channels", so that you don't have to scroll to the bottom of the page.

it would be better to add a menu item for "all channels"

weixelgeist commented 3 years ago

I'm with you ;-) And frankly, I never understood why the function for single channel should not reset the trim, 100% agreed. But we cannot change everything at once ;-)

It would be a good enhancement to get this

pfeerick commented 3 years ago

Or put the button at the top ... Or is that too far out of the box? I sort of don't like context menus having not context specific entries.

raphaelcoeffic commented 3 years ago

Or put the button at the top ... Or is that too fat out of the box? I sort of don't like context menus having not context specific entries.

Let’s ask @JimB40 😇

weixelgeist commented 3 years ago

Or put the button at the top ... Or is that too fat out of the box? I sort of don't like context menus having not context specific entries.

Let’s ask @JimB40 😇

it only makes sense if it is in the menu

JimB40 commented 3 years ago

Never used 'Copy sticks to subtrim' & 'Copy trims to subtrim'. What exactly does each of this commands?

weixelgeist commented 3 years ago

Never used 'Copy sticks to subtrim' & 'Copy trims to subtrim'. What exactly does each of this commands?

the value of the sticks or the trims are wrote into each channel subtrim value the result is the plane is leveled and the trims are in the middle position

JimB40 commented 3 years ago

Hmm. Checked 'Copy trims to subtrim'

  1. CH1 Min = -100, Max = 100, Subtrim = 0
  2. Pushed CH1 trim left one notch.
  3. Issued 'Copy trims to subtrim' command Now CH1 Subtrim = -1.5
  4. Issued 'Copy trims to subtrim' command again Now Subtrim = -3.0
  5. Pushed CH1 trim right one notch. 'Trim centered' alert sounds
  6. Issued 'Copy trims to subtrim' command again Now CH1 Subtrim = -3.0
  7. Pushed CH1 trim right one notch.
  8. Issued 'Copy trims to subtrim' command again Now CH1 Subtrim = -1.6

So to my eyes this command every time adds value of Trim relative position to Subtrim variable Is that exactly how it should work?

Mantamatt1 commented 3 years ago

I second ALL trims to sub trims in a drop down menu

pfeerick commented 3 years ago

it only makes sense if it is in the menu

I'm curious as to why you think that... the menu provides options specific to the row you've highlighted (hence why it's a "context menu") ...so why does it only make sense to have a "Apply all trims to subtrims" type option in the context menu? I would have thought it would have been better at the top of the page as a button along with another one for the copy stick state to subtrims button. And perhaps a third to recenter the trims (or reset the trims as part of the applying subtrims button for consistency).

pfeerick commented 3 years ago

So to my eyes this command every time adds value of Trim relative position to Subtrim variable Is that exactly how it should work?

Pretty much. If you were nearly at the end of the trim range, you would 'subtrim' and could recenter the trim, getting the full trim range back again. The copy all option on the X9D+ would apply all the trims as subtrims, and recenter the trims in one operation.

JimB40 commented 3 years ago

Checked against OpenTX 2.3.13. On my XD9. Same behaviour.

Copy trims to subtrim command from pop-up menu copies selected channel trim to Subtrim. To copy all trims values to equivalent Channel Subtrim there is Trims => Subtrims command at the bottom. So UI naming is so confusing

Next. Copy trims to subtrim command from pop-up menu add trim value to Subtrim variable everytime is issued Trims => Subtrims command at the bottom just copy trims values to equivalent Subtrims variables .

What a mess. What is right and what is wrong? Give me User scenario so I understand how plane pilot acts and I'll propose good solution.

weixelgeist commented 3 years ago

Checked against OpenTX 2.3.13. On my XD9. Same behaviour.

Copy trims to subtrim command from pop-up menu copies selected channel trim to Subtrim. To copy all trims values to equivalent Channel Subtrim there is Trims => Subtrims command at the bottom. So UI naming is so confusing

Next. Copy trims to subtrim command from pop-up menu add trim value to Subtrim variable everytime is issued Trims => Subtrims command at the bottom just copy trims values to equivalent Subtrims variables .

What a mess. What is right and what is wrong? Give me User scenario so I understand how plane pilot acts and I'll propose good solution.

Tha main thing is that after copying the trims to the subtrims the position of the trim tabs MUST be set to the middle position.

This should work with all menu selections.

Mantamatt1 commented 3 years ago

Have it the same as otx. But possibly put it at the top of the page. I do use it every time I have maidens a new plane. I have never used sticks to sub trims? That's what insta trim is for lol

weixelgeist commented 3 years ago

Have it the same as otx. But possibly put it at the top of the page. I do use it every time I have maidens a new plane. I have never used sticks to sub trims? That's what insta trim is for lol

So do I

pfeerick commented 3 years ago

The only time I could see the sticks tovsubtrim being of use is if you had a plane on the table, and wanted to subtrim the surfaces to be 'level' in one hit, without going through trim -> subtrim, but other than that I don't see the point either.

Mantamatt1 commented 3 years ago

If it's on the table wouldn't you be doing mechanical trimming!

On Sun, 27 Jun 2021, 10:52 Peter Feerick, @.***> wrote:

The only time I could see the sticks tovsubtrim being of use is if you had a plane on the table, and wanted to subtrim the surfaces to be 'level' in one hit, without going through trim -> subtrim, but other than that I don't see the point either.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/issues/354#issuecomment-869134781, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUT54YCKC4YLPWXLPEKLAG3TU3YGVANCNFSM47MC5QPQ .

raphaelcoeffic commented 3 years ago

If it's on the table wouldn't you be doing mechanical trimming!

Depends on how lazy you are / or how easy this is.

JimB40 commented 3 years ago

If it's on the table wouldn't you be doing mechanical trimming! You mean sticks calibration?

Okay still I don't know why Trims => Subtrims copy trims value while Copy trims to subtrim adds value. First one seems to aggregate all channels Copy trims to subtrim commands, so shouldn't it work same way?

JimB40 commented 3 years ago

Okay start to get it.

  1. You maiden plane
  2. Trim it proprerly with trim switches
  3. Land and issue Copy trims to subtrim for CH1,CH2,CH3,CH4 etc
  4. Set all trims position to 0 Now if issue Copy trims to subtrim command again it won't change Subtrim as trim is set to 0 so nothing will be added.

If something wrong you can repeat procedure again starting from point 1 Am I right?

Mantamatt1 commented 3 years ago

In a nut shell... It needs to work the same as in otx b/w or colour makes no difference, with the exception of putting it at the beginning of the list not the end.

On Sun, 27 Jun 2021, 11:34 Robert, @.***> wrote:

Okay start to get it.

  1. You maiden plane
  2. Trim it proprerly with trim switches
  3. Land and issue Copy trims to subtrim for CH1,CH2,CH3,CH4 etc
  4. Set all trims position to 0 Now if issue Copy trims to subtrim command again it won't change Subtrim as it's set to 0 Am I right?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/issues/354#issuecomment-869139609, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUT54YDUB6YSJIEUQWHIBTDTU35DDANCNFSM47MC5QPQ .

JimB40 commented 3 years ago

Okay went through Trims => Subtrims use So... this command works issuing Copy trims to subtrim for all CH1,CH2,CH3,CH4 It is adding trim value to equivalent Subtrim variables in each of four channels

And I was fooled because:

  1. Copy trims to subtrim for one channel adds trim value and leaves trim value at set previously position
  2. Trims => Subtrims add trim values to equivalent Subtrim variables in CH1,CH2,CH3,CH4 and then set all trims values at 0

I think in terms of UX this should be unified. Either trims values are left and user has to bring them back to 0 manually or they are set to 0 after command.

Mantamatt1 commented 3 years ago

Set to 0 after command!!

On Sun, 27 Jun 2021, 12:11 Robert, @.***> wrote:

Okay went through Trims => Subtrims use So... this command works issuing Copy trims to subtrim for all CH1,CH2,CH3,CH4 It is adding trim value to equivalent Subtrim variable

And I was fooled because:

  1. Copy trims to subtrim for one channel adds trim value and leaves trim value at set previously position
  2. Trims => Subtrims add trim value to equivalent Subtrim variable and then set all trims value at 0

I think in terms of UX this should be unified. Either trims values are left and user has to bring them back to 0 manually or they are set to 0 after command.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/EdgeTX/edgetx/issues/354#issuecomment-869144154, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUT54YEUVS3G7PU3TCFEATDTU4BPFANCNFSM47MC5QPQ .

pfeerick commented 3 years ago

Trims => Subtrims

Exactly. Think of that one as move, it moves the all trims to subtrims, and since it is move, it resets the trims. The other is copy, so it only copies the trim for the highlighted channel to subtrim, and since it is copy, it doesn't change the trim.

JimB40 commented 3 years ago

Trims => Subtrims

Exactly. Think of that one as move, it moves the all trims to subtrims, and since it is move, it resets the trims. The other is copy, so it only copies the trim for the highlighted channel to subtrim, and since it is copy, it doesn't change the trim.

Okay so Trims => Subtrims I understand as a part of plane configuration procedure.

Then in what scenario Copy trims to subtrim command is used??? Maybe this is one of these functions "Looks cool but don't know what for" :) Use case guys. Otherwise "Less is more"

pfeerick commented 3 years ago

Oh, interesting... talking of UX... it says "copy trims to subtrim" in the context menu on the X9D+... but it only applies to the selected channel... bad OTX... it should be trim, not trims! 😆 At least they got the grammar right on the T-Lite 'cause they had to shorten the text.

And that last one... dunno what it actually does... the sentence overran menu... is the same in ETX and doesn't appear to actually do anything.

OTX 2.3.12, X9D+ Sim image

pfeerick commented 3 years ago

MikeB might be a good one to ask about the benefit of that copy trim to subtrim command, as it appears he specifically added it to ersky9x ... (copying it from OTX)

When you have trims on different flight modes, doing a copy trim (to sub trim) changes the trims on other flight modes to compensate.

https://openrcforums.com/forum/viewtopic.php?t=5010

JimB40 commented 3 years ago

Description should reflect what will happen as far as possible. Reset > Reset Copy trims to subtrim -> Add Trim to Subtrim Copy stricks to subtrim -> Add Stick to Subtrim Trims => Subtrim -> Add CH1-CH4 Trims to Subtrims Copy min/max/center to all outputs -> Replicate mix/max/center to all Channels

I know. Some of names will be truncated on 128x64 LCD but just to show difference.

Last one changes all channels min/max/center with values from selected channel

pfeerick commented 3 years ago

Be careful about assuming 4 trims... It would probably do 6 channels on transmitters like the TX16s if you are using the T5 and T6 trims also. I can certainly copy those trims to subtrims if assigned to channels the inputs for channels (i.e. channels 5 and 6). But generally I like that rephrasing / simplification.

Ah, yes, that last one does work... I thought it wasn't working at first. So it just needs the text shortening so it doesn't go off screen ;) And potentially also added to colour screens as it's present on the X9D+ and T-Lite, so presumably all B&W targets.

JimB40 commented 3 years ago

It will be as good as knowledge gathered :) I haven't used yet trims once.

What I use often for quads (and that was submitted few times) is setting min to -97.8% and max to 97.8% for CH1-CH4 There is no simple copy/paste functionality in Output Tab so this task is mudane.

JimB40 commented 3 years ago

To to summarize Output Tab changes for now

  1. Selected channel us visible on color radios
  2. Button the top of outputs list Add all Trims to Subtrims
  3. Context Menu a. check if Copy trims to subtrim is needed.

    • If not needed dump it
    • if neeeded change Copy trims to subtrim -> Add Trim to Subtrim
    • check if trim reset after Add Trim to Subtrim command is needed (if so code it)

    b. Change Copy sticks to subtrim -> Add Stick to Subtrim c. Add Copy/Paste functionality d. Check if Copy min/max/center to all outputs is needed

    • Change Copy min/max/center to all outputs -> Replicate mix/max/center
jmxp69 commented 3 years ago

Or put the button at the top ... Or is that too fat out of the box? I sort of don't like context menus having not context specific entries.

Let’s ask @JimB40 😇

I would be all for seeing the option at the top.

pfeerick commented 3 years ago

@JimB40 I would make that button for point 2 "Move all Trims to Subtrims" as part of the expected behaviour is that trims will be reset, so it's not just copying the trims like the other context entries do.

Mikey1968 commented 3 years ago

Also since were in the outputs page, is there a way like opentx , you can make the direction arrow change for reverse the servo and also make it so when we move a stick it shows which line were using to make the change

pfeerick commented 3 years ago

To keep this in perspective... we're talking about trying to keep a lot of the behaviour from this: OTX 2.3.13, TAER with 3POS on CH5 (columns are Channel, Name, Subtrim, Min, (arrow indicating channel up, mid or down) Max, Direction (arrow), Curve, PPM Center, Subtrim Mode (the UI tells you the top right as move through the fields which field is what)): image

where this is the current UI in ETX (channel, min, max, subtrim, PPM center, subtrim mode, with INV, CURVE and and Name shown on a second line if set with what looks like absolute positioning): image

i.e. One thing I'd like to see changed is the a symbol to indicate normal or revered servo, rather than INV, and some indication of the changing channel, as well as the spacing be more dynamic - i.e. there's all that space on the right for the name, but it's just not used if only a name is set...

image

JimB40 commented 3 years ago
  1. What was the idea behind using two lines instead of one leaving empty 1/3 of ui space on the right side?
  2. Changing data position is not UX recommended. We get used to look for it in certain place. Better to place kind of ‘empty marker’ when there is no value
pfeerick commented 3 years ago

I suspect it's because it's all absolute positioning... But I still don't know why that empty space wasn't used... It was possible to fit it all in before, maybe it should be done that way again , or at least make it two line from the start, with inactive/dimmed placeholders.

At present, given I used maximum length name deliberately, I see no reason that name and curve couldn't be on the single line, with a normal/reverse symbol, as well as channel input arrow... There is enough space.