TauLabs / TauLabs

taulabs.org
Other
455 stars 393 forks source link

Refactor GCS mixer tab #299

Open kubark42 opened 11 years ago

kubark42 commented 11 years ago

The mixer is a bit of an unloved beast. Its layout is confusing to uneducated users, even though its functionality is very easy to intuitively understand. It's effectively a simple spreadsheet which multiplies the inputs on the left by the coefficient in the table and then sums the column, with the output driving the designated channel.

Screen Shot 2013-02-09 at 11 50 26 AM

In this instance, the value at Curve 1 gets multiplied by 127 and then sent to Channel 1. Channel 1 is of type motor, which imposes additional saturation behavior on the value, but that's conceptually a different idea.

Likewise, Channel 2 takes the roll value, multiplies it by 127 and assigns it to output Channel 2. This channel is configured as a servo, which has different behavior from the motor but that again is outside the role of the mixer.

So the mixer can be graphically nudged around to make it clearer what its goal is. Since we're working with column sums, it would graphically make more sense for the sink to be at the bottom instead of the top. Items on the top and left look like sources, items on the bottom and right look like sinks. It might be better to orient this table in a vertical sense, where an individual channel is read across, instead of down. That would leave more space to expand the two ideas that 1) we have a mathematical value assigned to that Channel number and 2) that channel number is of a certain type.

At the same time, I would suggest remapping the -128 to 127 to -1 to 1 (or -100% to 100%), so that it's clearer to the user what's happening, since most users won't recognize the relationship between 127 and an 8-bit binary overflow. Of course the actual mixer UAVO values wouldn't be changed, just the presentation in the table. This reflects the firmware code, where the individual channels are normalized by 128.

elafargue commented 11 years ago

You know what, you should copy this post to the wiki, it's a good explanation!

On Sat, Feb 9, 2013 at 3:06 AM, Kenn Sebesta notifications@github.comwrote:

The mixer is a bit of an unloved beast. Its layout is confusing to uneducated users, even though its functionality is very easy to intuitively understand. It's effectively a simple spreadsheet which multiplies the inputs on the left by the coefficient in the table and then sums the column, with the output driving the designated channel.

[image: Screen Shot 2013-02-09 at 11 50 26 AM]https://f.cloud.github.com/assets/1118185/141982/88105920-72a6-11e2-9a23-17f7bdaf0d19.png

In this instance, the value at Curve 1 gets multiplied by 127 and then sent to Channel 1. Channel 1 is of type motor, which imposes additional saturation behavior on the value, but that's conceptually a different idea.

Likewise, Channel 2 takes the roll value, multiplies it by 127 and assigns it to output Channel 2. This channel is configured as a servo, which has different behavior from the motor but that again is outside the role of the mixer.

So the mixer can be graphically nudged around to make it clearer what its goal is. Since we're working with column sums, it would graphically make more sense for the sink to be at the bottom instead of the top. Items on the top and left look like sources, items on the bottom and right look like sinks. It might be better to orient this table in a vertical sense, where an individual channel is read across, instead of down. That would leave more space to expand the two ideas that 1) we have a mathematical value assigned to that Channel number and 2) that channel number is of a certain type.

At the same time, I would suggest remapping the -128 to 127 to -1 to 1 (or -100% to 100%), so that it's clearer to the user what's happening, since most users won't recognize the relationship between 127 and an 8-bit binary overflow. Of course the actual mixer UAVO values wouldn't be changed, just the presentation in the table. This reflects the firmware code, where the individual channels are normalized by 128.

— Reply to this email directly or view it on GitHubhttps://github.com/TauLabs/TauLabs/issues/299..

kubark42 commented 11 years ago

I've been playing around a bit with ideas and have hit upon the idea that the table could be transposed, and we could use diagonal headers to save space. This is a really ugly mockup done with LibreOffice, but it shows the idea:

screen shot 2013-09-05 at 4 28 57 pm

There is a row live view and a column live view. The user can see the mixer inputs (roll, pitch, yaw, etc...) and the output that's going to the channel.

We could do this pretty easily by subclassing the horizontal header paint function.

KipK commented 11 years ago

I'm agree the custom mixer could be remodeled to be clearer for the new users. The problem is I often use the mixer for other purpose ( that confirm your statement above: "but that again is outside the role of the mixer." ). Some Rx like 4 channel Frsky have only few pins but can still output 8 channels ppm.

I'm remapping some channels of my radioto free pwm outputs for different usage:

