fix knock retard table init

[nmschulte]

column and row definition were swapped w/re: value lookup

fixes #246, #417

---

Andrey was right: #417 (comment)

is this rusefi/rusefi#6370?

Comparison to boost; RPM is columns:

boost:

// init
boostMapOpen.init(config->boostTableOpenLoop, config->boostTpsBins, config->boostRpmBins);
boostMapClosed.init(config->boostTableClosedLoop, config->boostTpsBins, config->boostRpmBins);

// getValue
float openLoop = luaOpenLoopAdd + m_openLoopMap->getValue(rpm, driverIntent.Value);
float target = m_closedLoopTargetMap->getValue(rpm, driverIntent.Value);

knock:

// init
m_maxRetardTable.init(config->maxKnockRetardTable, config->maxKnockRetardRpmBins, config->maxKnockRetardLoadBins);

// getValue
return m_maxRetardTable.getValue(Sensor::getOrZero(SensorType::Rpm), getIgnitionLoad());

[rusefillc]

While you are fixing this defect root cause is order of arguments on the method. See rusefi changes flipping it.

[nmschulte]

fixes #246, #417

I haven't confirmed this; this statement might be an overreach.

I'm also not sure this mistake explains how m_maximumRetard would be zero if the table is entirely non-zero.

---

While you are fixing this defect root cause is order of arguments on the method. See rusefi changes flipping it.

I'll check that out soon; had looked at it just a little before.

As well, the type system should be able to save us from mistakes like this somehow. "traits" naively come to mind but I'm admittedly unfamiliar; the standard kit (pre-traits) should be able to do it though too if I'm not mistaken.

[nmschulte]

@rk-iv has effected this change by altering the TS .ini, like so; remapping the bins and the table axes so the m_maxRetardTable->init logic does what it should and TS editor can be used as intended w/o brain games:

 maxKnockRetardTable = array, U08, 19556, [6x6], "deg", 0.25, 0, 0, 30, 2
-maxKnockRetardLoadBins = array, U08, 19592, [6], "%", 1, 0, 0, 250, 0
-maxKnockRetardRpmBins = array, U08, 19598, [6], "RPM", 100.0, 0, 0, 25000, 0
+maxKnockRetardRpmBins = array, U08, 19592, [6], "%", 1, 0, 0, 250, 0
+maxKnockRetardLoadBins = array, U08, 19598, [6], "RPM", 100.0, 0, 0, 25000, 0

     table = maxKnockRetardTbl, maxKnockRetardMap, "Max knock retard",    1
-        xBins        = maxKnockRetardRpmBins, RPMValue
-        yBins        = maxKnockRetardLoadBins, ignitionLoad
+        xBins        = maxKnockRetardLoadBins, RPMValue
+        yBins        = maxKnockRetardRpmBins, ignitionLoad
         zBins        = maxKnockRetardTable
         gridOrient  = 250,    0, 340 ; Space 123 rotation of grid in degrees.

It doesn't fully solve the problem; the CLFC off->on dance is still required to achieve desired effect.

[nmschulte]

I found that KnockController module had no "initialize" method, and so m_maximumRetard was not being initialized on ECU reset. Added an implementation similar to that of BoostController; probably not perfect but should resolve the issue.

[nmschulte]

note this flips the load/rpm bins to match the current implementation

[mck1117]

not the minimal diff to fix the bug, sigh

[rusefillc]

I found that KnockController module had no "initialize" method, and so m_maximumRetard was not being initialized on ECU reset.

if that's the case, and it seems to be the case, I assume the most important next step is to make sure such a defect never occurs again? rusefi/rusefi#6461

[nmschulte]

I assume the most important next step is to make sure such a defect never occurs again?

I agree, see also #422 (and #421).