diff --git a/libraries/AP_Common/AP_Var.cpp b/libraries/AP_Common/AP_Var.cpp index b874166692..d4c049bb8d 100644 --- a/libraries/AP_Common/AP_Var.cpp +++ b/libraries/AP_Common/AP_Var.cpp @@ -421,6 +421,23 @@ bool AP_Var::_EEPROM_scan(void) eeprom_address += sizeof(var_header) + var_header.size + 1; } + // Mark any variables that weren't assigned addresses as not-allocated, + // so that we don't waste time looking for them again later. + // + // Note that this isn't done when the header is not found on an empty EEPROM. + // The first variable written on an empty EEPROM falls out as soon as the + // header is not found. The second will scan and find one variable, then + // mark all the rest as not allocated. + // + vp = _variables; + while (vp) { + if (vp->_key & k_key_not_located) { + vp->_key |= k_key_not_allocated; + log("key %u not allocated", vp->key()); + } + vp = vp->_link; + } + // Scanning is complete log("scan done"); _tail_sentinel = eeprom_address; @@ -448,20 +465,21 @@ bool AP_Var::_EEPROM_locate(bool allocate) return true; // it has } - // We don't know where this variable belongs; try scanning the EEPROM + // We don't know where this variable belongs. If the variable isn't + // marked as already having been looked for and not found in EEPROM, + // try scanning to see if we can locate it. // - // 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(); + if (!(_key & k_key_not_allocated)) { + log("need scan"); + _EEPROM_scan(); - // Has the variable now been located? - // - if (!(_key & k_key_not_located)) { - return true; // it has + // 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 (!allocate) { @@ -508,7 +526,7 @@ bool AP_Var::_EEPROM_locate(bool allocate) // tail sentinel location. // new_location = _tail_sentinel; - _tail_sentinel += sizeof(Var_header) + size; + _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 @@ -522,7 +540,7 @@ bool AP_Var::_EEPROM_locate(bool allocate) // var_header.key = key(); var_header.size = size - 1; - eeprom_write_block(&var_header, (void *)new_location, sizeof(Var_header)); + 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. diff --git a/libraries/AP_Common/AP_Var.h b/libraries/AP_Common/AP_Var.h index fa4bc5eb51..4d35a3a3c7 100644 --- a/libraries/AP_Common/AP_Var.h +++ b/libraries/AP_Common/AP_Var.h @@ -108,6 +108,12 @@ public: /// static const Key k_key_not_located = (Key)1 << 15; + /// A key that has this bit set was not found during a scan of the EEPROM. + /// If this bit is set in the key, it's not useful to scan the EEPROM again + /// in an attempt to find it. + /// + static const Key k_key_not_allocated = (Key)1 << 14; + /// Key assigned to the terminal entry in EEPROM. /// static const Key k_key_sentinel = 0xff; @@ -115,7 +121,7 @@ public: /// A bitmask that removes any control bits from a key giving just the /// value. /// - static const Key k_key_mask = ~(k_key_not_located); + static const Key k_key_mask = ~(k_key_not_located | k_key_not_allocated); /// The largest variable that will be saved to EEPROM. /// This affects the amount of stack space that is required by the ::save, ::load, diff --git a/libraries/AP_Common/examples/AP_Var/AP_Var.pde b/libraries/AP_Common/examples/AP_Var/AP_Var.pde index 036d2da26c..f21c0b0d69 100644 --- a/libraries/AP_Common/examples/AP_Var/AP_Var.pde +++ b/libraries/AP_Common/examples/AP_Var/AP_Var.pde @@ -307,13 +307,16 @@ setup(void) AP_Float f1(10.0, 1); AP_Float f2(123.0, 2); + AP_Int8 i(17, 3); REQUIRE(true == AP_Var::save_all()); f1 = 0; f2 = 0; + i = 0; REQUIRE(true == AP_Var::load_all()); REQUIRE(f1 == 10.0); REQUIRE(f2 == 123.0); + REQUIRE(i == 17); AP_Var::erase_all(); } @@ -339,57 +342,6 @@ setup(void) AP_Var::erase_all(); } -#if SAVE - // AP_Var: load and save - { - TEST(var_load_save); - - AP_Float f1(10, 4); - AP_Float f2(0, 4); - - f2.save(); - f2 = 1.0; - f2.load(); - REQUIRE(f2 == 0); - - f1.save(); - f2.load(); - REQUIRE(f2 == 10); - } - - // AP_Var: group load/save - { - TEST(var_group_loadsave); - - AP_Var_group group(PSTR("group_"), 4); - AP_Float f1(10.0, 8); - AP_Float f2(1.0, 4, PSTR("var"), &group); - - f1.save(); - f1.load(); - REQUIRE(f1 == 10); - - f2.save(); - f2.load(); - REQUIRE(f2 == 1); - - f1.load(); - REQUIRE(f1 == 1); - } - - // AP_Var: derived types - { - TEST(var_derived); - - AP_Float16 f(10.0, 20); - - f.save(); - f = 0; - REQUIRE(f == 0); - f.load(); - REQUIRE(f = 10.0); - } -#endif Test::report(); }