Use more generic interface for barometer and timer

This patch abstracts the global symbols for getting barometer readings behind
a "struct of function pointers" interface as popular in Linux Kernel, U-Boot
and others. This means that unit testing can take place with mocked hardware
peripherals. The old "global" drivers are still available, behind explicit
function calls required to access the now-static functions.

This patch also introduces a timer peripheral software module with the same
model, to support future unit testing of altitude rate etc.

11 files changed, 133 insertions, 36 deletions

diff --git a/Makefile b/Makefile
index 6ba3a4f..809c153 100644
--- a/Makefile
+++ b/Makefile
@@ -69,6 +69,7 @@ REAL_OBJ = \
 	$(REAL_DIR)/display_sim.o \
 	$(REAL_DIR)/altimeter.o \
 	$(REAL_DIR)/util.o \
+	$(REAL_DIR)/timer.o \
 	$(REAL_DIR)/data_manager.o
 
 SIM_OBJ = \
@@ -76,6 +77,7 @@ SIM_OBJ = \
 	$(SIM_DIR)/display_sim.o \
 	$(SIM_DIR)/altimeter.o \
 	$(SIM_DIR)/util.o \
+	$(SIM_DIR)/timer.o \
 	$(SIM_DIR)/data_manager.o
 
 ALL_OBJ = $(TEST_OBJ) $(REAL_OBJ) $(SIM_OBJ)
diff --git a/altimeter.c b/altimeter.c
index 861b054..db45eb7 100644
--- a/altimeter.c
+++ b/altimeter.c
@@ -10,6 +10,8 @@
 
 #include "data_manager.h"
 #include "display.h"
+#include "timer.h"
+#include "barometer.h"
 
 #ifdef WDT_DISABLE
 # warning "WDT disabled in this build"
@@ -47,17 +49,23 @@ ISR(TIMER1_COMPA_vect)
 
 int main(void)
 {
+	struct barometer barometer;
+	struct timer timer;
+
 	/* Initialise display before enabling interrupts */
 	display_init();
 	display_clear();
-	data_manager_init(&dctx);
 
-	DEBUG_INIT;
+	/* get descriptors for system default peripherals */
+	get_system_barometer(&barometer);
+	get_system_timer(&timer);
+
+	timer.init();
+	barometer.init();
 
-	/* Initialise timers for /1024 prescaler, 1 Hz comparator val */
-	TCCR1B |= (1 << CS10) | (1 << CS12) | (1 << WGM12);
-	TIMSK1 |= (1 << OCIE1A);
-	OCR1A = F_CPU / 1024;
+	data_manager_init(&dctx, &timer, &barometer);
+
+	DEBUG_INIT;
 
 #ifdef USBCON
 	/* Disable USB controller if one is present - this spams (latches?) USB_GEN
diff --git a/barometer.h b/barometer.h
index 8bb51f7..2358af7 100644
--- a/barometer.h
+++ b/barometer.h
@@ -1,4 +1,9 @@
 #pragma once
 
-void barometer_init(void);
-float barometer_read(void);
+struct barometer {
+	void (*init)(void);
+	float (*read_pressure)(void);
+	/*float (*read_temperature)(void); */
+};
+
+void get_system_barometer(struct barometer *);
diff --git a/barometer_sim.c b/barometer_sim.c
index c695590..5c5b7d4 100644
--- a/barometer_sim.c
+++ b/barometer_sim.c
@@ -1,3 +1,5 @@
+#include "barometer.h"
+
 /* Dummy pressures to return in sequence, each call to barometer_read */
 static const float pressures[] = {
 	1019.5,
@@ -15,13 +17,21 @@ static const float pressures[] = {
 };
 static int cursor = 0;
 
-void barometer_init(void)
+static void barometer_init(void)
 {
 	cursor = 0;
 }
 
-float barometer_read(void)
+static float barometer_read(void)
 {
 	cursor %= sizeof(pressures)/sizeof(pressures[0]);
 	return pressures[cursor++];
 }
+
+/**/
+
+void get_system_barometer(struct barometer *barometer)
+{
+	barometer->init = barometer_init;
+	barometer->read_pressure = barometer_read;
+}
diff --git a/chibios b/chibios
new file mode 160000
+Subproject 1ce430e49350e38bf02dfc7c58920c345e384a3
diff --git a/data_manager.c b/data_manager.c
index 17abdda..b77ba81 100644
--- a/data_manager.c
+++ b/data_manager.c
@@ -4,6 +4,7 @@
 
 #include "data_manager.h"
 #include "barometer.h"
+#include "timer.h"
 
 
 /* utility functions */
@@ -12,21 +13,20 @@ static float pressure_to_metres_asl(float real, float setting)
 	return 44330.f*(1.f-powf(real/setting, 1/5.255));
 }
 
-void data_manager_init(struct data_ctx *ctx)
+void data_manager_init(struct data_ctx *ctx, struct timer *timer, struct barometer *barometer)
 {
 	ctx->setting = 1013.25;
-	barometer_init();
+	ctx->timer = timer;
+	ctx->barometer = barometer;
 	data_manager_tick(ctx);
 }
 
 void data_manager_tick(struct data_ctx *ctx)
 {
-	/* FIXME alt rate */
-	/* FIXME calculate on demand? */
-	ctx->pressure = barometer_read();
-
-	/* FIXME we need atomic access to alt setting once user can set it. Check
-	 * if AVR allows multiple/nested ISRs to run concurrently, pretty sure not
-	 */
+	/* FIXME altitude rate */
+//	struct data_ctx previous_ctx = *ctx;
+	if (ctx->timer)
+		ctx->timestamp = ctx->timer->get_time();
+	ctx->pressure = ctx->barometer->read_pressure();
 	ctx->altitude = pressure_to_metres_asl(ctx->pressure, ctx->setting);
 }