So I use the custom mixer to relay Accessory 1, 2 & 3 to some pwm pins. This is more a hack than a normal feature. You have to select a category in the drop down menu & then affecting it to Curve 2. If you already need Curve 2 for another purpose, you're pretty screwed. And for newcomers, it's perhaps a bit frightening to get in this area only to remap some channels or change some values in the default mixer.

Here's what I think could be a good way to achieve a better UI experience on this:

I feel strange that we have to quit a specific Vehicle Type to access the mixer. Mixer should be available from a preselected type. Why not reducing a bit the top part ( with the Frame Type visual / Mix Level/ Throttle Curve , they're big babes ), removing the Motor Output Channel part belows, and replacing it by a light mixer table with only needed motors, you select the motor output channels, then throttle , pitch, roll &yaw are prefilled. I'm not fan of the diagonals area, not really ergonomic ( I haven't felt it evident when I saw it first, should be a sign ;) )

A simple table like this, should fit:

Alt text

Instead of numerical values for curves, their name could be display instead , "Throttle" in this case.

a simple table like this too. Clear, simple, effective. No curves defined means linear output.

Alt text

Something like this :

Alt text

Curve selection could also be used here. ( ie for Pitch gimbal curve for ex. ).

kubark42 commented 11 years ago

I think I see what you're getting at, and like the idea a lot. Correct me if this isn't building on what you're suggesting:

For additional curves, see https://github.com/TauLabs/TauLabs/issues/307. If the curve values were replaced by int8 instead of floats, we could have 8 mixers for the current price of 2. So this is completely reasonable, and we could simply have a named mixer for the four primary channels and then have four generic mixers for all the other channels.

P.S. I use the custom mixer and then have to sneak into a UAVO to reclassify my airframe as an airplane, instead of a custom model. And, yes, you're screwed when you need other inputs. Named channels here are an absolute necessity.

kubark42 commented 11 years ago

I'm not fan of the diagonals area, not really ergonomic ( I haven't felt it evident when I saw it first, should be a sign ;) )

I think that was because of the way I drew it. Diagonals are common in tables.

Here's an update that looks much closer to what I was thinking. (Thank god for OmniGraffle, it made this so much easier.)

With the true UAVO values: screen shot 2013-09-07 at 8 16 19 am

With the UAVO values scaled to percentages: screen shot 2013-09-07 at 8 36 15 am

You've certainly noticed that there are a lot of new columns here. I'm foreseeing a day when we finally add support for interesting named channels, so they will all have to show up in the master table. The only way I have found to do that compactly is with diagonals.

KipK commented 11 years ago

Yes you get the idea. Instead I was more thinking of moving light/flaps/spoilers/other switches out of the motor mixer. It looks more logical to me to set this in RC Input, but that's just a detail.

kubark42 commented 11 years ago

At some point we should represent the entire mixer, so that the entire effect of all inputs on an actuator can be visible. For instance, certain inputs like flaps and spoilers will have an effect on the pitch so the mixer is where you would want to tune that. The mixer isn't aware of the difference between the different actuator types, it's just a simple b = A*x multiplication.

In my mind, we would present all these options only on the custom mixer tab. I don't know why a user would want to link lights with the throttle, but here's where he could do it! Or maybe we could start the custom mixer tab with no columns at all, and the user can pick what columns he would like.

Do you find the table a little more ergonomic now that it's better presented? It's basically an extension of the table you presented, only I included a live-view so that users can have direct feedback on their settings.

KipK commented 11 years ago

Do you find the table a little more ergonomic now that it's better presented? It's basically an extension of the table you presented, only I included a live-view so that users can have direct feedback on their settings.

Yeah looks clear. Just Curve 1 Live view is confusing & useless I think. Keeping the output live view looks sufficient. Is it really needed ? I'd use percentage too, looks more intuitive.

Why not like I've proposed using a throttle column, and a Curve selection instead of giving throttle percentage in the curve directly ? Then when we will have more curves it won't need new columns too.

In my mind, we would present all these options only on the custom mixer tab. I don't know why a user would >want to link lights with the throttle, but here's where he could do it.

Ok, then I'd give access to those custom things on the mixer tab present on every page too. ie. in Quad X you have already 4 motors preconfigured, ( default output could be prefilled too, clearly noob won't change their pin mapping but just following provided doc/tutos ) . Then you can add some lines for needed addons or custom motor servo configuration too ( without having to go in Custom vehicle page )

In Custom page, then like you said, it's empty by default & you add your own lines. This seems a proper way to do it. Like it's presented in the GCS for now "Custom" means more custom vehicle than custom mixer. & I would keep it that way, this is a really nice feature to custom a vehicle without touching the code.

