MultiGeiger icon indicating copy to clipboard operation
MultiGeiger copied to clipboard

Refactor GMC data processing

Open t-pi opened this issue 3 years ago • 5 comments

Refactor different functions for GMC processing into single one.

  • DISPLAYREFRESH --> HEARTBEAT as base processing interval (default 10s)
  • Other events snap to this heartbeat interval
  • Improved handling of future additions (e.g. Telegram signaling) as separate event in single location
  • Removed MINCOUNT as trigger for heartbeat.

t-pi avatar Feb 06 '22 17:02 t-pi

diff isn't easy to review, would've been better done in multiple commits with special focus in each.

ThomasWaldmann avatar Feb 06 '22 18:02 ThomasWaldmann

Sorry, was struggling a bit to get it working - and did not want to commit some kind of intermediate failing status

t-pi avatar Feb 06 '22 19:02 t-pi

the current code mix is not pretty (unrelated to this PR), maybe we could rather have something like this:

  • keep some global global gm counts + thp historical values
  • have some code that ONLY updates it with new data (not calling any other functions)
  • have some code that computes measurements / averages / min / max from the historical data (could be also kept in globals), not calling other functions
  • have some code that sends / alarm-checks measurements

we could do fancy graphical stuff if we had some arrays with gm counts, thp for the last N secs / mins / hours.

guess we'ld need some "ring buffers" to easily keep / update historic values in a limited memory.

ThomasWaldmann avatar Feb 06 '22 19:02 ThomasWaldmann

Agree about the current code mix. But I guess that ship has sailed...

Regarding the variables, at the moment we keep the main counters in the main loop and with this PR the specific historic states as needed per tracked event in the process_GMC function - at least a little bit cleaner than before :)

IMO a more general approach to keep historic data reaches beyond this PR, as several things have to be considered: values to be kept, frequency, aggregation, memory to be spent - and not least why would we do this.

Here I was aiming to prepare a better handling of the Telegram (and other) messages which are not necessarily in sync with the sensor.community etc. updates.

t-pi avatar Feb 06 '22 20:02 t-pi

@ThomasWaldmann would it be possible to merge this PR? I would like to rebase Telegram on it and add MQTT as well as BME680 AQI, too. Or else, what needs to be done prior to a merge? Thanks!

t-pi avatar Feb 10 '22 22:02 t-pi