Merge pull request #3 from madcow/eeprom-testing

Pass variable structure as argument for memory functions

src/common/memory.c

@@ -1,110 +1,112 @@
 #include "common.h"
 
 #include <util/atomic.h>
-#include <stdio.h>
+#include <string.h>
 
 #define MEM_SIZE        1024
-#define MEM_NUM_CELLS   2
+#define MEM_BLOCK_SIZE  ((int) sizeof(mem_block_t))
-#define MEM_CELL_SIZE   ((int) sizeof(word))
-#define MEM_BLOCK_SIZE  (MEM_NUM_CELLS * MEM_CELL_SIZE + 1)
 #define MEM_MAX_BLOCKS  (MEM_SIZE / MEM_BLOCK_SIZE)
 #define MEM_IS_DIRTY    0x80 // Dirty block marker
 
-// TODO: Automated testing /w multiple write cycles
-// TODO: Accept structure containing variable data
-
 static int   GetUsedBlock(void);
 static int   GetFreeBlock(void);
-static byte  ReadRaw(int addr);
+static void  WriteBlock(int n, mem_block_t *in);
+static void  ReadBlock(int n, mem_block_t *out);
 static void  WriteRaw(int addr, byte data);
+static byte  ReadRaw(int addr);
+
+// TODO: Update and clean up documentation.
+// TODO: Invert sentinel value when memory is full
+// and always set first block as "used". This allows
+// us to reset the flags without erasing any memory.
 
 void MEM_Init(void)
 {
-	// Valid first sentinel?
+	byte flag;
-	if (ReadRaw(0) == 0xFF) {
+
+	flag = ReadRaw(0);
+
+	// We divide the memory space into blocks and
+	// provide each block with a "dirty" flag to
+	// indicate if the block is in use.
+
+	//  /--- used
+	// 80 50 DD EE FF 60 AA BB CC
+	// 00 00 00 00 00 00 00 00 00
+	//  \--- free
+
+	// We initialize the memory first if we don't find
+	// a valid flag at the first sentinel position. This
+	// value can be either a 0x00 or 0x80. Empty memory
+	// on Atmel chips is set to 0xFF so we must not use
+	// that as a flag.
+
+	if (flag != 0x00 && flag != MEM_IS_DIRTY) {
 		Info("Initializing fresh memory...");
-		MEM_Free(); // Set dirty block markers
+		MEM_Free(); // Reset all dirty flags
 	}
 }
 
-void MEM_Read(word *temp, word *dewp)
+void MEM_Write(mem_block_t *in)
 {
-	int bl;
+	int n;
-	word addr;
-	byte msb[2], lsb[2];
 
-	bl = GetUsedBlock();
+	// As we add entries it's trivial to keep track of
+	// the last one: We check the flags for each block in
+	// sequence until we find a "free" one and go back
+	// one step.
 
-	if (bl < 0) {
+	// When we have to write new data but there are no
-		*temp = 0;
+	// "free" entries, we start writing from the first
-		*dewp = 0;
+	// entry and only reset the flags (for now).
-		return; // Empty
-	}
 
-	addr = bl * MEM_BLOCK_SIZE;
+	// EEPROM cells are written by the hardware in a two
+	// step operation: First, the cell is set to all ones,
+	// then the bits to be written (effectively only those
+	// that are 0) are actually written.
 
-	msb[0] = ReadRaw(addr + 1);
+	n = GetFreeBlock();
-	lsb[0] = ReadRaw(addr + 2);
+	if (n < 0) { // Full?
-	msb[1] = ReadRaw(addr + MEM_CELL_SIZE + 1);
-	lsb[1] = ReadRaw(addr + MEM_CELL_SIZE + 2);
-
-	*temp = ((msb[0] & 0xFF) << 8) | (lsb[0] & 0xFF);
-	*dewp = ((msb[1] & 0xFF) << 8) | (lsb[1] & 0xFF);
-}
-
-void MEM_Write(word temp, word dewp)
-{
-	int bl;
-	word addr;
-	byte msb[2], lsb[2];
-
-	bl = GetFreeBlock();
-
-	if (bl < 0) {
-		// Full
 		MEM_Free();
-		bl = 0;
+		n = 0;
 	}
 
-	addr = bl * MEM_BLOCK_SIZE;
+	in->flag = MEM_IS_DIRTY;
+	WriteBlock(n, in);
+}
 
-	msb[0] = (temp >> 8) & 0xff;
+int MEM_Read(mem_block_t *out)
-	lsb[0] = temp & 0xff;
+{
-	msb[1] = (dewp >> 8) & 0xff;
+	int n;
-	lsb[1] = dewp & 0xff;
 
-	WriteRaw(addr, MEM_IS_DIRTY);
+	// To get the current block we search for the next
-	WriteRaw(addr + 1, msb[0]);
+	// free one and go back by one. If we reach the end
-	WriteRaw(addr + 2, lsb[0]);
+	// we return a negative value to indicate empty
-	WriteRaw(addr + MEM_CELL_SIZE + 1, msb[1]);
+	// memory.
-	WriteRaw(addr + MEM_CELL_SIZE + 2, lsb[1]);
+
+	n = GetUsedBlock();
+	if (n < 0) { // Empty?
+		return -1;
+	}
+
+	ReadBlock(n, out);
+
+	return 0;
 }
 
 void MEM_Free(void)
 {
-	int bl;
+	int n;
 
-	// Clear all dirty block markers
+	// For now we clear all dirty block markers but it
-	for (bl = 0; bl < MEM_MAX_BLOCKS; bl++) {
+	// would be better to invert the sentinel value when
-		WriteRaw(bl * MEM_BLOCK_SIZE, 0x0);
+	// we've reached the end (and set the first block to
-		if ((bl % 64) == 0) WDT_Reset();
+	// always be "used"). This means we don't have to
-	}
+	// erase any flags below.
-}
 
-void MEM_Dump(void)
+	for (n = 0; n < MEM_MAX_BLOCKS; n++) {
-{
+		WriteRaw(n * MEM_BLOCK_SIZE, 0x0);
-	byte raw;
+		if ((n % 64) == 0) WDT_Reset();
-	char buf[3*16+1];
-
-	Info("Dumping EEPROM memory:");
-	for (int i = 0; i < 64; i++) {
-		for (int j = 0; j < 16; j++) {
-			raw = ReadRaw(i * 16 + j);
-			sprintf(buf + j * 3, "%02X ", raw);
-		}
-
-		WDT_Reset();
-		Info(buf);
 	}
 }
 
@@ -114,23 +116,51 @@ static int GetUsedBlock(void)
 
 	bl = GetFreeBlock();
 
-	if (bl == 0)
+	if (bl == 0) {
 		return -1;
-	else if (bl < 0)
+	} else if (bl < 0) {
 		return MEM_MAX_BLOCKS;
-	else
+	} else {
 		return bl - 1;
 	}
+}
+
+static void ReadBlock(int n, mem_block_t *out)
+{
+	int addr;
+	byte block[MEM_BLOCK_SIZE];
+
+	addr = n * MEM_BLOCK_SIZE;
+	for (int i = 0; i < MEM_BLOCK_SIZE; i++) {
+		block[i] = ReadRaw(addr + i);
+	}
+
+	memcpy(out, block, sizeof(mem_block_t));
+}
+
+static void WriteBlock(int n, mem_block_t *in)
+{
+	int addr;
+	byte *raw;
+
+	raw = (byte *) in;
+
+	addr = n * MEM_BLOCK_SIZE;
+	for (int i = 0; i < MEM_BLOCK_SIZE; i++) {
+		WriteRaw(addr + i, raw[i]);
+	}
+}
 
 static int GetFreeBlock(void)
 {
-	int bl;
+	int n;
 	byte flag;
 	bool is_free;
 
-	for (bl = 0; bl < MEM_MAX_BLOCKS; bl++) {
+	for (n = 0; n < MEM_MAX_BLOCKS; n++) {
-		// Check dirty flag for current block
+		// Only read first member of struct
-		flag = ReadRaw(bl * MEM_BLOCK_SIZE);
+		flag = ReadRaw(n * MEM_BLOCK_SIZE);
+
 		if (flag != MEM_IS_DIRTY) {
 			is_free = true;
 			break;
@@ -138,7 +168,7 @@ static int GetFreeBlock(void)
 	}
 
 	// Is there any space left?
-	return (is_free) ? bl : -1;
+	return (is_free) ? n : -1;
 }
 
 static void WriteRaw(int addr, byte data)

src/common/memory.h

@@ -1,13 +1,20 @@
 #ifndef MAD_CORE_COMMON_MEMORY_H
 #define MAD_CORE_COMMON_MEMORY_H
 
-#define M_TEMPERATURE  0
+typedef struct mem_block_s mem_block_t;
-#define M_DEWPOINT     1
+
+// Important: Must reset EEPROM memory when the size of
+// mem_block_s is changed. Also the flag value must be
+// kept at the first position of the struct.
+
+struct mem_block_s {
+	byte   flag;
+	float  temp, dewp;
+} __attribute__((packed));
 
 void  MEM_Init(void);
-void  MEM_Write(word temp, word dewp);
+void  MEM_Write(mem_block_t *in);
-void  MEM_Read(word *temp, word *dewp);
+int   MEM_Read(mem_block_t *out);
 void  MEM_Free(void);
-void  MEM_Dump(void);
 
 #endif // MAD_CORE_COMMON_MEMORY_H

src/main.c

@@ -32,6 +32,8 @@ static word dewp, dewp_target;
 
 static int Init(void)
 {
+	mem_block_t mem;
+
 	state = S_IDLE;
 
 	// MOSFETS control things like the heating element
@@ -73,12 +75,17 @@ static int Init(void)
 	// used.
 
 	MEM_Init();
-	MEM_Read(&temp_target, &dewp_target);
-	Info("Persistent memory contains [%d, %d]",
-		temp_target, dewp_target);
 
-	// MEM_Write(20, 60);
+	// mem.temp = 30.50f;
-	// MEM_Dump();
+	// mem.dewp = 15.25f;
+	// MEM_Write(&mem);
+	// MEM_Free();
+
+	if (MEM_Read(&mem) == 0) {
+		Info("Found persistent configuration in EEPROM!");
+		Info("Setting targets TEMP='%.2f', DEWP='%.2f'.",
+			mem.temp, mem.dewp);
+	}
 
 	// There is a possiblity to use interrupt signals
 	// for I2C communication but only as one large