[bugster_de]

Hi,

not sure, this is a bug and I seem to be not fully able to describe side effects.

In comms.ino in the receiveValue function, there is:
case canbusPage:
pnt_configPage = &configPage9;
//For some reason, TunerStudio is sending offsets greater than the maximum page size. I'm not sure if it's their bug or mine, but the fix is to only update the config page if the offset is less than the maximum size
if (valueOffset < npage_size[currentPage])
{
*((byte *)pnt_configPage + (byte)valueOffset) = newValue;
}
break;

and in globals.h npage_size is defined as:
const int16_t npage_size[11] PROGMEM = {0,128,288,288,128,288,128,240,192,192,192};

And here's the BUT: as npage_size is defined as being part of PROGMEM, access to these variable must be run through a macro.
So doing these:
if (valueOffset < npage_size[currentPage]) always evaulates to false, as npage_size[currentPage] always return 0.
So to my understanding, the if statement should look like this:
if (valueOffset < pgm_read_word_near( npage_size[currentPage] ) ) {

Again not sure on the effect, as only warmupPage and canbusPage have this check.

Let me know, if I can do some more testing on this or if I misunderstood something here.

[noisymime]

This is an interesting question!

On the face of it you're 100% correct that a pgm_read_word_near() or similar would be required, but it's also clear that the code in these case appears to be working. The npage_size array is also used (without a pgm_read_x() call) as part of the new CRC32 check in utils.ino and it's definitely working there as well (Otherwise the CRC check would simply be failing).

The answer appears to be that, because the array is never used via a pointer, is declared constant and the compile options are to unroll loops, the compiler is simply abstracting the whole thing away and replacing the array lookups with literals. All of the lookups to npage_size can actually be performed at compile time (assuming the loops are unrolled) and so that's what the compiler is doing. The array is never actually created.

The safest option here appears to be to simply remove the PROGMEM usage for that array as it's not making any difference anyway. It will avoid a heap of confusion though, and potentially problems if people change compiler options.