SAMPLE_SIZE 1 breaks averaging logic

[ericklein]

Sample size = 1 breaks the averaging logic because /SAMPLE_SIZE is dividing by 1.

[ericklein]

SAMPLE_SIZE logic is also effective one less than expected, e.g., SAMPLE_SIZE 2 triggers on first run, as the logic resets it to 1 on publish, not 0.

[disquisitioner]

Setting SAMPLE_SIZE to 1 (e.g., in config.h) breaks the averaging logic for a couple of reasons, but then the averaging logic is currently broken for a different reason. If left running over an extended period of time the environmental values reported e.g. via Dweet or InfluxDB will gradually drift higher and higher due to a bug that relates to how the averaging logic currently works. (This is because all the external reporting functions use the average values computed, not the current sensor readings.) Recently my Air Quality unit has been reporting temperatures in my dining room in excess of 104F and rising, which is clearly wrong. This error will also make the delta value shown on the display get higher and higher without end, though the current readings shown on the display are correct.

More specifically: a) The averaging logic uses a sampleCounter variable that is stored in and read from NV memory b) The averaging logic retains the most recently computed average and includes it as a data point in computing the next average. In every averaging cycle that previous rolling average is therefore always data point #1 (out of SAMPLE_SIZE total data points). c) The averaging logic divides by SAMPLE_SIZE, not sampleCounter, though sampleCounter is compared to SAMPLE_SIZE to determine when to do the averaging and report those averaged values. This is OK, but dangerous. d) To do the averaging correctly you need to divide by the actual number of measurements, including some number of sensor readings plus the most recently reported average value (stored in and retrieved from NV memory). This means sampleCounter needs to start at 1 since is is pre-incremented (and then stored) as part of the averaging process. e) Lastly, and this is the bug that causes the averaged value to increase monotonically over time and therefore eventually give erroneously high readings), when the sampleCounter variable is reset after a completed averaging cycle the code at line #174 in air_quality.cpp resets it to 0 when it should reset it to 1. (Note that sampleCounter is initialized to 1 in line #569 of air_quality.cpp if it is otherwise not present in NV memory, which is correct.)

Having the sampleCounter reset to 0 after an averaging cycle results in the average being computed incorrectly as it adds the most recently reported average together with SAMPLE_SIZE sensor readings, giving a total of (SAMPLE_SIZE +1) measures, but then that aggregate total is divided by SAMPLE_SIZE. Changing line #174 in air_quality.ino so it initializes sampleCounter to 1 fixes this problem as it results in ((averageTemp + (SAMPLE_SIZE -1) sensor readings) / SAMPLE_SIZE) and causes the unit to begin reporting accurate average values (for all sensors, I might add) via Dweet, InfluxDB, and MQTT.

Alternatively, you could have sampleCounter reset to 0 at the beginning of each cycle (and be initialized to 0 in line #569) but would need to change how sampleCounter and SAMPLE_SIZE are used and compared in implementing the averaging cycle. Given the way the code actually works at present, sampleCounter needs to be initialized to 1, not 0.

It would also be possible to modify the code so setting SAMPLE_SIZE to 1 worked properly by altering the if() logic that tests for when to do the averaging calculation. Having an averaging that's only of 1 sample isn't really averaging, though, so perhaps isn't appealing as a feature of the air_quality software.

[ericklein]

I'll work through your comments and I've already revised much of this code in my ui+avg-logic branch, which I've had in testing for some time. Didn't want to integrate into master until I knew it was solid.

[disquisitioner]

I've only changed one line in my local repo instance of main, and that's to adjust line #174 in air_quality.cpp so it initializes sampleCounter to 1 and not 0. I can abandon that change locally given I'm going to switch to the ui+avg+logic branch to help test the collection of changes made there and which includes your fix for the averaging process.

[ericklein]

Fixed verified in store_rssi branch from master during testing.