Bug found in MS3 1.4.0: incorrect output afrtgt

[Shad Laws]

Hello,

I found a bug in MS3 1.4.0, and think I found where it occurs in the source, too.

Symptom: datalogs of afrtgt1 just don't work, whether serial or SD. They start at 14.7 with the engine off, then instantly drop to 10.2 once the engine is running. However, it must be correctly calculated somewhere as "incorporate AFR target" doesn't result in a super-rich engine.

Cause: I believe the issue is typecasting differences between gl_afrtgt1 (which is used to determine the value and for the "incorporate AFR target" option) and outpc.afrtgt1 (which goes into the datalogs):
- ms3.h, line 1625: struct with afrtgt1, afrtgt2 as unsigned char (8 bit)
- ms3_vars.h, line 693: gl_afrtgt1, gl_afrtgt2 as int (16 bit)
- ms3_init.c, line 4714: outpc.afrtgt1, outpc.afrtgt2 initialized at 147 (8 bit)
- ms3_ego.c, lines 251-293: gl_afrtgt1, gl_afrtgt2 calculated by dummy variables tmp1, tmp2, which are unsigned int (16 bit)
- ms3_inj.c, lines 2847-2848: gl_afrtgt1, gl_afrtgt2 used for "incorporate AFR target" option
- ms3_ego.c, lines 246-247: outpc.afrtgt1, outpc.afrtgt2 (8 bit) set using gl_afrtgt1, gl_afrtgt2 (16 bit) --> bug!
This explains why "incorporate AFR target" works, why the initialization of the outpc variable works, and why it doesn't work once the engine is running.

Solution: I think it's as simple as changing ms3_ego.c, lines 246-247 from:

Code: Select all

        outpc.afrtgt1 = gl_afrtgt1;
        outpc.afrtgt2 = gl_afrtgt2;

to:

Code: Select all

        outpc.afrtgt1 = (unsigned char) gl_afrtgt1;
        outpc.afrtgt2 = (unsigned char) gl_afrtgt2;

Does this seem correct? If so, can it be rolled into the next build?

Thanks!

Take care,
Shad

[jsmcortina]

Code: Select all

        outpc.afrtgt1 = (unsigned char) gl_afrtgt1;
        outpc.afrtgt2 = (unsigned char) gl_afrtgt2;

Have you compared the generated code?
make dump
then examine ms3.dmp

The compiler generates identical code.

James

[Shad Laws]

Hi James,

I hadn't tried recompiling it yet... certainly, if the result is the same, then it's not a bug .

But, I did find the issue - it was related to my MS2->MS3 conversion. Since I typically run open loop, I never noticed that my egoType of "Single Wide Band" got translated as "Narrowband" in my MSQ (case of invalid-gets-replaced-by-default, I believe).

That said, I do have one suggestion... can you please revise the comments in ms3.h lines 314-317

Code: Select all

        BaroOption,             // 0=no baro, 1=baro is 1st reading of map (before cranking), (baroCorr)
        // 2=independent barometer (=> EgoOption not 2 or 4)
        EgoOption,              // 0 = no ego;1= nb o2;2=2 nb o2;3=single wbo2;4=dual wbo2. NOTE:
        //  BaroOption MUST be < 2 if EgoOption = 2 or 4.

... to reflect the changes as documented in core.ini lines 521-522?

Code: Select all

      baroCorr        = bits  ,  U08,    97,      [0:1], "None", "Initial MAP Reading", "Two Independent Sensors", "INVALID"; BaroOption
      egoType         = bits  ,  U08,    98,      [0:1], "Disabled", "Narrow Band", "Wide Band", "INVALID"                  ; egoOption

At minimum, it'd help people like me debug why their car is acting strangely...

Thanks!

Take care,
Shad