Refactor GMC data processing #444
Conversation
|
diff isn't easy to review, would've been better done in multiple commits with special focus in each. |
|
Sorry, was struggling a bit to get it working - and did not want to commit some kind of intermediate failing status |
|
the current code mix is not pretty (unrelated to this PR), maybe we could rather have something like this:
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. |
|
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. |
|
@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. |
| struct GM_State { | ||
| unsigned long timestamp; | ||
| unsigned long counts; | ||
| unsigned long last_count_timestamp; | ||
| unsigned long hv_pulses; | ||
| }; | ||
|
|
||
| // Events | ||
| const byte HEARTBEAT = 0; | ||
| const byte MEASUREMENT = 1; | ||
| const byte ONE_MINUTE = 2; | ||
|
|
||
| const int savedstates_count = 3; | ||
|
|
||
| static GM_State saved_state[savedstates_count]; | ||
| long event_interval[savedstates_count] = { | ||
| HEARTBEAT_INTERVAL * 1000, // Basic heartbeat interval | ||
| MEASUREMENT_INTERVAL * 1000, // Send measurements to server interval | ||
| 60 * 1000 // 60 sec | ||
| }; |
There was a problem hiding this comment.
somehow i find the names confusing.
| read_GMC(&gm_counts, &gm_count_timestamp, &gm_count_time_between); | ||
|
|
||
| read_THP(current_ms, &have_thp, &temperature, &humidity, &pressure); | ||
|
|
||
| read_hv(&hv_error, &hv_pulses); | ||
| set_status(STATUS_HV, hv_error ? ST_HV_ERROR : ST_HV_OK); | ||
|
|
||
| update_ble_status(); | ||
| int wifi_status = update_wifi_status(); | ||
| setup_ntp(wifi_status); | ||
|
|
||
| update_ble_status(); | ||
|
|
||
| // do any other periodic updates for uplinks | ||
| poll_transmission(); | ||
| read_hv(&hv_error, &hv_pulses); | ||
| set_status(STATUS_HV, hv_error ? ST_HV_ERROR : ST_HV_OK); | ||
|
|
||
| publish(current_ms, gm_counts, gm_count_timestamp, hv_pulses, temperature, humidity, pressure); | ||
| read_GMC(&gm_counts, &gm_count_timestamp, &gm_count_time_between); | ||
| read_THP(current_ms, &have_thp, &temperature, &humidity, &pressure); | ||
|
|
There was a problem hiding this comment.
why did you change order here?
| default: | ||
| break; |
There was a problem hiding this comment.
something missing here?
|
|
||
| const int savedstates_count = 3; | ||
|
|
||
| static GM_State saved_state[savedstates_count]; |
There was a problem hiding this comment.
are the array members in a defined state now, like e.g. is it documented that this is all-zero?
Refactor different functions for GMC processing into single one.