diff --git a/data_manager.h b/data_manager.h
index 83126f6..2197ef0 100644
--- a/data_manager.h
+++ b/data_manager.h
@@ -1,10 +1,15 @@
 #pragma once
 
+#include <stdint.h>
+
 struct data_ctx {
+	uint16_t timestamp;
 	float setting;
 	float pressure;
 	float altitude;
+	struct timer *timer;
+	struct barometer *barometer;
 };
 
-void data_manager_init(struct data_ctx*);
+void data_manager_init(struct data_ctx*, struct timer*, struct barometer*);
 void data_manager_tick(struct data_ctx*);
diff --git a/menu.h b/menu.h
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/menu.h
@@ -0,0 +1 @@
+
diff --git a/test_data_manager.c b/test_data_manager.c
index f21cc97..00b5a2a 100644
--- a/test_data_manager.c
+++ b/test_data_manager.c
@@ -2,32 +2,41 @@
 
 #include "data_manager.h"
 #include "test_runner.h"
+#include "barometer.h"
 #include "unity.h"
 
+static int mock_baro_called;
+
+static void reset_mock_get_baro(void) {
+	mock_baro_called = 0;
+}
+
+static float set_mock_get_baro(void) {
+	mock_baro_called = 1;
+	return 1.0;
+};
+
+static struct barometer mock_barometer = {
+	.read_pressure = set_mock_get_baro,
+};
+
 RUNNER_DECLARE_TEST(test_data_manager_init_setting)
 {
 	struct data_ctx ctx;
-	data_manager_init(&ctx);
-	TEST_ASSERT_FLOAT_WITHIN(FLT_EPSILON, 1013.25, ctx.setting);
+	reset_mock_get_baro();
+	data_manager_init(&ctx, NULL, &mock_barometer);
+	TEST_ASSERT_EQUAL(1, mock_baro_called);
 }
 
 RUNNER_DECLARE_TEST(test_data_manager_first_readings)
 {
 	struct data_ctx ctx;
-	data_manager_init(&ctx);
-	TEST_ASSERT_FLOAT_WITHIN(FLT_EPSILON, 1019.5, ctx.pressure);
-	data_manager_tick(&ctx);
-	TEST_ASSERT_FLOAT_WITHIN(FLT_EPSILON, 1019.45, ctx.pressure);
-}
+	reset_mock_get_baro();
+	data_manager_init(&ctx, NULL, &mock_barometer);
+	TEST_ASSERT_EQUAL(1, mock_baro_called);
 
-RUNNER_DECLARE_TEST(test_data_manager_reinit)
-{
-	struct data_ctx ctx;
-	/* init 1 */
-	data_manager_init(&ctx);
-	TEST_ASSERT_FLOAT_WITHIN(FLT_EPSILON, 1019.5, ctx.pressure);
 
-	/* check re-init */
-	data_manager_init(&ctx);
-	TEST_ASSERT_FLOAT_WITHIN(FLT_EPSILON, 1019.5, ctx.pressure);
+	reset_mock_get_baro();
+	data_manager_tick(&ctx);
+	TEST_ASSERT_EQUAL(1, mock_baro_called);
 }
diff --git a/timer.c b/timer.c
new file mode 100644
index 0000000..7d82b0f
--- /dev/null
+++ b/timer.c
@@ -0,0 +1,47 @@
+#include <avr/io.h>
+#include <stdint.h>
+
+#include "timer.h"
+
+/* On AVR, two 16-bit timers are used:
+ *   timer1 for emitting a low frequency IRQ to update the display
+ *   timer3 freerunning quickly for timestamping purposes
+ *
+ * XXX note: most of the functions in this module must be called with
+ * interrupts already disabled in order to be safe
+ */
+
+/* XXX hardcoded based upon TCCR3B selecting /256 prescaled clock */
+#define TIMER_HZ (F_CPU / 256)
+
+static void timer_init(void)
+{
+	/* timer1 has /1024 prescaler, 1 Hz comparator val, enable IRQ */
+	TCCR1B |= (1 << CS10) | (1 << CS12) | (1 << WGM12);
+	TIMSK1 |= (1 << OCIE1A);
+	OCR1A = F_CPU / 1024;
+
+	/* timer3 has /256 prescaler, no comparartor (free-running with overflow)
+	 * and no IRQ enabled. /256 chosen so that the counter doesn't overflow
+	 * within a 1 second window */
+
+	TCCR3B |= (1 << CS32);
+}
+
+static float timer_stamps_to_seconds(uint16_t start, uint16_t end)
+{
+	return (float)(end - start) / TIMER_HZ;
+}
+
+static uint16_t timer_get_value(void)
+{
+	return TCNT3;
+}
+
+/**/
+
+void get_system_timer(struct timer *timer)
+{
+	timer->init = timer_init;
+	timer->get_time = timer_get_value;
+}
diff --git a/timer.h b/timer.h
new file mode 100644
index 0000000..2f2e4b3
--- /dev/null
+++ b/timer.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#include <stdint.h>
+
+struct timer {
+	void (*init)(void);
+	uint16_t (*get_time)(void);
+};
+
+void get_system_timer(struct timer *);