Fixes for EEPROM space allocation and mapping.

- We can't count on the first EEPROM scan knowing about everything; new variables may be constructed later so be willing to go back and re-scan the EEPROM if we encounter a new variable that hasn't been loaded yet.
- Sort out where _key actually points (the variable's data) in the EEPROM and get everyone to use the same interpretation.
- Fix return values from ::save and ::load.
- Make it possible to re-save after ::erase_all by de-locating all variables before the EEPROM is blown away.
- Fix args to eeprom_read_block/eeprom_write_block so that we don't spam random memory.

Add unit tests for basic save/load operations.


git-svn-id: https://arducopter.googlecode.com/svn/trunk@1532 f9c3cf11-9bcb-44bc-f272-b75c42450872
This commit is contained in:
DrZiplok@gmail.com 2011-01-23 08:11:53 +00:00
parent 1a2ce433e1
commit 3adec05593
3 changed files with 122 additions and 68 deletions

View File

@ -9,6 +9,14 @@
/// of variables to be passed to blocks for floating point
/// math, memory management, etc.
#if 0
# include <FastSerial.h>
extern "C" { extern void delay(unsigned long); }
# define log(fmt, args...) do {Serial.printf("%s:%d: " fmt "\n", __FUNCTION__, __LINE__ , ##args); delay(100); } while(0)
#else
# define log(fmt, args...)
#endif
#include "AP_Var.h"
// Global constants exported for general use.
@ -23,8 +31,6 @@ AP_Float AP_Float_zero ( 0.0, AP_Var::k_key_none, NULL, AP_Var::k_flag_
AP_Var *AP_Var::_variables;
AP_Var *AP_Var::_grouped_variables;
uint16_t AP_Var::_tail_sentinel;
bool AP_Var::_EEPROM_scanned;
// Constructor for standalone variables
//
@ -141,6 +147,7 @@ bool AP_Var::save(void)
// locate the variable in EEPROM, allocating space as required
if (!_EEPROM_locate(true)) {
log("locate failed");
return false;
}
@ -149,9 +156,11 @@ bool AP_Var::save(void)
// if it fit in the buffer, save it to EEPROM
if (size <= sizeof(vbuf)) {
log("saving %u to %u", size, _key);
eeprom_write_block(vbuf, (void *)_key, size);
return true;
}
return true;
return false;
}
// Load the variable from EEPROM, if supported
@ -168,6 +177,7 @@ bool AP_Var::load(void)
// locate the variable in EEPROM, but do not allocate space
if (!_EEPROM_locate(false)) {
log("locate failed");
return false;
}
@ -181,10 +191,11 @@ bool AP_Var::load(void)
// has converted _key into an EEPROM address.
//
if (size <= sizeof(vbuf)) {
log("loading %u from %u", size, _key);
eeprom_read_block(vbuf, (void *)_key, size);
unserialize(vbuf, size);
return unserialize(vbuf, size);
}
return true;
return false;
}
// Save all variables that don't opt out.
@ -230,15 +241,31 @@ bool AP_Var::load_all(void)
// Erase all variables in EEPROM.
//
// We first walk the variable set and recover their key values
// from EEPROM, so that we have a chance of saving them later.
//
void
AP_Var::erase_all()
{
uint8_t c;
AP_Var *vp;
log("erase EEPROM");
// Scan the list of variables/groups, fetching their key values and
// reverting them to their not-located state.
//
vp = _variables;
while (vp) {
vp->_key = vp->key() | k_key_not_located;
vp = vp->_link;
}
// overwrite the first byte of the header, invalidating the EEPROM
//
c = 0xff;
eeprom_write_byte(&c, 0);
eeprom_write_byte(0, 0xff);
// revert to ignorance about the state of the EEPROM
_tail_sentinel = 0;
}
// Return the key for a variable.
@ -255,8 +282,10 @@ AP_Var::key(void)
return _key & k_key_mask;
}
// read key from EEPROM
eeprom_read_block(&var_header, (void *)_key, sizeof(var_header));
// Read key from EEPROM, note that _key points to the space
// allocated for storage; the header is immediately before.
//
eeprom_read_block(&var_header, (void *)(_key - sizeof(var_header)), sizeof(var_header));
return var_header.key;
}
@ -325,37 +354,50 @@ bool AP_Var::_EEPROM_scan(void)
struct EEPROM_header ee_header;
struct Var_header var_header;
AP_Var *vp;
uint16_t eeprom_address;
// assume that the EEPROM is empty
// Assume that the EEPROM contents are invalid
_tail_sentinel = 0;
// read the header and validate
eeprom_read_block(0, &ee_header, sizeof(ee_header));
eeprom_address = 0;
eeprom_read_block(&ee_header, (void *)eeprom_address, sizeof(ee_header));
if ((ee_header.magic != k_EEPROM_magic) ||
(ee_header.revision != k_EEPROM_revision))
(ee_header.revision != k_EEPROM_revision)) {
log("no header, magic 0x%x revision %u", ee_header.magic, ee_header.revision);
return false;
}
// scan the EEPROM
//
// Avoid trying to read a header when there isn't enough space left.
//
_tail_sentinel = sizeof(ee_header);
while (_tail_sentinel < (k_EEPROM_size - sizeof(var_header) - 1)) {
eeprom_address = sizeof(ee_header);
while (eeprom_address < (k_EEPROM_size - sizeof(var_header) - 1)) {
// read a variable header
eeprom_read_block(&var_header, (void *)_tail_sentinel, sizeof(var_header));
// Read a variable header
//
log("reading header from %u", eeprom_address);
eeprom_read_block(&var_header, (void *)eeprom_address, sizeof(var_header));
// if the header is for the sentinel, scanning is complete
if (var_header.key == k_key_sentinel)
// If the header is for the sentinel, scanning is complete
//
if (var_header.key == k_key_sentinel) {
log("found tail sentinel");
break;
}
// if the variable plus the sentinel would extend past the end of EEPROM, we are done
// Sanity-check the variable header and abort if it looks bad
//
if (k_EEPROM_size <= (
_tail_sentinel + // current position
sizeof(ee_header) + // header for this variable
eeprom_address + // current position
sizeof(var_header) + // header for this variable
var_header.size + 1 + // data for this variable
sizeof(ee_header))) { // header for sentinel
break;
sizeof(var_header))) { // header for sentinel
log("header overruns EEPROM");
return false;
}
// look for a variable with this key
@ -363,18 +405,23 @@ bool AP_Var::_EEPROM_scan(void)
while (vp) {
if (vp->key() == var_header.key) {
// adjust the variable's key to point to this entry
vp->_key = _tail_sentinel;
vp->_key = eeprom_address + sizeof(var_header);
log("update %p with key %u -> %u", vp, var_header.key, vp->_key);
break;
}
vp = vp->_link;
}
if (!vp) {
log("key %u not claimed", var_header.key);
}
// move to the next variable header
_tail_sentinel += sizeof(var_header) + var_header.size + 1;
eeprom_address += sizeof(var_header) + var_header.size + 1;
}
// Scanning is complete
_EEPROM_scanned = true;
log("scan done");
_tail_sentinel = eeprom_address;
return true;
}
@ -389,6 +436,7 @@ bool AP_Var::_EEPROM_locate(bool allocate)
// Is it a group member, or does it have a no-location key?
//
if (_group || (_key == k_key_none)) {
log("not addressable");
return false; // it is/does, and thus it has no location
}
@ -398,15 +446,25 @@ bool AP_Var::_EEPROM_locate(bool allocate)
return true; // it has
}
// If the EEPROM has not been scanned, try that now.
// We don't know where this variable belongs; try scanning the EEPROM
//
if (!_EEPROM_scanned) {
_EEPROM_scan();
// XXX this is going to *suck* for mass-save operations. Do we need
// a flag/key bit that indicates that a variable was known during
// but not located by a scan?
//
log("need scan");
_EEPROM_scan();
// Has the variable now been located?
//
if (!(_key & k_key_not_located)) {
return true; // it has
}
// If not located and not permitted to allocate, we have failed.
//
if ((_key & k_key_not_located) && !allocate) {
if (!allocate) {
log("not found, cannot allocate");
return false;
}
@ -415,26 +473,31 @@ bool AP_Var::_EEPROM_locate(bool allocate)
// space for it.
//
size = serialize(NULL, 0);
if ((size == 0) || (size > k_size_max))
if ((size == 0) || (size > k_size_max)) {
log("size %u out of bounds", size);
return false;
}
// Make sure there will be space in the EEPROM for the variable, its
// header and the new tail sentinel.
//
if ((_tail_sentinel + size + sizeof(Var_header) * 2) > k_EEPROM_size)
if ((_tail_sentinel + size + sizeof(Var_header) * 2) > k_EEPROM_size) {
log("no space in EEPROM");
return false;
}
// If there is no data in the EEPROM, write the header and move the
// sentinel.
//
if (0 == _tail_sentinel) {
log("writing header");
EEPROM_header ee_header;
ee_header.magic = k_EEPROM_magic;
ee_header.revision = k_EEPROM_revision;
ee_header.spare = 0;
eeprom_write_block(0, &ee_header, sizeof(ee_header));
eeprom_write_block(&ee_header, (void *)0, sizeof(ee_header));
_tail_sentinel = sizeof(ee_header);
}
@ -442,8 +505,9 @@ bool AP_Var::_EEPROM_locate(bool allocate)
// Save the location we are going to insert at, and compute the new
// tail sentinel location.
//
_tail_sentinel += sizeof(Var_header) + size;
new_location = _tail_sentinel;
_tail_sentinel += sizeof(Var_header) + size;
log("allocated %u/%u for key %u new sentinel %u", new_location, size, key(), _tail_sentinel);
// Write the new sentinel first. If we are interrupted during this operation
// the old sentinel will still correctly terminate the EEPROM image.
@ -459,8 +523,9 @@ bool AP_Var::_EEPROM_locate(bool allocate)
eeprom_write_block(&var_header, (void *)new_location, sizeof(Var_header));
// We have successfully allocated space and thus located the variable.
// Update _key to point to the space allocated for it.
//
_key = new_location;
_key = new_location + sizeof(var_header);
return true;
}

View File

@ -64,7 +64,7 @@ public:
/// either as a result of scanning or due to allocation of new
/// space in the EEPROM, the k_key_not_located bit will be cleared
/// and the _key value gives the offset into the EEPROM where
/// the variable's header can be found.
/// the variable's data can be found.
///
typedef uint16_t Key;
@ -329,7 +329,6 @@ private:
// EEPROM space allocation and scanning
static uint16_t _tail_sentinel; ///< EEPROM address of the tail sentinel
static bool _EEPROM_scanned; ///< true once the EEPROM has been scanned and addresses assigned
static const uint16_t k_EEPROM_size = 4096; ///< XXX avr-libc doesn't consistently export this

View File

@ -275,40 +275,30 @@ setup(void)
}
}
#if 0
// AP_Var: enumeration
// note that this test presumes the singly-linked list implementation of the list
// AP_Var: save and load
{
TEST(var_enumeration);
TEST(var_save_load);
AP_Var *v = AP_Var::lookup_by_index(0);
AP_Float f1(10.0, 1);
// test basic enumeration
AP_Float f1(0, AP_Var::k_key_none, PSTR("test1"));
REQUIRE(AP_Var::lookup_by_index(0) == &f1);
REQUIRE(AP_Var::lookup_by_index(1) == v);
// test that new entries arrive in order
{
AP_Float f2(0, AP_Var::k_key_none, PSTR("test2"));
REQUIRE(AP_Var::lookup_by_index(0) == &f2);
REQUIRE(AP_Var::lookup_by_index(1) == &f1);
REQUIRE(AP_Var::lookup_by_index(2) == v);
{
AP_Float f3(0, AP_Var::k_key_none, PSTR("test3"));
REQUIRE(AP_Var::lookup_by_index(0) == &f3);
REQUIRE(AP_Var::lookup_by_index(1) == &f2);
REQUIRE(AP_Var::lookup_by_index(2) == &f1);
REQUIRE(AP_Var::lookup_by_index(3) == v);
}
}
// test that destruction removes from the list
REQUIRE(AP_Var::lookup_by_index(0) == &f1);
REQUIRE(AP_Var::lookup_by_index(1) == v);
AP_Var::erase_all();
REQUIRE(true == f1.save());
REQUIRE(f1 == 10.0);
f1 = 0;
REQUIRE(true == f1.load());
REQUIRE(f1 == 10.0);
}
#endif
// AP_Var: reload
{
TEST(reload);
AP_Float f1(0, 1);
REQUIRE(true == f1.load());
REQUIRE(f1 == 10.0);
}
#if SAVE
// AP_Var: load and save