kubark42 commented 11 years ago

I agree that Curve 1 should be replaced by "Throttle". At first, I just grabbed the current mixer inputs and added a few new ones, but this time I've made a mockup taking your thoughts into account. The Curve # cells are supposed to be comboboxes, but I didn't have this possibility with OmniGraffle, so I just used a linear blend fill to evoke the effect.

screen shot 2013-09-07 at 1 23 33 pm

I think you understood perfectly my understanding of what you wrote. ;) I like where this is going. By doing what you suggest, users will never have to go to the custom vehicle tab unless they want to make something truly custom. Otherwise, they'll just add new fields to the local mixer.

kubark42 commented 11 years ago

Actually, Curve # could be replaced by Curve type, with elements "Linear", "expo", "Custom 1", etc...

KipK commented 11 years ago

expo would need another value , Wouldn't it be better to keep the curve type where we will set/add different curves.

KipK commented 11 years ago

and no curve selected would means linear of course

kubark42 commented 11 years ago

Yeah, so maybe it's only "normal" or "custom 1", "custom 2", ... which would work without forcing the user to use the curve tab. And we should probably allow users to rename the custom curves, so they can call them "expo" if they want to.

kubark42 commented 11 years ago

Actually, thinking this through a little more, this could be a really bizarre place for the curves. Remember that the mixer is not a TX-->Actuator mixer, but a StabilizationDesired-->Actuator mixer. So this means that you can set up non-linearities in the actuator response to a linear PID. That could be really powerful, or really bad. I have to think through what that means, and whether the user is likely to get it horribly wrong. This might be improved by better labeling, so that the obvious mistake of linking "Tx Roll" with "Stabilized roll" isn't so easy.

KipK commented 11 years ago

Actually what I was thinking is using those curves in the Input window for the expo things. I don't know what's the most effective way to display this, but It could be in the "Confgure each stabilisation for each axis ". We could have 2 columns per axis, one for stab, the other one for curves.

  Pitch Roll Yaw
Mode Curve Mode Curve Mode Curve
Stab 1 Attitude Normal Attitude Normal Rate Normal
Stab 2 Rate Curve 2 Rate Curve 2 Rate Normal
Stab 3 VBar Curve 2 VBar Curve 2 Rate Normal
kubark42 commented 11 years ago

Got it. It should be workable for the GUI, but since it's new functionality it's a little hard to be sure of what the final product will need to look like.

What we've described in this conversation is flexible enough to work both for the existing approach (Curve 1/Curve 2) and the approach with many custom curves. If you agree, we can decouple the table UX from the firmware changes. This is valuable because the table can be done over the weekend, whereas the firmware changes will take a significant time to architect and test.

KipK commented 11 years ago

That's the goal yes we can keep actual curve structure for now in the new table system, but it's already ready for curves extension. I like it. :+1:

kubark42 commented 11 years ago

In the interests of keeping things simple, how does this look for a first step?

screen shot 2013-09-08 at 1 10 15 pm

It's the same cell layout, although the more compact diagonal features aren't present. This isn't a large concern right now, because we're not running out of space (yet). I have started work on the diagonal elements, but there's a lot of repainting that has to go on so it will take some time to get it finished and presentable.

The only real concern I have is the white space in the upper left. I could fill that with a couple arrows, though, in order to make it very clear what's being referred to.

kubark42 commented 11 years ago

Something like this: screen shot 2013-09-08 at 1 17 30 pm

EDIT: Or like this, with two "down" arrows: screen shot 2013-09-08 at 1 18 45 pm

KipK commented 11 years ago

Standard ergonomic rules would be more like this so

Alt text

kubark42 commented 11 years ago

This is better than what I put, but the issue is that Qt doesn't handle multi-row headers gracefully. What I suggested can be done in just a few minutes, although I definitely see the advantage to yours. Perhaps I can figure out some way to subclass or combine multiple tables for the same look. I did something similar on the TelemetryScheduler gadget, so it's definitely possible, although for more effort.

kubark42 commented 11 years ago

I realized there's a bit of an issue with the "blue-only" format, which is that by necessity I can only use the word "Live view". I was thinking about naming one "Live input [%]" and the other "Live output [%]".

Here's a mockup with hierarchal headers: screen shot 2013-09-08 at 2 36 14 pm

I think that this could be a good way to do it. The diagonal text is short enough that I might avoid the pesky paint problems I'm getting with Qt where drawing across both the table and the header is proving difficult.