Page 1 of 1

Concerns over the air density correction calcs

Posted: Thu May 31, 2012 1:24 am
by gslender
Hi,

Was looking at the code in MS2 and me thinks I've found a bug in the air density correction code...

There is a cast to an unsigned char (ccor) when in the cor calc above it could be less than 100 and therefore "cor - 100" could be -3 and when cast to unsigned char this would be a bad/incorrect value yeah? Even worse, the sign from the previous calc is also lost by making it unsigned.

Code: Select all

int aircor_eq(int mat)
{
    signed int cor;
    signed char ccor;
    // returns air density correction from equation in % (100 is no correction)
... <snip> .... 

        cor = 529700 / (mat + 4597);
// Changed for 3.0.3w to ignore this (unused) setting always use degF internally.
//    }
//    else  {                          // deg C
//        cor = 293200 / (mat + 2732);
//    }
    if (flash5.airden_scaling) { // only if non-zero
        ccor = (unsigned char)cor - 100;    /// <<<<<<< this shouldn't be cast to an unsigned char !!!
        cor = ((signed int)(ccor * flash5.airden_scaling)) / 100;
        cor = cor + 100;
    }
    return (int)cor;
}

Re: Bug found in air density correction

Posted: Thu May 31, 2012 3:21 am
by Peter Florance
subscribed to topic

Re: Bug found in air density correction

Posted: Thu May 31, 2012 3:47 am
by Zaphod
Are the MS2 and MS3 MAT corrections the same?

Bug found in air density correction

Posted: Thu May 31, 2012 3:53 am
by gslender
Source for the very latest ms3 code isn't released, so can't say, but a lot of the code often is similar.

Re: Bug found in air density correction

Posted: Thu May 31, 2012 4:05 am
by Peter Florance
What are the implications? Is it something I can check in a log?
Thanks!

Re: Bug found in air density correction

Posted: Thu May 31, 2012 4:08 am
by jsmcortina
gslender wrote:There is a cast to an unsigned char (ccor) when in the cor calc above it could be less than 100 and therefore "cor - 100" could be -3 and when cast to unsigned char this would be a bad/incorrect value yeah?
The cast is on the _input_ to, not the output from that calc. (EDIT: Although at a glance I'm not sure why I added that cast.)
Even worse, the sign from the previous calc is also lost by making it unsigned.
The previous calc is a positive number divided by a positive number - how can that give a negative result?

Have you checked the generated assembler for this code?
Have you tested it to see what range of values are produced?

James

Re: Bug found in air density correction

Posted: Thu May 31, 2012 4:22 am
by gslender
Ahhh, true - precedence says the cast happens first, then the subtraction... good point. My bad. :oops:

Re: Concerns over the air density correction calcs

Posted: Thu May 31, 2012 8:43 pm
by gslender
Thought I'd share what the values do so that you can see for youself how the code impacts fueling based on tempature and the various % in correction scaling.

The fueling is scaled by the % calculated in the last two columns (one is for 100% MAT scaling, the other is values used at 50% MAT scaling).

I'll admit I was quick to think a bug exists when in fact the odd unsigned cast through my thinking around precedence.

I am now wondering if the lack of resolution makes any difference - note how the band of no correction (shown around 68-70F where it is at 100% correction) is only a small area as you change from 100% to 50% MAT scaling. When you look at 32F, the resulting correction is more pronounced (perhaps as it should be), but should the change to scaling have no impact to the correction seen at 64F deg, which is 1% additional fuel regardless of whether you have 100% scaling or 50% scaling? That seems wrong yeah? but perhaps the resulting fuel difference is negligble too... in which case when does it have an impact?

Code: Select all

F	C	100% 	50% 
		Corr	Corr

32	0	108	104
36	2	107	103
40	4	106	103
44	7	105	103
48	9	104	102
52	11	104	102
56	13	103	101
60	16	102	101
64	18	101	101
68	20	100	100
72	22	100	100
76	24	99	99
80	27	98	99
84	29	97	99
88	31	97	98
92	33	96	98
96	36	95	98
100	38	95	97
104	40	94	97
108	42	93	97
112	44	93	96
116	47	92	96
120	49	91	96

Re: Concerns over the air density correction calcs

Posted: Thu May 31, 2012 10:32 pm
by ol boy
I'm glad you did this table. I can now see the % of change in the curve. I've changed my ini file to allow lower than 50% scaling, I run mine down around 20% and use the correction table to fine tune. I think heat soak issues are causing rough starts and poor running after a hot start. It's getting warm in southern AZ. 108*F tomorrow.

Re: Concerns over the air density correction calcs

Posted: Fri Jun 01, 2012 3:02 am
by jsmcortina
In MS3 we already changed the 'correction' curve to 0.1% steps. In a future code release we'll likely merge the built-in calculation and the correction curve.

James

Re: Concerns over the air density correction calcs

Posted: Fri Jun 01, 2012 3:18 am
by gslender
Cool. Looks like a fairly easy thing to do... any problems if I give that a crack?

Re: Concerns over the air density correction calcs

Posted: Tue Jun 05, 2012 6:07 am
by aaronc7
any progress? :)

Re: Concerns over the air density correction calcs

Posted: Tue Jun 05, 2012 12:58 pm
by gslender
Well I was hoping I'd get a blessing first, so I might look into it, but alas would prefer Ken or James to be ok with that first.

G

Re: Concerns over the air density correction calcs

Posted: Tue Jun 05, 2012 1:38 pm
by muythaibxr
I'm OK with it, just watch for overflows.

Ken

Re: Concerns over the air density correction calcs

Posted: Wed Jun 06, 2012 9:01 am
by robd
I'd be very keen to try out a new code with this change in it for MS2. :)

Re: Concerns over the air density correction calcs

Posted: Thu Jun 07, 2012 4:08 am
by gslender
Ok, I've got a build I'm testing right now. Shouldn't be too much longer 8)

G

Re: Concerns over the air density correction calcs

Posted: Thu Jun 07, 2012 7:55 am
by aaronc7
woohoo! Were you able to add any of the Air density correction taper features like I had mentioned? I've been looking into it on my own...but I am not a programmer, so it's going pretty slow. I've found it in the MS1 code and now looking to see how to incorporate that into MS2 existing code. But... if you're been working on it already then I suppose I'll just stop and wait!

Re: Concerns over the air density correction calcs

Posted: Thu Jun 07, 2012 1:32 pm
by gslender
aaronc7 wrote:woohoo! Were you able to add any of the Air density correction taper features like I had mentioned? I've been looking into it on my own...but I am not a programmer, so it's going pretty slow. I've found it in the MS1 code and now looking to see how to incorporate that into MS2 existing code. But... if you're been working on it already then I suppose I'll just stop and wait!
If you mean a RPM based taper, then yes.... I've extended the basic scaling to be 5 pt rpm based. Defaults to 100% like the single value scaler, but the curve can be changed to any % at any rpm so you can zero at idle 100% at 1000rpm sliding to zero at 4000 etc.

G

Re: Concerns over the air density correction calcs

Posted: Fri Jun 08, 2012 5:20 am
by aaronc7
awesome, can't wait!

too bad my motor is still being built