sylvandb / gruvin9x

Automatically exported from code.google.com/p/gruvin9x
0 stars 0 forks source link

Order of DR/EXPO lines affect whether switch works or not #71

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
frsky branch r1220

What steps will reproduce the problem?
1. Set up a DR/EXPO line using AIL switch to activate
2. Set up a second DR/EXPO line with no switch
3. Watch the curves to note that the EXPO does indeed switch in and out when 
flicking the AIL switch
4. Swap the order of the two DR/EXPO lines, such that the one with the switch 
setting is now the second line.
5. Now note that flicking the AIL switch has no affect 
6. Swap the lines back. AIL switch works again

(See screen snapshots -- noting their filenames for description.)

What is the expected output? What do you see instead?
It should not matter in what order the DR/EXPO lines are, as to whether or not 
the switch function works or not.

Or should it? If so, why?

I'm pretty sure we've already covered this one in the past and already fixed 
it. But somehow, it's regressed.

Original issue reported on code.google.com by gru...@gmail.com on 18 Nov 2011 at 5:40

Attachments:

GoogleCodeExporter commented 8 years ago
I double checked this morning. Without seeing any issue here. The order of 
EXPOS for a channel is important, because it's the first one where all 
conditions match that will be taken into account. 

As a consequence, if you have a line without a condition as the first line, all 
others will be simply ignored.

Original comment by bson...@gmail.com on 18 Nov 2011 at 8:36

GoogleCodeExporter commented 8 years ago
Last time you commented on this, you said the opposite. You said that you 
agreed that you yourself would normally set up the non-expo lines first and 
then later add the expo lines, below. You said you'd fix it and then you did 
fix it -- and I tested it and it was fine -- pretty sure.

I'll try to find your comments in the previous issue.

Regardless, it makes no sense. It should not matter the order of the lines. If 
it does matter -- then where is the indication to the user as to what is 
happening? And what happens if you want to add a third line, on a different 
switch. Perhaps the third line only changes weight, while the first line 
changes only expo. Then the third line is useless if the first line is in 
effect, right? 

I also forget again why we even have multiple lines. There never used to be. 
But that's another issue (of my own).

Original comment by gru...@gmail.com on 19 Nov 2011 at 3:58

GoogleCodeExporter commented 8 years ago
Apologies. I've gone back and read the previous issues text and I see nothing 
to support my claims above. It must have been self-fabricated memories. Weird 
-- but I'm human, so yeah. :-/

My self and James (local guy using the radio) are still certain that the order 
of lines should not matter. But by all means, if it makes no sense to you, then 
try to educate us. :-D

Original comment by gru...@gmail.com on 19 Nov 2011 at 4:01

GoogleCodeExporter commented 8 years ago
Let's ask to Cam! What I am sure: at the end it will be fine for all of us :)

Original comment by bson...@gmail.com on 19 Nov 2011 at 8:08

GoogleCodeExporter commented 8 years ago
Sure -- Cam? Amy thoughts? (Does he get emails of these when not assigned as 
owner?)

I can only reiterate that the way it is now makes it impossible to have more 
than one switch assignment active at any time. Thus, you cannot add one switch 
to another to get 'even more' of, whatever. What if I select a flight phase on 
line one but then want to add dual-rates on AIL in line two? I guess I just 
have to use !AIL in line one, along with FP1, for example. Hmmm. 

It all seems complicated and strange to me. Surely there's an easier way?

Original comment by gru...@gmail.com on 19 Nov 2011 at 10:46

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Why not 2 switches? Do you mean an AND on two switches? Then you only have to 
use a custom switch (i.e. SW1=AIL AND TRN)

Or you mean using 2 switches as in er9x for a high/middle/low weight?
- line1=AIL
- line2=ELE
- line3=---

The reason for the order: Suppose you want that in a flight phase, the weight 
is 50% and in all other cases it depends on a switch:
- line1: FP1 => 100%
- line2: AIL => 50%
- line3: --- => 75%
If this example, if you invert line 1 and line 2, it will mean: You always want 
50% when AIL active, then 100% if FP1 and 75% in all other cases.

Does it make sense now?

Original comment by bson...@gmail.com on 19 Nov 2011 at 11:01

