Mixer still active, even Switch state should set if off; combined with Slow

[GoogleCodeExporter]

Which board (stock / gruvin9x / sky9x) are you using?
stock

What is your open9x FW version?
r1855

What is your open9x EEPROM version?
212-1

What steps will reproduce the problem?
1. First define a Curve (e.g. Cv1) and create a curve which is not 0.0 for Y at 
X 0.0. Complicated, I know here is an example:
Select a 3 point curve (not custom, just the simple one)
left Y value 0
middle Y value 12
right Y value 100

2. Goto Mixes and create a new mixer for a free channel (I choose CH05)
Enter the following parameters:
Source:   Thr
Weight:   100
Offset:   0
Curve/Diff:  Curve 1;  Diff 0 (Doesn't have an effect on Curve anyway?)
Include Dr/Expo: ticked
Phases:   All
Switch:   THR
Warning:  OFF
Multiplex:  ADD
Delay:  Up 0.0; Down 0.0
Slow:   Up 1.5; Down 1.5     (!) Important you set these
3. Start simulation in companion9x or return on main window on transmitter and 
make output channels visible

What is the expected output? What do you see instead?
I expect to see an output for CH05 of 0.0 if the Switch THR is not set and a 
value according the curve if THR is on
Actually I see the output for CH05 of 11.9 (nearly 12.0), when Switch THR is 
off. This value do not change whatever position you have with your throttle 
stick. When you switch THR on, the output changes according the positon of the 
stick. If you remove THR switch the channel changes back to 11.9 slowly instead 
of 0.0.

Please provide any additional information below.

This behaviour disappears if you set Slow to 0.0 in mixer parameters. So 
instead of slowing to 0.0 for the mixer output, what I expect it slows to 0.0 
for the input of the mixer and in case of my curve the output is not 0.0.
I am not sure if a similar bug exists if you set the Delay option. A quick 
check, shows it's mainly correct there, but had some crazy jumps up and down 
which I didn't expect, but unclear how to reproduce.

Original issue reported on code.google.com by open.20.fsguruh@xoxy.net on 2 Feb 2013 at 9:39

[GoogleCodeExporter]

Additional information about the bug:
If you play a little bit further with the parameters, it shows

Offset parameter in Mix is ignored when switch is off
Weigt parameter in Mix is still active when switch if off

In provided example, do following: 
Activate THR switch and put throttle to max quickly. 
CH05 raises slowly to 100.0 value.
Move throttle stick quickly down to minimum (-100).
CH05 slows down to 12.0 quick fast as well, and then very slowly down to 0.0 
because of the curve. 
This makes sense, because you can influence the speed of the output with 
defined curve. However a disabled mix should not create a value, maybe this is 
where it comes from.

Original comment by open.20.fsguruh@xoxy.net on 2 Feb 2013 at 9:50

[GoogleCodeExporter]

Interesting! 

Original comment by bson...@gmail.com on 2 Feb 2013 at 12:38

• Changed state: Accepted

[GoogleCodeExporter]

Differential should be grayed (and 0) when a curve is active, these 2 
parameters are excluding each other. In other words, a differential is a curve.

Original comment by bson...@gmail.com on 2 Feb 2013 at 12:40

[GoogleCodeExporter]

Original comment by bson...@gmail.com on 3 Feb 2013 at 4:25

• Changed state: Fixed

[GoogleCodeExporter]

Fixed in revision 1908.

Will be part of the next release as soon as more tests have been performed 
(it's a mixer change!)

Original comment by bson...@gmail.com on 3 Feb 2013 at 4:27

[GoogleCodeExporter]

Yes of course, testing is required. I will try to load the sources and compile, 
never done before, but we will see...
Didn't expect you do this so quickly, thanks a lot.

A further idea, don't know if I should add.
For me it is always confusing, the offset and weight is calculated to the 
source of the mixer, rather than the output. Mainly this do not make any 
difference, but it does for curves or other functions. Actually the order seems 
to be first offset, than weight than the rest, even the order is different in 
menu.
Personally I would prefer to have the offset and weight calculated to the 
output instead of the source. I do not know what is the default in other 
transmitters. The best could be an option, to select, if the weight and offset 
is to source or to output, costs one bit per mixer, potentially still one free? 
The best might be both possibilities, which would add 2 bytes per mixer, too 
much for stock? But maybe an option for sky9x?
What do you think, should I add a further request, describing this in detail?

Original comment by open.20.fsguruh@xoxy.net on 3 Feb 2013 at 6:53

[GoogleCodeExporter]

Romolo, Franck, André, Gabriel, your opinion about this last comment? (the 
"normal" order and the additional bits)? Thanks! 

Original comment by bson...@gmail.com on 3 Feb 2013 at 7:27

• Changed state: Started

[GoogleCodeExporter]

Original comment by bson...@gmail.com on 3 Feb 2013 at 7:28

[GoogleCodeExporter]

Compile finished, work like perfect. Didn't expect to be so easy, will report 
later, but up to now everything look OK. 

Original comment by open.20.fsguruh@xoxy.net on 3 Feb 2013 at 7:37

[GoogleCodeExporter]

first fix helped to issue 188, but this magic flip-flop mix is no more working:

CH16 (+100%)CS1 Delay(u1:d1) Slow(u1:d1)

CS1  CH16 < 0

comment#6 weight is sure for source. we can do weight calculation on second mix 
with multiply.
ofset is also for source. if we need ofset on output there are limits.

Original comment by gbir...@gmail.com on 3 Feb 2013 at 7:50

[GoogleCodeExporter]

Right, but you will have to use CS1 as the source and also as the switch!

Original comment by bson...@gmail.com on 3 Feb 2013 at 8:09

[GoogleCodeExporter]

In fact I made it more consistent, but I can revert if needed:
The delay is only applied when the switch is toggled (on and off)

Original comment by bson...@gmail.com on 3 Feb 2013 at 8:31

[GoogleCodeExporter]

Hi all,

The "normal" (and logical) order should be like it is today (and like it is in 
any commercial radio) : add the offset to the source, than apply the weight.

Adding an option to apply the offset after the weight (equal : to offset the 
output) is not necessary because it can already be done by an additionnal 
mixing line to the channel with MAX source. An this mixing line is much less 
ambiguous then an offset option inside a mix...

Original comment by f.ague...@wanadoo.fr on 3 Feb 2013 at 8:33

[GoogleCodeExporter]

@fsguruh
So we have another beta tester! Cool, on which board?

@franck
Thanks, it makes sense to me

@gabriel
I reverted this part of the commit, not related to the issue

Original comment by bson...@gmail.com on 3 Feb 2013 at 9:05

• Changed state: Fixed

[GoogleCodeExporter]

till now just stock which gives me up to now plenty of features.
I agree for a second mix in line, that would implement the offset weight on 
output. Sometimes people thinking too complicated.
I will report tomorrow about testing...

Original comment by open.20.fsguruh@xoxy.net on 3 Feb 2013 at 10:03

[GoogleCodeExporter]

Tests finished. Works perfect. Even toggling the switch will cause a slow 
change to 0 for the mix as desired. Perfect change for me!

Original comment by open.20.fsguruh@xoxy.net on 4 Feb 2013 at 5:24

[GoogleCodeExporter]

A question about the toolchain: I used WinAVR 20100110 which is gcc 4.3.3.
I noticed the programming of EEPROM is much slower compared with r1855. Do you 
use the same compiler or do you use the one from atmel?

Original comment by open.20.fsguruh@xoxy.net on 4 Feb 2013 at 5:27

[GoogleCodeExporter]

@f.ague:
> Adding an option to apply the offset after the weight (equal : to offset the 
output) > is not necessary because it can already be done by an additionnal 
mixing line to the > channel with MAX source. An this mixing line is much less 
ambiguous then an offset 
> option inside a mix...
After further testing I do not fully agree with this.
I propose to calculate all; weight, offset, and delay and slow to output rather 
than input controlled with a flag, configurable inside the mixer. To reach 
both, having offset and weight for the output costs two further mixer lines, 
because you need one to multiply and a further one to add.
Arguments 
Pro:
- If needed it removes need for two further mixing lines. (Saves 10 bytes, but 
need 1 bit)
- Adds possiblity to have slow calculation on output rather than input, which 
could be annoying if curve has a crazy form.
Contra:
- Costs always one bit, even not needed for each mix line. (If nothing is left)

What do you think, is it worth to be added?

Original comment by open.20.fsguruh@xoxy.net on 4 Feb 2013 at 5:38

[GoogleCodeExporter]

Only one additionnal line is enought, no need to multiply because the initial 
mixing line has its own weigth.

Example (firt line can offset the source by xx, second line offset the output 
by zz) :
CHx source = ss, offset = xx, weight = yy
:= source = MAX, offset = 0, weight = zz

Some arguments :
- offseting the output has to be seen in the Mixer menu without editing the mix 
(very important to know what you send to each channel).
- offseting the output, when needed, has to be done only one time per channel : 
so it is more logical to do it in one mixing line, instead to have an option in 
all mixes.

Original comment by f.ague...@wanadoo.fr on 4 Feb 2013 at 7:10

[GoogleCodeExporter]

Hi f.ague,

thanks for the quick response. Can you explain it a little bit better?
I do not understand it properly. Your second line is a add, multiply or replace?

My problem is not only to add offset and weight. In the beginning I explained, 
the combination with a curve or function leads to this proposal. 
In your example above you put the weight in the first line, but this would be 
applied before the curve is calculated. The point was to have both weight and 
offset after curve, function, etc. And this should be optional, so either 
before or after. The overview would not be changed because of this. Just one 
additional bit, which allows to define, if offset, weight, slow calculation is 
done on input before curve or function is calculated, or afterwards on output.
Your example lacks the curve.
I define an example, to make it better understandable:

CH06   (+100%)Rud Curve(Curve 1)  --> original curve
      *(+80%)MAX                  --> Weight on output; 1st additional line
       (+100%)MAX Offset(-90%)    --> Offset of +10% on output (complicated anyway); 2nd additional line

That was the only change to define this function:
CH06 = Curve1(Rud)*80% + 10%

What is currently possible in mix with offset and weight would lead to this mix 
line:
Ch05 (+80%)Rud Offset(10%) Curve(Cuver 1)
This is actually 

CH06 = Curve1( (Rud+10%)*80% ) 

This is very different to the function I actually want.

Still not convinced it would be worth to spend one additional bit?

CH06 = Curve1(Rud)*80% + 10%
CH06 = Curve1( (Rud+10%)*80% ) 

Of course there would be a third option to define the order of offset and 
weight, but this may be too much.

Original comment by open.20.fsguruh@xoxy.net on 4 Feb 2013 at 9:36

[GoogleCodeExporter]

Sorry, it was "+=" and not ":="...

So, your example can be rewrite as simple as :
CH06 Rud weight=+80% Curve(Curve 1)
  += MAX weight=+10%

=> CH06 = Curve1(Rud) * 80% + 10%. Is it not what you want to have ?
And this method is explicit, not implicit like an option hidden in mix.

Original comment by f.ague...@wanadoo.fr on 4 Feb 2013 at 10:10

[GoogleCodeExporter]

Yes I agree, tried it and it is actually the same.
Very confusing, I thought weight is multiplied on the input (also described in 
manual that way, the same as offset in the manual), but actually the weight is 
multiplied on output. Only offset is applied to input. 
I thought it would do:
CH06 = Curve1(Rud*80) +10%; 
but it actually does what I mean. I am happy with this combination, because I 
prefer the calculation on output. The offset and slow can be done with second 
mix hopefully, which is OK for me.

Hopefully, noone want's weight on input, like it is described in docu...

Original comment by open.20.fsguruh@xoxy.net on 4 Feb 2013 at 10:37

[GoogleCodeExporter]

No problem :)

Original comment by f.ague...@wanadoo.fr on 4 Feb 2013 at 11:05