From 3abc7c11494b622c17af8a75883e50aea2a9ba7a Mon Sep 17 00:00:00 2001
From: David Bauer <mail@david-bauer.net>
Date: Tue, 23 Jul 2024 15:05:43 +0200
Subject: [PATCH] reporter: cleanup code and remove resubmission

---
 src/reporter.c | 70 +++++++++++++++++++++++++++++++-------------------
 src/reporter.h | 15 ++++++++---
 src/ubus.c     | 12 +--------
 3 files changed, 55 insertions(+), 42 deletions(-)

diff --git a/src/reporter.c b/src/reporter.c
index 2e22f6e..37f4efc 100644
--- a/src/reporter.c
+++ b/src/reporter.c
@@ -6,7 +6,6 @@
 #include "ubus.h"
 #include "udp.h"
 
-#define FOSR_SUBMISSION_RETRY 60
 
 static int fosr_metrics_submit(struct fosr *fosr, struct fosr_metrics *metrics)
 {
@@ -17,9 +16,39 @@ static int fosr_metrics_submit(struct fosr *fosr, struct fosr_metrics *metrics)
 	return fosr_udp_submit(fosr, metrics);
 }
 
+static uint32_t fosr_should_submit(struct fosr *fosr)
+{
+	struct fosr_metrics *current = &fosr->metrics.current;
+	struct fosr_metrics *last_submitted = &fosr->metrics.last_submitted;
+	uint32_t submission_reasons;
+
+	submission_reasons = 0;
+
+	/* Submit if charging-state changed */
+	if (current->charging != last_submitted->charging) {
+		MSG(INFO, "Charging status changed: charging=%u\n", current->charging);
+		submission_reasons |= FOSR_SUBMISSION_REASON_CHARGING;
+	}
+
+	/* Submit if state-of-charge changed on battery */
+	if (!current->charging && current->soc != last_submitted->soc) {
+		MSG(INFO, "SOC changed on battery: soc=%u\n", current->soc);
+		submission_reasons |= FOSR_SUBMISSION_REASON_SOC;
+	}
+
+	/* Submit in configured interval */
+	if (!fosr->last_interval_submission || fosr->sysinfo.uptime - fosr->last_interval_submission > fosr->config.interval) {
+		MSG(INFO, "Submission interval reached\n");
+		submission_reasons |= FOSR_SUBMISSION_REASON_INTERVAL;
+	}
+
+	return submission_reasons;
+}
+
 static void fosr_work(struct uloop_timeout *timeout)
 {
 	struct fosr *fosr = container_of(timeout, struct fosr, work_timeout);
+	uint32_t should_submit;
 	int ret;
 
 	/* Update system uptime */
@@ -28,35 +57,22 @@ static void fosr_work(struct uloop_timeout *timeout)
 	/* Always update battery status */
 	fosr_ubus_battery_status_update(fosr);
 
-	/* Update metrics if changed */
-	if (fosr->metrics_updated) {
-		fosr->metrics_updated = 0;
-		fosr->pending_submission = FOSR_SUBMISSION_RETRY;
-		MSG(DEBUG, "MCU reported changed metrics\n");
-	}
+	/* Check if metrics should be submitted */
+	should_submit = fosr_should_submit(fosr);
 
-	/* Regular update interval */
-	if (!fosr->last_submission || fosr->sysinfo.uptime - fosr->last_submission > fosr->config.interval) {
-		fosr->last_submission = fosr->sysinfo.uptime;
-		fosr->pending_submission = FOSR_SUBMISSION_RETRY;
-		MSG(DEBUG, "Metrics submission interval reached\n");
-	}
+	if (should_submit) {
+		/* Submit metrics */
+		ret = fosr_metrics_submit(fosr, &fosr->metrics.current);
+		if (ret) {
+			MSG(ERROR, "Failed to submit metrics\n");
+		}
 
-	if (fosr->pending_submission) {
-		/* Attempt to send */
-		ret = fosr_metrics_submit(fosr, &fosr->metrics);
-		fosr->pending_submission--;
+		/* Update last submitted metrics*/
+		memcpy(&fosr->metrics.last_submitted, &fosr->metrics.current, sizeof(fosr->metrics.current));
 
-		if (ret) {
-			/* Could not send - Don't spam log despite error */
-			MSG(DEBUG, "Failed to submit metrics\n");
-			if (!fosr->pending_submission) {
-				MSG(ERROR, "Failed to submit metrics - Retries exceeded\n");
-			}
-		} else {
-			MSG(INFO, "Metrics submitted soc=%u, charging=%u, temperature=%.2f\n",
-			    fosr->metrics.soc, fosr->metrics.charging, fosr->metrics.temperature);
-			fosr->pending_submission = 0;
+		/* Update last-interval submission */
+		if (should_submit & FOSR_SUBMISSION_REASON_INTERVAL) {
+			fosr->last_interval_submission = fosr->sysinfo.uptime;
 		}
 	}
 
diff --git a/src/reporter.h b/src/reporter.h
index 2c422b7..8ecfa9f 100644
--- a/src/reporter.h
+++ b/src/reporter.h
@@ -19,6 +19,11 @@ struct fosr_config {
     int32_t interval;
 };
 
+enum fosr_submission_reason {
+    FOSR_SUBMISSION_REASON_SOC = 1 << 0,
+    FOSR_SUBMISSION_REASON_CHARGING = 1 << 1,
+    FOSR_SUBMISSION_REASON_INTERVAL = 1 << 2,
+};
 
 struct fosr {
     struct ubus_context ctx;
@@ -27,11 +32,13 @@ struct fosr {
 
     struct fosr_config config;
 
-    struct fosr_metrics metrics;
-    uint8_t metrics_updated;
+    struct {
+        struct fosr_metrics current;
+        struct fosr_metrics last_submitted;
+    } metrics;
+
 
-    uint8_t pending_submission;
-    unsigned long last_submission;
+    unsigned long last_interval_submission;
 
     struct uloop_timeout work_timeout;
 };
\ No newline at end of file
diff --git a/src/ubus.c b/src/ubus.c
index a7ade01..9553e29 100644
--- a/src/ubus.c
+++ b/src/ubus.c
@@ -57,17 +57,7 @@ static void fosr_ubus_battery_status_update_cb(struct ubus_request *req, int typ
 
 	MSG(DEBUG, "Battery status: soc=%u, charging=%u, temperature=%.2f\n", metrics.soc, metrics.charging, metrics.temperature);
 
-	if (metrics.charging != fosr->metrics.charging) {
-		MSG(INFO, "Charging status changed: charging=%u\n", metrics.charging);
-		fosr->metrics_updated = 1;
-	}
-
-	if (!metrics.charging && metrics.soc != fosr->metrics.soc) {
-		MSG(INFO, "SOC changed on battery: soc=%u\n", metrics.soc);
-		fosr->metrics_updated = 1;
-	}
-
-	memcpy(&fosr->metrics, &metrics, sizeof(metrics));
+	memcpy(&fosr->metrics.current, &metrics, sizeof(metrics));
 }
 
 int fosr_ubus_battery_status_update(struct fosr *fosr)