GoogleCodeExporter commented 8 years ago
That example does make perfect sense, yes. But so did the example James gave 
this afternoon, which I've now forgotten. What I have to do is get this reason 
why clear in my head, so I can convey it back to him with confidence. I'll 
sleep on it.

Original comment by gru...@gmail.com on 19 Nov 2011 at 11:06

GoogleCodeExporter commented 8 years ago
Good night, you will give me the example from James when you have it in mind.

Original comment by bson...@gmail.com on 19 Nov 2011 at 11:12

GoogleCodeExporter commented 8 years ago
OK. Woke with a fresh mind and set to working this out again (again). I am now 
clear about exactly what the problem is and hopefully also how to explain it 
more clearly.

Referring to the attached snapshot (noting the <- and -> on lines 2 and 3 
respectively) ... the problem is with line 1.

Specifically, line 1 should not have to have !AIL assigned to make is active 
when the AIL switch is OFF. With no switch assigned at all, it should already 
be active ... simply because there is no switch requirement selected -- 
therefore, is should 'just be active'. But in fact, when lines 2 and 3 are 
present, line one is only active if it has !AIL assigned as its switch.

I am certain we have discussed this before and that it has been fixed before. I 
am guessing then, that the recent getSwitch() upgrade must have regressed this 
issue.

Agreed -- yes/no?

Original comment by gru...@gmail.com on 20 Nov 2011 at 12:31

Attachments:

GoogleCodeExporter commented 8 years ago
My previous comment describes the WHOLE of the problem too, by the way. This is 
so because changing the order of the lines -- putting line one at the end -- 
makes it works as expected again. Thus the problems was described to me as, 
"The order of lines should not matter." But in fact it is this "not" switch 
requirement that is causing the bad effect, making it seem that it is the order 
of lines that is the problem, when in fact it is something a little different.

Original comment by gru...@gmail.com on 20 Nov 2011 at 12:38

GoogleCodeExporter commented 8 years ago
Perhaps another comment is needed, just to be safe.

For the switch assertion assessment --

========================

[---]  should mean "this line is always active, no matter the state of any 
other line"

[AIL]  should mean "this line is active only if AIL switch is on"

[!AIL] should mean "this line is active only if AIL switch is off"

========================

In other words, "---" does NOT mean "not AIL", but "!AIL" does mean "not AIL". 
In stead, "---" means "I am always active, no matter what.

Following on from that, in the case where two line with no switch in either are 
present, then the weight and expo would be computed twice. Thus, two lines with 
weights both set to 50% would result in a final weight of 25%. This is clearly 
not programming that anyone would typically ever want to have -- but that is 
the way it should work. (Same for expo.)

Original comment by gru...@gmail.com on 20 Nov 2011 at 12:56

GoogleCodeExporter commented 8 years ago
Answer to comment #10. If there is no switch (---) do you confirm that no other 
line is used? There can be only one expo line used, no mixing at this stage! I 
just tried it on the simu. It's good, the lines 2 and 3 are NOT used at all.

In fact, I do have to understand if the problem is a bug or something new that 
you would like to use!

Original comment by bson...@gmail.com on 20 Nov 2011 at 8:51

GoogleCodeExporter commented 8 years ago
It's a bug. Definitely. We've already found, discussed and cured this bug in 
the past. We're having the exact same conversation all over again.

Original comment by gru...@gmail.com on 20 Nov 2011 at 10:53

GoogleCodeExporter commented 8 years ago
I wrote in comment #10 to #12 the answers to your other question in comment #13.

You didn't seem to have any problem both a) agreeing and b) fixing this the 
last time. I don't understand what the problem is this time. :-/

It's also not true that only one DR/EXPO line can be active at any one time. 
There are times when you must have at least two -- for example, to implement 
separate settings for when:X>0 and when:x<0, for the same switch setting (as 
shown in the examples above.) Moreover, this *is* working just fine at the 
moment. Both lines 2 and 3 are functional in the above examples, a they should 
be.

The only problem is that sw:--- is not being treated right.

Original comment by gru...@gmail.com on 20 Nov 2011 at 11:02

GoogleCodeExporter commented 8 years ago
No I have to repeat again. The current algo makes one line active at one time. 
Your example with X>0 and X<0 is a perfect example. Depending on X different 
settings will be applied.

Sw: --- should be applied exactly the same way as Phase: --- or When: ---

