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.
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
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