Concerns over the air density correction calcs

This is a forum for discussing the development and testing of alpha MS2/Extra code. Documentation
(Runs on MS2 and Microsquirt)

Moderators: jsmcortina, muythaibxr

Concerns over the air density correction calcs

Postby gslender » Thu May 31, 2012 1:24 am

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;
}
Last edited by gslender on Thu May 31, 2012 7:53 pm, edited 1 time in total.
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Bug found in air density correction

Postby Peter Florance » Thu May 31, 2012 3:21 am

subscribed to topic
Peter Florance
PF Tuning
81 BMW Euro 528i ESP Car
60-2 Wheel LS2 Coils, Low Z Inj
Check your grounds: http://www.msextra.com/doc/general/grounding.html
User avatar
Peter Florance
Super MS/Extra'er
 
Posts: 3382
Joined: Fri Apr 02, 2004 8:40 pm
Location: Virginia Beach, VA

Re: Bug found in air density correction

Postby Zaphod » Thu May 31, 2012 3:47 am

Are the MS2 and MS3 MAT corrections the same?
--------------------------------
fun is not a straight line

Image

Sven
http://www.mx-5club-sachsen.de
http://miata.cardomain.com/id/svenmx5
NB-1998-1,6-Garrett T25 HGP Turbo Stage I
Zaphod
Experienced MS/Extra'er
 
Posts: 314
Joined: Thu Aug 14, 2008 11:38 pm
Location: Germany

Bug found in air density correction

Postby gslender » Thu May 31, 2012 3:53 am

Source for the very latest ms3 code isn't released, so can't say, but a lot of the code often is similar.
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Bug found in air density correction

Postby Peter Florance » Thu May 31, 2012 4:05 am

What are the implications? Is it something I can check in a log?
Thanks!
Peter Florance
PF Tuning
81 BMW Euro 528i ESP Car
60-2 Wheel LS2 Coils, Low Z Inj
Check your grounds: http://www.msextra.com/doc/general/grounding.html
User avatar
Peter Florance
Super MS/Extra'er
 
Posts: 3382
Joined: Fri Apr 02, 2004 8:40 pm
Location: Virginia Beach, VA

Re: Bug found in air density correction

Postby jsmcortina » Thu May 31, 2012 4:08 am

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
I can supply, repair or upgrade Megasquirts in UK.

My Success story: viewtopic.php?f=104&t=34277
MSEXTRA documentation at: http://www.msextra.com/doc/index.html
jsmcortina
Site Admin
 
Posts: 28501
Joined: Mon May 03, 2004 1:34 am
Location: Birmingham, UK

Re: Bug found in air density correction

Postby gslender » Thu May 31, 2012 4:22 am

Ahhh, true - precedence says the cast happens first, then the subtraction... good point. My bad. :oops:
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby gslender » Thu May 31, 2012 8:43 pm

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
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby ol boy » Thu May 31, 2012 10:32 pm

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.
289 SBFord, Torquer II EFI intake, 36lbs injectors, 8 LS2 coils in waste spark and a TURBO added MAF!, 4R70W, MS2extra 3.4a7
ol boy
Master MS/Extra'er
 
Posts: 760
Joined: Mon Sep 10, 2007 3:06 am
Location: Tucson, Az

Re: Concerns over the air density correction calcs

Postby jsmcortina » Fri Jun 01, 2012 3:02 am

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
I can supply, repair or upgrade Megasquirts in UK.

My Success story: viewtopic.php?f=104&t=34277
MSEXTRA documentation at: http://www.msextra.com/doc/index.html
jsmcortina
Site Admin
 
Posts: 28501
Joined: Mon May 03, 2004 1:34 am
Location: Birmingham, UK

Re: Concerns over the air density correction calcs

Postby gslender » Fri Jun 01, 2012 3:18 am

Cool. Looks like a fairly easy thing to do... any problems if I give that a crack?
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby aaronc7 » Tue Jun 05, 2012 6:07 am

any progress? :)
aaronc7
MS/Extra Newbie
 
Posts: 23
Joined: Tue Apr 05, 2011 10:15 pm

Re: Concerns over the air density correction calcs

Postby gslender » Tue Jun 05, 2012 12:58 pm

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
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby muythaibxr » Tue Jun 05, 2012 1:38 pm

I'm OK with it, just watch for overflows.

Ken
Megasquirt is not for use on pollution controlled vehicles. Any advice I give is for off road use only.

:msq:
muythaibxr
Site Admin
 
Posts: 7462
Joined: Thu Oct 14, 2004 12:48 pm

Re: Concerns over the air density correction calcs

Postby robd » Wed Jun 06, 2012 9:01 am

I'd be very keen to try out a new code with this change in it for MS2. :)
robd
Experienced MS/Extra'er
 
Posts: 302
Joined: Wed Feb 29, 2012 6:14 am

Re: Concerns over the air density correction calcs

Postby gslender » Thu Jun 07, 2012 4:08 am

Ok, I've got a build I'm testing right now. Shouldn't be too much longer 8)

G
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby aaronc7 » Thu Jun 07, 2012 7:55 am

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!
aaronc7
MS/Extra Newbie
 
Posts: 23
Joined: Tue Apr 05, 2011 10:15 pm

Re: Concerns over the air density correction calcs

Postby gslender » Thu Jun 07, 2012 1:32 pm

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
Mazda MX5 + MS3 Pro
User avatar
gslender
Super MS/Extra'er
 
Posts: 1072
Joined: Fri Sep 16, 2011 5:29 am
Location: Brisbane, Australia

Re: Concerns over the air density correction calcs

Postby aaronc7 » Fri Jun 08, 2012 5:20 am

awesome, can't wait!

too bad my motor is still being built
aaronc7
MS/Extra Newbie
 
Posts: 23
Joined: Tue Apr 05, 2011 10:15 pm


Return to MS2/Extra Development

Who is online

Users browsing this forum: No registered users and 0 guests