Original comment by bson...@gmail.com on 20 Nov 2011 at 11:42

GoogleCodeExporter commented 8 years ago
I don't know if I am clear enough: this is what I have implemented (unless 
there is a bug):

If a line match, no other will be applied! And a line match provided all the 
parameters (X, flight phase, switch) match.

You are proposing to change that so that if a line match, we continue with the 
next ones, applying ALL lines that match. Am I right?

Original comment by bson...@gmail.com on 20 Nov 2011 at 12:37

GoogleCodeExporter commented 8 years ago
OK guys, I didn't get an email till Bertrand just dropped me one. Give me some 
time to catch up and I will see if I can help (rather than make it worse). 

Original comment by th9...@gmail.com on 20 Nov 2011 at 12:47

GoogleCodeExporter commented 8 years ago
Let's forget about the "only one line" thing -- it is not relevant to the 
actual fault I'm trying to convey.

If no switches are on and no other lines match, then a line with Switch=[---] 
***SHOULD MATCH*** -- no matter where in the list or what order the lines are 
in. If no other lines match, then that line MUST match. But it does not match 
if other lines have AIL *unless* the faulty line is set to !AIL, which it 
should NOT have to be.

Again -- we have already discussed this in the past. You agreed in the past 
(very quickly). You fixed it in the past (also very quickly). So what the hell 
is going on here?

Original comment by gru...@gmail.com on 21 Nov 2011 at 12:26

GoogleCodeExporter commented 8 years ago
Indeed. This last comment makes perfect sense to me. And if it doesn't work as 
you said, there is a bug somewhere. But I didn't reproduce it (it's true that I 
only tested on the simu).

In the snapshot above:
1: 100 0 ---
2: 60 30 AIL ->
3: 40 30 AIL <-
Line 1 should ALWAYS the active one. NEVER the others.

In the example you wrote a little bit above:
1: 50 0 ---
2: 50 0 ---
Line 1 should ALWAYS the active one. NEVER the line 2. Therefore the result 
will never be 25% as I understand you are saying.

Original comment by bson...@gmail.com on 21 Nov 2011 at 8:35

GoogleCodeExporter commented 8 years ago
What? ;-/ NO. That is NOT correct and it is illogical.

In the snapshot above:
1: 100 0 ---
2: 60 30 AIL ->
3: 40 30 AIL <-

Here, in fact, line 1 should be active whenever no other lines are active -- 
because it carries no switch conditional. No conditional means no condition. 
Simple.

Further, lines 2 AND 3 need to operate together (whel AIL=true), otherwise only 
line 2 would operate, causing the X<0 expo or rate to be ignored, which is 
WRONG.

Further logical evidence for my argument  can be easily found by considering 
the case where there is only ONE line ...

In the snapshot above:
1: 100 0 ---

You are suggesting that this one line on its own should never be active!! This 
is clearly not correct.

Original comment by gru...@gmail.com on 21 Nov 2011 at 9:35

GoogleCodeExporter commented 8 years ago
In this case
1: 100 0 --- --- 
(only one line then)
Line 1 is always active. No condition means always (as in the previous example).

Original comment by bson...@gmail.com on 21 Nov 2011 at 9:45

GoogleCodeExporter commented 8 years ago
What it should do in this case?

1: 100 0 --- --- FP1   
2: 60 30 AIL ->
3: 40 30 AIL <-

With my logic, 1 is active when all contitions met (then FP1, no matter AIL).
2 is used if 1 is not used and all contitions of 2 met
3 is used if 1 and 2 not used and all conditions of 3 met

If no condition at all is met, then it's the default: 100% weight, 0% expo (the 
stick value, no dual rate, no expo)

Original comment by bson...@gmail.com on 21 Nov 2011 at 9:49

GoogleCodeExporter commented 8 years ago
That part of your logic is 100% OK. The other part is not.

Original comment by gru...@gmail.com on 21 Nov 2011 at 9:53

GoogleCodeExporter commented 8 years ago
Hmmm. I think I understand why you are having difficulty with this, now. It's 
because of the actual implementation, which is to drop down the list just once 
and stop as soon as a line with matching conditions is found. Therefore, line 1 
in the 3-line examples would block lines 2 and 3, always.

