Hi Kevin, Bit of a delayed response!
On Sun, Sep 09, 2018 at 07:18:44PM +0200, Kevin Townsend wrote: > Is there currently an obvious mechanism to persist 'stats' across resets ( > https://mynewt.apache.org/master/os/modules/stats/stats.html), repopulating > them with appropriate values coming out of reset? > > It seems like a potentially common requirement, so I wanted to ping the dev > list before diving into an implementation myself. Has anyone made a viable > attempt at this already, that might be able to share any blocking issues > they ran into? > > There will always be a gray area between the moment you persist the data > (IF you persisted it as well) and the moment you try to restore it, plus > wear on the flash memory since the nRF52 is limited here compared to some > other MCUs), but those are just things you'll have accept and try to > minimise. I agree this would be a very useful feature. Did you end up investigating this? I have written some of my thoughts on the subject below. All comments welcome. I. What gets persisted - individual stats, or entire stat modules? 1. Individual stats: we waste flash by including the module name in every record. 2. Entire stat modules - two options: a. Waste flash persisting unchanged stats, or b. Need to to remember which stats have changed since the last time the module was persisted. This allows us to only persist stats whose values have changed. I think option 2b is the best. There is some implementation effort, but the savings in flash usage is worth it, in my opinion. II. When do we persist? There are a few options. I think the most flexible option is: give each stat module a timer and a configurable "delay". When a stat changes, schedule the module's timer to expire after its configured delay (only if the timer isn't already scheduled). When the timer expires, the module gets persisted. If the user wants stats to be persisted immediately each time they change, the user can configure their stats modules with a delay of 0. For less important stats, a much larger delay can be used. III. Implementation 1. Add a timer to each stats module. An instance of `struct os_callout` is 32 bytes, which I think is a bit too big to burden every stats module with. Instead of adding the callout to `struct stats_hdr`, I propose we "subclass" stats_hdr as follows: struct stats_hdr { char *s_name; uint8_t s_size; uint8_t s_cnt; uint16_t s_flags; /* <-- This used to be padding */ #if MYNEWT_VAL(STATS_NAMES) const struct stats_name_map *s_map; int s_map_cnt; #endif STAILQ_ENTRY(stats_hdr) s_next; }; struct stats_persisted_hdr { struct stats_hdr sp_hdr; struct os_callout sp_callout; os_time_t sp_delay; }; A bit from the new `s_flags` field could indicate whether the stats module is "classic" or "persisted". 2. Remember which stats have changed since being persisted. Each stat needs a "dirty bit." There are two ways to do this: a. Hijack the most-significant-bit of each stat. b. Maintain dirty state in a separate bitmap in the stats module. Option a has some issues: * We lose half the range of each stat. * Looking at these stats in gdb will be quite confusing. Unfortunately, I think we are stuck with option a for now. Option b requires each module to contain a block of memory sized according to the number of stats in the module. The way stats are declared (`STATS_SECT_START` and `STATS_SECT_END`) just doesn't allow for us to do this. This is the same reason we need to define stats names separately with the `STATS_NAME_START` / `STATS_NAME_END` macros). Ultimately, I think we may want to add stats support to the newt tool. Newt could generate the stats definitions C code. This would allow the dirty state to be stored in a bitmap in the module, and it would eliminate the annoyance of defining each stats module twice just to get names. However, I think we should get a working implementation of stat persistence before considering this path. Thanks, Chris