Well then simply, the implementation is bad. It simply make no sense to the 
user that line 1 is 'always active' when on its own, yet never active when 
other lines exist. That is a big problem.

Original comment by gru...@gmail.com on 21 Nov 2011 at 9:55

GoogleCodeExporter commented 8 years ago
No I don't say that "line 1 is 'always active' when on its own, yet never 
active when other lines exist".

In both examples I gave:

1: 100 0 ---
2: 60 30 AIL ->
3: 40 30 AIL <-

and 

1: 100 0 ---

I say that 1 is ALWAYS active. In BOTH cases. Line 1 condition is always met.

Original comment by bson...@gmail.com on 22 Nov 2011 at 8:32

GoogleCodeExporter commented 8 years ago
Now about your proposal. Are you suggesting that all matching lines should be 
applied?

Original comment by bson...@gmail.com on 22 Nov 2011 at 8:32

GoogleCodeExporter commented 8 years ago
Re. comment 26 

I'm not concerned with with what you think I said that you said. (I did not.) I 
am only concerned with the fact of what actually happens in testing! Sheesh. :-/

What is true in my testing is NOT what you said should be true in your comment 
26. I agree with what you said should be true in your comment 26. But that is 
NOT what is in fact happening in testing.

What is true in testing is that, in the presence of lines 2 and 3, line 1 is 
NEVER matched -- under ANY circumstance -- though I agree, it SHOULD be. This 
is the problem I have been trying to convey all this while. You don't seem to 
be 'getting' it.

(Line one can be made to match in the presence of lines 2 and 3 if it's switch 
parameter is set to !AIL. But this should NOT be necessary.)

Again I'll say it. This bug was present once before and was fixed (by you) once 
before. It has somehow regressed.

Original comment by gru...@gmail.com on 22 Nov 2011 at 10:24

GoogleCodeExporter commented 8 years ago
But ... I did test this scenario:

1: 100 0 ---
2: 60 30 AIL ->
3: 40 30 AIL <-

Line 1 was always used (because condition 1 always met), never 2 and 3.

I will re-test again!

Original comment by bson...@gmail.com on 22 Nov 2011 at 10:30

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
I just woke up because I think my brain has been doing its usual dyslexia 
mixing up of fact crap again. :-/ (My comment 30 deleted.)

You are correct about comment 29. What I was remembering is not that line did 
not work but that lines 2 and 3 did not work if line 1 was present and at the 
top.

Damn it. This has been a highly frustrating, annoying and confusing exercise. 
Hopefully I can now REMEMBER how and why it works, so that I can explain it to 
the local users in some way they can accept.

Given all the information now in my head, it seems there is no logical choice 
but to simply accept that the very line that matches will prevent all following 
lines from even being checked. "Order counts and that is just that."

Original comment by gru...@gmail.com on 22 Nov 2011 at 11:36

GoogleCodeExporter commented 8 years ago
STUPID #@!#@$ fingers not freakin typing what I ask them :/

ERRATA re comment 31:
... What I was remembering is not that line *1* did not work ...

... but to simply accept that the very *first* line that matches ...

*sigh*

Original comment by gru...@gmail.com on 22 Nov 2011 at 11:39

GoogleCodeExporter commented 8 years ago
Oh and by the way ... all that stuff I said about how all lines must be 
considered regardless, was only because I thought that both lines 2 and 3 must 
work 'together'.

In fact, only one of the lines 2 or 3 ever has to match at any one time to 
create the condition where it seems they are both active. That is, once when 
x<0 and the other when x>0. I was thinking only of the FP and SW fields as 
being part of the match condition. That is to say, I was not considering 'When' 
as part of the match criteria. Thus, I concluded that both lines 2 and 3 had to 
be active at the same time. I was wrong.

In other words, yes, you are right (again) and only one line need be active at 
any one time.

Therefore, this entire issue 71 can be marked invalid. I will leave that job 
for you, so as to give you some recompense for wasting your time! :-P

Now I will go back to sleep. Perhaps I should never wake? :-P

Original comment by gru...@gmail.com on 22 Nov 2011 at 11:49

GoogleCodeExporter commented 8 years ago
Sleep on peace. I am happy to close this one. I was afraid you were asking for 
a new mixer applied to inputs (not only outputs).

Ouf !

Original comment by bson...@gmail.com on 22 Nov 2011 at 1:09