Il 12/08/2016 09:52, David Brown ha scritto:> On 12/08/16 00:17, pozz wrote: >> Il 11/08/2016 23:07, David Brown ha scritto: >>> On 11/08/16 16:53, Don Y wrote: >>>> On 8/11/2016 7:12 AM, David Brown wrote: >>>> >>>>> But for the common case of storing a small set of parameters, I usually >>>>> work with a struct definition. Along with the parameters in >> question, I >>>>> include a "magic number" for identification, a "parameter structure >>>>> version number" (very useful for upgrading the parameter set if a later >>>>> version of the firmware uses more parameters), an update serial number, >>>>> a CRC check field, and some space reserved for future additional >>>>> parameters. >>>> >>>> To be clear, you are wrapping *all* of your parameters into a single >>>> "parameters_to_be_saved" struct. >>>> >>>> This can work if they are "configuration parameters" -- data that are >>>> consulted when setting up the application, restarting it, etc. >>> >>> Yes, exactly. I find this to be quite a common situation - but we all >>> agree that different systems have different requirements and therefore >>> different "best" solutions. >>> >>>> >>>> This is different than trying to "save application/process state" >>>> across power outages. In that case, the "state" may be scattered >>>> through many variables dispersed in memory. And, more difficult to >>>> ensure catching a "consistent" snapshot. >>>> >>>> I.e., if *this* variable is changed, are there any others whose state >>>> must be reconciled *as* it is being preserved? >>> >>> Usually in such cases, it is still easy enough to have everything in one >>> structure. At its simplest, all those different state variables >>> scattered around the program can be fields of the one state struct. >> >> "Everything in one structure" means you have a global (as the scope in >> C) struct definition that is accessed by every piece of code in the >> project, even if they are very different section of the project. >> > > Yes, that is correct. It is still modular, structured and organised - > just organised in a different way. It is a simple and clear method, but > works best if the code is being written by a single person rather than a > team working on the different parts of the program. > >> In the past I used this approach with a global struct container: >> /* In one .h file */ >> struct settings_s { >> uint8_t backlight_level; >> enum DisplayTheme theme; >> uint16_t psu_output_voltage_calibration; >> char password[7]; >> ... >> }; >> extern struct settings_s settings; >> >> /* In one .c file */ >> struct settings_s settings; >> >> /* In display module */ >> backligh_set(settings.backlight_level); >> >> Every module that manages one or more persistent parameters should know >> the presence of a global settings variable with an exposed type >> definition (struct settings_s). >> I think this is a bad approach that increase the coupling between >> modules instead of decreasing it as it should be. >> > > Coupling like this has disadvantages, but it is not necessarily a bad > thing. And usually you are going to end up with some sort of coupling > anyway - the display module and the settings module have to exchange > information here somehow. They either do so directly, or they do so via > a third module that co-ordinates the process. Direct coupling can make > it easier to follow what is happening, but is most suited to smaller > programs than larger ones.In your approach all the modules should know exactly which parameters (among theirs) must be considered persistent and which aren't. During initialization, the persistent parameters must be get, from a global struct that includes persistent parameters from other modules too or from a wrapper get_persistent(PERSISTENT_PARAMETER_ID, ...). The global struct is simple and effective, but there is a strong coupling between module and the global struct that could change in a different project where I'd like to reuse the same module. The wrapper function hide the global struct, but introduces another problem. What is the return value of get_persistent()? uint32_t could be a solution for the worst case... however I could have uint64_t parameters or big structs. Another problem with this approach is every module *must* know all the parameters that are classified persistent *for that project*, i.e. this must be loaded from the global struct/wrapper, that are not persistent (they could have a fixed default value set during initialization or changed at run-time through a function). What happens if you change the classification of persistent parameters during developing (it's better to save also the last channel selected by the user so I can restore it at next start-up)? Or the next project needs to consider persistent a parameter that wasn't considered persistent in the current project? In both cases you need to change some things in the module. I'm thinking at another approach. The module starts with default parameters (persistent and not persistent) at init() function. The "nvconfig" (non-volatile configuration) module loads all the parameters from the non-volatile memory (most probably it will be a simple raw C struct) and put them in a *local* container (C struct). Now nvconfig_restore() function will be responsible to set each parameter by calling relevant set() functions of relevant module. Most probably, you already have a set() function to change one parameter. When the user changes a persistent (for that project) parameter, a nvconfig_set_<param_name>() is called. This funcion is a wrapper for the relevant set() function of relevant module. If the new value is accepted, it is copied in the local nvconfig C struct. And, if needed, the nvconfig_save() can be automatically called... otherwise it can be called by the user when he click on the "OK" button. Some piece of code is better than a million words :-) /* nvconfig.c */ #include "display.h" #include "tuner.h" #include "psu.h" #include "audio.h" *static* struct { uint16_t magic; uint16_t sn; struct { uint16_t backlight_level; uint16_t contrast_level; Color background_color; } display; uint16_t psu_calibration_value; struct { uint8_t last_selected_channel; enum TUNER_MODE mode; // analog, digital } tuner; struct { uint16_t volume_level; uint16_t balance; } audio; uint16_t crc16; } nvconfig; static void nvconfig_set_default(); void nvconfig_load(void) { ... restore nvconfig data from non-volatile memory... if (nvconfig.magic != MY_MAGIC_NUMBER) { nvconfig_set_default(); } if (nvconfig.crc16 != crc16(&nvconfig, sizeof(nvconfig))) { nvconfig_set_default(); } } void nvconfig_restore(void) { display_backlight_set(nvconfig.display.backlight_level); display_contrast_set(nvconfig.display.contrast_level); psu_calibrate(nvconfig.psu_calibration_value); tuner_channel_set(nvconfig.tuner.last_selected_channel); tuner_mode_set(nvconfig.tuner.mode); audio_volume_set(nvconfig.audio.volume_level); audio_balance_set(nvconfig.audio.balance); } void nvconfig_volume_set(uint16_t new_value) { if (audio_volume_set(new_value) == OK) { nvconfig.audio.volume_level = new_value; } nvconfig_save(); // If necessary } With this approach, the modules aren't interesting in knowing which parameters are persistent... they doesn't know the existence of the "nvconfig" module and non-volatile memory at all.>>>> [It's unclear what the OP is addressing] >>> >>> Indeed. >> >> Mainly I'm interested in saving configuration parameters/settings (color >> theme, calibration data, backlight level, and so on). >> They are parameters that are changed by the user not so frequent. >> >
What is your strategy to save non volatile data?
Started by ●August 11, 2016
Reply by ●August 12, 20162016-08-12
Reply by ●August 12, 20162016-08-12
On 12/08/16 16:30, pozz wrote:> In your approach all the modules should know exactly which parameters > (among theirs) must be considered persistent and which aren't.True. But /some/ module must know that! (We are considering a relatively simple and static system here. It would be different if you had hundreds of parameters that are mostly the default values, and you only wanted to store those few that are non-default.)> During initialization, the persistent parameters must be get, from a > global struct that includes persistent parameters from other modules too > or from a wrapper get_persistent(PERSISTENT_PARAMETER_ID, ...). >Yes.> The global struct is simple and effective, but there is a strong > coupling between module and the global struct that could change in a > different project where I'd like to reuse the same module.True. It is possible to use multiple include files, or smart pre-processing in your build system, in order to minimise or eliminate this. For example, your re-usable module could be packages as: woozle.h # Normal interface file woozle.c # Normal implementation file woozle_params.inc # Struct include file: struct { uint32_t numberOfWoozles; float woozleScale; } woozle; Then your global settings struct in settings.h looks like this: typedef struct { uint16_t crc; uint16_t magicNumber; ... #include "woozle_params.inc" #include "wotsit_params.inc" ... } settings_t; And your woozle.c code can access it as "currentSettings.woozle.numberOfWoozles". A smart build process could even generate a single "all_params.inc" file from all the "*_params.inc" files to reduce that coupling too.> > The wrapper function hide the global struct, but introduces another > problem. What is the return value of get_persistent()? uint32_t could be > a solution for the worst case... however I could have uint64_t > parameters or big structs.That's one of the reasons I'm not too keen on wrapper functions like this. But it shouldn't be too hard to have a few versions returning different type, or you could use a union. A bigger problem is making sure that you are consistent about the type used for each parameter (preferably in a way that is checked by the compiler as much as possible).> > Another problem with this approach is every module *must* know all the > parameters that are classified persistent *for that project*, i.e. this > must be loaded from the global struct/wrapper, that are not persistent > (they could have a fixed default value set during initialization or > changed at run-time through a function).The rest of the code can /see/ the other modules' parameters - but it can happily ignore them.> What happens if you change the classification of persistent parameters > during developing (it's better to save also the last channel selected by > the user so I can restore it at next start-up)? Or the next project > needs to consider persistent a parameter that wasn't considered > persistent in the current project? > In both cases you need to change some things in the module. > > I'm thinking at another approach. The module starts with default > parameters (persistent and not persistent) at init() function. > The "nvconfig" (non-volatile configuration) module loads all the > parameters from the non-volatile memory (most probably it will be a > simple raw C struct) and put them in a *local* container (C struct). > > Now nvconfig_restore() function will be responsible to set each > parameter by calling relevant set() functions of relevant module. > Most probably, you already have a set() function to change one parameter.This means even more coupling between the nvconfig module and the other modules. But certainly it can be a workable method.> > When the user changes a persistent (for that project) parameter, a > nvconfig_set_<param_name>() is called. This funcion is a wrapper for > the relevant set() function of relevant module. If the new value is > accepted, it is copied in the local nvconfig C struct. And, if needed, > the nvconfig_save() can be automatically called... otherwise it can be > called by the user when he click on the "OK" button. > > Some piece of code is better than a million words :-) > > /* nvconfig.c */ > #include "display.h" > #include "tuner.h" > #include "psu.h" > #include "audio.h" > > *static* struct { > uint16_t magic; > uint16_t sn; > struct { > uint16_t backlight_level; > uint16_t contrast_level; > Color background_color; > } display; > uint16_t psu_calibration_value; > struct { > uint8_t last_selected_channel; > enum TUNER_MODE mode; // analog, digital > } tuner; > struct { > uint16_t volume_level; > uint16_t balance; > } audio; > uint16_t crc16; > } nvconfig; > > static void nvconfig_set_default(); > > void nvconfig_load(void) { > ... restore nvconfig data from non-volatile memory... > if (nvconfig.magic != MY_MAGIC_NUMBER) { > nvconfig_set_default(); > } > if (nvconfig.crc16 != crc16(&nvconfig, sizeof(nvconfig))) { > nvconfig_set_default(); > } > }Minor niggle - you might want that second "if" to be an "else if". There's no point in setting the defaults twice, or checking the crc after you have restored the defaults.> > void nvconfig_restore(void) { > display_backlight_set(nvconfig.display.backlight_level); > display_contrast_set(nvconfig.display.contrast_level); > > psu_calibrate(nvconfig.psu_calibration_value); > > tuner_channel_set(nvconfig.tuner.last_selected_channel); > tuner_mode_set(nvconfig.tuner.mode); > > audio_volume_set(nvconfig.audio.volume_level); > audio_balance_set(nvconfig.audio.balance); > } > > void nvconfig_volume_set(uint16_t new_value) { > if (audio_volume_set(new_value) == OK) { > nvconfig.audio.volume_level = new_value; > } > nvconfig_save(); // If necessary > } > > > With this approach, the modules aren't interesting in knowing which > parameters are persistent... they doesn't know the existence of the > "nvconfig" module and non-volatile memory at all. >Yes. It's another way to arrange things, and may be a better choice for some uses. If this suits you for your project, I see nothing wrong with it.> > >>>>> [It's unclear what the OP is addressing] >>>> >>>> Indeed. >>> >>> Mainly I'm interested in saving configuration parameters/settings (color >>> theme, calibration data, backlight level, and so on). >>> They are parameters that are changed by the user not so frequent. >>> >> >
Reply by ●August 12, 20162016-08-12
On 8/12/2016 12:02 AM, pozz wrote:> Il 12/08/2016 00:30, Don Y ha scritto: >> On 8/11/2016 3:17 PM, pozz wrote: >> >> [much elided] >> >>> "Everything in one structure" means you have a global (as the scope in C) >>> struct definition that is accessed by every piece of code in the >>> project, even >>> if they are very different section of the project. >> >> No. It only has to be *effectively* global. >> >> E.g., you can have a global "update_parameter()" that takes a >> "key" (name for a parameter -- this could be a literal string >> *or* just a small integer) and a parameter value (or pointer to >> a parameter value). Update_parameter() then moves the "value" >> into the referenced portion of the "parameters to be saved struct". > > You use update_parameter() function to only move the value from the variable in > RAM (managed by the module that really uses that variable) in another place in > RAM (the struct that will be saved in a persisten way), right?*You* define the semantics. I.e., each "update_parameter()" *could* cause that parameter to be flushed to FLASH, checksum updated, etc. Or, it could simply cause a cached copy of the appropriate parameter "member" in the struct to be updated. Some *other* mechanism could cause the struct to be flushed to FLASH (e.g., update_parameter(FLUSH)) A lot depends on how often you are twiddling those parameters AND the hardware that forms your persistent store. E.g., if you have BBSRAM available, there is little cost to keeping that struct *in* the BBSRAM. So, an "update" can tweek the struct member and update the checksum in one shot. No "slow writes" to worry about or limited media durability. And, now your power fail handling is localized to *that* routine: take lock mark struct as inconsistent update member compute checksum mark struct as self-consistent release lock If power fails at any time OTHER THAN while the lock is held, the struct is intact -- as indicated by the "consistent" marking. Otherwise, the contents are "suspect". (some, none or all of them may be dubious -- depends on how you've encoded them)> In this case, exactly who call update_parameter() and when?Anyone who wants to update one of these "persistent parameters". E.g., the radio in my car would update "current_station" each time I change stations. In this way, my most recent selection would be preserved if I turned the ignition off unceremoniously. Alternatively, a cached/shadow copy of the "current station" parameter could be maintained in "normal" RAM. And, when the ignition is turned off, the shutdown procedure could quickly issue the update_parameter() calls for all pertinent parameters. It depends on how much "work" you have to do and how much time you have to do it (as well as the hardware capabilities alluded to previously).
Reply by ●August 12, 20162016-08-12
On 8/12/2016 2:54 AM, David Brown wrote:> On 12/08/16 00:30, Don Y wrote: >> On 8/11/2016 3:17 PM, pozz wrote: >> >> [much elided] >> >>> "Everything in one structure" means you have a global (as the scope in C) >>> struct definition that is accessed by every piece of code in the >>> project, even >>> if they are very different section of the project. >> >> No. It only has to be *effectively* global. >> >> E.g., you can have a global "update_parameter()" that takes a >> "key" (name for a parameter -- this could be a literal string >> *or* just a small integer) and a parameter value (or pointer to >> a parameter value). Update_parameter() then moves the "value" >> into the referenced portion of the "parameters to be saved struct". > > First, /don't/ use strings for that sort of thing unless you have no > other choice. And don't use arbitrary integers. Use an enum, that is > defined in one place. That way you get it right. Your code refers to > the parameters using sensible names, rather than numbers, you don't get > overlap or gaps (unless you want them, of course), and you can easily > change things when you need to. And when one part of your program > writes parameter "backgroundColour" and another part reads parameter > "backgroundColor", you get a compile-time error rather than weird > behaviour at run-time.You throw an error at run-time -- because you *still* have to do run-time checks on parameters and their values! And, it would be silly to type out the name of the key in each invocation. Instead, you'd use: #define PARAM_BACKGROUND_COLOR ("bacKgRouNdCoLOR") #define PARAM_FOREGROUND_COLOR ("4GrouNDcoLOUr") etc. How is this any different from: #define PARAM_BACKGROUND_COLOR (1) #define PARAM_FOREGROUND_COLOR (2) etc.? Enums, IMO, are a bad idea -- it's far too easy to slip a new one "in the middle" and effectively bodge the keys associated with all those that follow. I.e., a software revision changes: {PARAM_BACKGROUND_COLOR, PARAM_FOREGROUND_COLOR, ...} to {PARAM_BACKGROUND_COLOR, PARAM_BORDER_COLOR, PARAM_FOREGROUND_COLOR, ...} and now any "legacy" configuration files are effectively unreadable. (How do you refer to "old_PARAM_FOREGROUND_COLOR" in the code that attempts to patch-up a legacy configuration dataset in light of the *new* "PARAM_FOREGROUND_COLOR"? I.e., you end up having to create another enumeration for the legacy representation -- using DIFFERENT names!) By contrast, if you manually enumerate them, you are keenly aware of the original (name,key) mapping. Add new names to the end of the list (who cares if "COLOR_X" and "COLOR_Y" have adjacent keys?!). And, you can simply put a comment after any keys that have been obsoleted over time: #define PARAM_VARIABLE_X (...) // obsolete as per rev x.y.z Strings have value in that you can *see* them in logs: log( "Updating parameter %s to value %s", param, value ) instead of: log( "updating parameter %d to value %d", param, value ) or (ick): log( "Updating parameter %s to value %s", param_to_string(param), value_to_string(value) ) Integers (and enums) suffer in poorly typed languages: update_parameter(PARAM_BACKGROUND_COLOR, VALUE_BROCOLLI); I.e., there's no way to ensure only the values valid for PARAM_BACKGROUND_COLOR are applied to PARAM_BACKGROUND_COLOR! The above function call may succeed (depends on whether VALUE_BROCOLLI happens to coincide with a value for a PARAM_BACKGROUND_COLOR) -- or not. I.e., you still have to rely on run-time checks; you can't do it all at compile time. In my current project, parameters are just fields in an RDBMS table. So, the RDBMS *enforces* "correct" (in the sense of "valid") values. Can't set Background_Color to "Blue" unless "Blue" is a valid choice for *that* field. And, surely can't set it to "Brocolli"! :> And, as the RDBMS is charged with maintaining the integrity of this data, there's no need to *check* it when reading it from the RDBMS (it was checked "on the way in"!) As everything is SQL, I *see* "Blue" not some arbitrary *encoding* of PARAM_BACKGROUND_COLOR_BLUE (which might differ from PARAM_FOREGROUND_COLOR_BLUE -- even though they are the same color!)> But passing the data back and forth through functions like > "update_parameter" and "get_parameter" is an alternative to accessing > the struct directly, and can be easier to manage. It is certainly > convenient if you want to trigger automatic saving of parameters.It creates a private namespace for *just* those parameters. And, also puts a "gatekeeper" in place; limiting "who" can access and alter the parameters stored therein (e.g., password) as well as *when* they can be altered. Even if you don't have any run-time authentication mechanisms, its easy to stub update-parameter() and have it announce each time it is invoked (during development) than to have to set a "watchpoint" on individual parameter struct members.>> In this way, the definition of the struct can remain opaque. >> The same sort of "accessor" can be used to retrieve those >> parameters from the struct -- without exposing the struct >> or the manner in which the individual parameters are "encoded" (!) >> in the struct. > > Yes. > >> E.g., you can pack a bunch of (potentially unrelated) parameters >> into a bitfield within that struct. Data coming *into* it gets >> massaged to manipulate the appropriate fields. Likewise for data >> coming *out*. > > That sounds like a complication that is unlikely to be worth the effort, > but it's possible. In many cases its enough to simply say "all > parameters are 32 bits - it's up to the app if this is 4 characters, a > float, an integer, etc.".Depends on resources available *and* how parameters are accessed. Surely easier to fetch a *set* of related small fields in one access than it is to have to fetch several individual fields (each consisting of lots of 0 bits!) It also hides the representation from the caller. Should I store the time-of-last-update as a time_t? Or, as some sort of string? Or, as something that really only makes sense to the routine that does the updates?>> Finally, because this represents a monitor, of sorts, you can ensure >> atomic operations on that "data to be preserved" -- up to and including >> an action that automatically saves the struct to persistent store >> when power is failing (the rest of the time, "updates" happen to >> the "in-RAM" copy of the struct so you don't incur the costs of >> updating FLASH more often than necessary) > > Also true - it's always important to make sure your data is consistent > at all times.
Reply by ●August 12, 20162016-08-12
pozz wrote:> Many times I work on embedded projects where it is required to save some > data in a non volatile way. Usualy I use external (to the main > microcontroller) serial/parallel EEPROM or internal Flash memory (with a > software layer for EEPROM emulation). > > However my goal isn't to discuss the hw alternatives, but the data > format and strategy. > > I know there are many serialization data format (XML, JSON, Protocol > Buffer, SQLite), but I don't think they are valid solutions for > medium/low end microcontrollers. They are complex to implement and they > aren't really necessary.They call these "serialization formats". The simplest XML solution I've found is ezXML. It's sort of fiddly. JSON is another choice. IMO, JSON is a little easier to look at. This being said, just dotted names in a text file is equivalent to each of these. Line of Class.instance.42.attribute.thing="86.42"\n works just as well.> Data are written and read from the same platform, so the endianess and > byte order are not a problem. >Text formats make this problem go away.> IMHO directly saving a C structure to the non-volatile memory is the > best method.Unless the shape of the struct changes. Having to maintain and feed a version ID to cover all possible struct shapes is a pain.> When you will load bytes from the non volatile memory, you > will have magically your C structure in RAM filled with the saved values. > > Is it the same strategy that you use?I have, but there's some advantage to mapping the struct to a text file of attribute-value pairs through a table of names. How much trouble this is depends on how many attribute value pairs you're storing. -- Les Cargill
Reply by ●August 12, 20162016-08-12
On 12/08/16 18:40, Don Y wrote:> On 8/12/2016 2:54 AM, David Brown wrote: >> On 12/08/16 00:30, Don Y wrote: >>> On 8/11/2016 3:17 PM, pozz wrote: >>> >>> [much elided] >>> >>>> "Everything in one structure" means you have a global (as the scope >>>> in C) >>>> struct definition that is accessed by every piece of code in the >>>> project, even >>>> if they are very different section of the project. >>> >>> No. It only has to be *effectively* global. >>> >>> E.g., you can have a global "update_parameter()" that takes a >>> "key" (name for a parameter -- this could be a literal string >>> *or* just a small integer) and a parameter value (or pointer to >>> a parameter value). Update_parameter() then moves the "value" >>> into the referenced portion of the "parameters to be saved struct". >> >> First, /don't/ use strings for that sort of thing unless you have no >> other choice. And don't use arbitrary integers. Use an enum, that is >> defined in one place. That way you get it right. Your code refers to >> the parameters using sensible names, rather than numbers, you don't get >> overlap or gaps (unless you want them, of course), and you can easily >> change things when you need to. And when one part of your program >> writes parameter "backgroundColour" and another part reads parameter >> "backgroundColor", you get a compile-time error rather than weird >> behaviour at run-time. > > You throw an error at run-time -- because you *still* have to do > run-time checks on parameters and their values! >The difference here is that with checks on parameter values, you can (normally) just set them to safe default values if they occur, and it is something that typically would never happen. But if you have a spelling mistake in your texts, your program is likely to silently ignore storage of changes in parameters.> And, it would be silly to type out the name of the key in each > invocation. Instead, you'd use: > > #define PARAM_BACKGROUND_COLOR ("bacKgRouNdCoLOR") > #define PARAM_FOREGROUND_COLOR ("4GrouNDcoLOUr") > etc. > > How is this any different from: > > #define PARAM_BACKGROUND_COLOR (1) > #define PARAM_FOREGROUND_COLOR (2) > etc.?It is different, because it is - IMHO - daft. Working with strings like this is vastly less efficient, and more error prone. And if you've got a good place to write out these #defines, then you are better off using an enum there with numerical values: enum { paramBackgroundColour, paramForegroundColour, etc. } It's far more efficient, easier on the eye, easier to get IDE help in automatic completion, and easier for the compiler to check (such as compiler warnings if you use switch on an enum but forget some cases). And if you are using C++ rather than C, you can use type-safe strong enums, as well as have better scoping and namespace usage.> > Enums, IMO, are a bad idea -- it's far too easy to slip a new one > "in the middle" and effectively bodge the keys associated with all > those that follow. I.e., a software revision changes: > > {PARAM_BACKGROUND_COLOR, PARAM_FOREGROUND_COLOR, ...} > to > {PARAM_BACKGROUND_COLOR, PARAM_BORDER_COLOR, PARAM_FOREGROUND_COLOR, ...} > > and now any "legacy" configuration files are effectively unreadable.If that is a concern (and it sometimes /is/ a concern), give the enum explicit values: enum { paramBackgroundColour = 1, paramForegroundColour = 2, etc. }> > (How do you refer to "old_PARAM_FOREGROUND_COLOR" in the code that > attempts to patch-up a legacy configuration dataset in light of > the *new* "PARAM_FOREGROUND_COLOR"? I.e., you end up having to create > another enumeration for the legacy representation -- using DIFFERENT > names!)And how is that worse than using different names with #define'd values?> > By contrast, if you manually enumerate them, you are keenly aware of > the original (name,key) mapping. Add new names to the end of the > list (who cares if "COLOR_X" and "COLOR_Y" have adjacent keys?!). > And, you can simply put a comment after any keys that have been obsoleted > over time: > > #define PARAM_VARIABLE_X (...) // obsolete as per rev x.y.z > > Strings have value in that you can *see* them in logs: > log( "Updating parameter %s to value %s", param, value ) > instead of: > log( "updating parameter %d to value %d", param, value ) > or (ick): > log( "Updating parameter %s to value %s", > param_to_string(param), value_to_string(value) ) > > Integers (and enums) suffer in poorly typed languages: > update_parameter(PARAM_BACKGROUND_COLOR, VALUE_BROCOLLI); > > I.e., there's no way to ensure only the values valid for > PARAM_BACKGROUND_COLOR are applied to PARAM_BACKGROUND_COLOR! > The above function call may succeed (depends on whether > VALUE_BROCOLLI happens to coincide with a value for a > PARAM_BACKGROUND_COLOR) -- or not. I.e., you still have to > rely on run-time checks; you can't do it all at compile time. > > In my current project, parameters are just fields in an RDBMS table. > So, the RDBMS *enforces* "correct" (in the sense of "valid") values. > Can't set Background_Color to "Blue" unless "Blue" is a valid choice > for *that* field. And, surely can't set it to "Brocolli"! :> > > And, as the RDBMS is charged with maintaining the integrity of this > data, there's no need to *check* it when reading it from the RDBMS > (it was checked "on the way in"!) > > As everything is SQL, I *see* "Blue" not some arbitrary > *encoding* of PARAM_BACKGROUND_COLOR_BLUE (which might differ > from PARAM_FOREGROUND_COLOR_BLUE -- even though they are the > same color!)I don't disagree that storing things as strings can be useful sometimes. But we are not talking about big, wasteful PC software - we are talking about a small number of parameters in a small embedded program. SQL is too big by orders of magnitude for such a system. It would a different matter if we were talking about an embedded linux system, for example - then you just write your code in Python, use a JSON string to hold a dict of the parameters, and you've got the system working in an hour of developer time. Different tools for different tasks.> >> But passing the data back and forth through functions like >> "update_parameter" and "get_parameter" is an alternative to accessing >> the struct directly, and can be easier to manage. It is certainly >> convenient if you want to trigger automatic saving of parameters. > > It creates a private namespace for *just* those parameters. > And, also puts a "gatekeeper" in place; limiting "who" can > access and alter the parameters stored therein (e.g., password) > as well as *when* they can be altered. > > Even if you don't have any run-time authentication mechanisms, > its easy to stub update-parameter() and have it announce each time > it is invoked (during development) than to have to set a "watchpoint" > on individual parameter struct members. > >>> In this way, the definition of the struct can remain opaque. >>> The same sort of "accessor" can be used to retrieve those >>> parameters from the struct -- without exposing the struct >>> or the manner in which the individual parameters are "encoded" (!) >>> in the struct. >> >> Yes. >> >>> E.g., you can pack a bunch of (potentially unrelated) parameters >>> into a bitfield within that struct. Data coming *into* it gets >>> massaged to manipulate the appropriate fields. Likewise for data >>> coming *out*. >> >> That sounds like a complication that is unlikely to be worth the effort, >> but it's possible. In many cases its enough to simply say "all >> parameters are 32 bits - it's up to the app if this is 4 characters, a >> float, an integer, etc.". > > Depends on resources available *and* how parameters are accessed. > Surely easier to fetch a *set* of related small fields in one > access than it is to have to fetch several individual fields > (each consisting of lots of 0 bits!)If you have the resources to use string keys for the parameters, you have the resources for using individual data items for your bits. And if you have a number of related bits to track together, then use a flags parameter - have the application module keep the flags in a uint and pass them all together.> > It also hides the representation from the caller. Should I store > the time-of-last-update as a time_t? Or, as some sort of string? > Or, as something that really only makes sense to the routine that > does the updates? > >>> Finally, because this represents a monitor, of sorts, you can ensure >>> atomic operations on that "data to be preserved" -- up to and including >>> an action that automatically saves the struct to persistent store >>> when power is failing (the rest of the time, "updates" happen to >>> the "in-RAM" copy of the struct so you don't incur the costs of >>> updating FLASH more often than necessary) >> >> Also true - it's always important to make sure your data is consistent >> at all times. >
Reply by ●August 12, 20162016-08-12
In article <gaydnb_QsOlinzPKnZ2dnUU7-eGdnZ2d@earthlink.com>, jchisolm6 @earthlink.net says...> > On Thu, 11 Aug 2016 14:48:57 +0200, pozz wrote: > > > Many times I work on embedded projects where it is required to save some > > data in a non volatile way. Usualy I use external (to the main > > microcontroller) serial/parallel EEPROM or internal Flash memory (with a > > software layer for EEPROM emulation). > > > > However my goal isn't to discuss the hw alternatives, but the data > > format and strategy. > > > > I know there are many serialization data format (XML, JSON, Protocol > > Buffer, SQLite), but I don't think they are valid solutions for > > medium/low end microcontrollers. They are complex to implement and they > > aren't really necessary. > > Data are written and read from the same platform, so the endianess and > > byte order are not a problem. > > > > IMHO directly saving a C structure to the non-volatile memory is the > > best method. When you will load bytes from the non volatile memory, you > > will have magically your C structure in RAM filled with the saved > > values. > > > > Is it the same strategy that you use? > > I think JSON, XML, etc are over kill for data that will never be > exposed to the outside world. If you are going to have an editable > config file, then yes. > > Writing structs directly to storage can introduce upgrade/downgrade > issues. But depending on the stability of the structs and how you > manage introducing new data sets into the struct it can work. Saving > the entire config data set works well with flash where you are block > erasing, block writing anyway. > > I have always used either on chip or external byte writable EEPROM. > Address zero is always a version id of the data in the eeprom. I decouple > the FW rev from the eeprom rev. All new items are added at the end. > FW rev 1 reads 0x00 to say 0x50 with offsets known. FW rev 2 > reads 0x00-0x50 for existing items, and 0x51... for new items. If I need > to downgrade to FW rev 1 the EEPROM items 0x51 and beyond are simply > ignored.I try to AVOID using struct control parameters (version, magic number, type, byte count, CRC/checksum) to ever have 0, 0xFF, -1 as legal values to avoid various erased block values being confused as valid values. Personally rather than having to have tables of which version has how many bytes, I store byte count as well to compare against software version defaults size to work out which count to useeither they are same or defaults size is smaller so used (downgrade) or external is smaller (upgrade). This works if you are concientious in your> Another simple way is a tag-length-value list. You have a search > routine that reads through the eeprom looking for the specific tag. > > With either of these approaches you have to deal with the issue of > config item "X" is a u_int8 and now needs to be a u_int32. How do you > manage upgrade path fw ver 1 -> fw ver 2 (that needs u_int32) and then > downgrade back to fw ver 1 because of some fatal flaw in ver 2 that will > take you a month to fix and QA. You have to roll back the fw so the > units will be operational until you can fix ver 2.If changing the size of a parameter, I put the NEW type on end and leave old one at last used setting. For your example I would not want new version to use same position in struct, for exact reasons above. That sort of change is going to require code change anyway and really should be noted more than just the change of a declaration. The operations on that parameter If you have to do special conversions then do that based on knowing the different versions between defaults (firmware version) and external version. As per your example upgrade changes type so the default would either need conversion or using new default. This should be part of your update design. I try to work on using new defaults for extra value as the norm as update usually means recalibration or similar task on first use. -- Paul Carpenter | paul@pcserviceselectronics.co.uk <http://www.pcserviceselectronics.co.uk/> PC Services <http://www.pcserviceselectronics.co.uk/pi/> Raspberry Pi Add-ons <http://www.pcserviceselectronics.co.uk/fonts/> Timing Diagram Font <http://www.badweb.org.uk/> For those web sites you hate
Reply by ●August 13, 20162016-08-13
On 8/12/2016 1:40 PM, David Brown wrote:> On 12/08/16 18:40, Don Y wrote: >> On 8/12/2016 2:54 AM, David Brown wrote: >>> On 12/08/16 00:30, Don Y wrote: >>>> On 8/11/2016 3:17 PM, pozz wrote: >>>> >>>> [much elided] >>>> >>>>> "Everything in one structure" means you have a global (as the scope >>>>> in C) >>>>> struct definition that is accessed by every piece of code in the >>>>> project, even >>>>> if they are very different section of the project. >>>> >>>> No. It only has to be *effectively* global. >>>> >>>> E.g., you can have a global "update_parameter()" that takes a >>>> "key" (name for a parameter -- this could be a literal string >>>> *or* just a small integer) and a parameter value (or pointer to >>>> a parameter value). Update_parameter() then moves the "value" >>>> into the referenced portion of the "parameters to be saved struct". >>> >>> First, /don't/ use strings for that sort of thing unless you have no >>> other choice. And don't use arbitrary integers. Use an enum, that is >>> defined in one place. That way you get it right. Your code refers to >>> the parameters using sensible names, rather than numbers, you don't get >>> overlap or gaps (unless you want them, of course), and you can easily >>> change things when you need to. And when one part of your program >>> writes parameter "backgroundColour" and another part reads parameter >>> "backgroundColor", you get a compile-time error rather than weird >>> behaviour at run-time. >> >> You throw an error at run-time -- because you *still* have to do >> run-time checks on parameters and their values! > > The difference here is that with checks on parameter values, you can (normally) > just set them to safe default values if they occur, and it is something that > typically would never happen. > > But if you have a spelling mistake in your texts, your program is likely to > silently ignore storage of changes in parameters.Why would your program silently ignore them? Don't you write ROBUST code? If you tried to set parameter 47 to 982 -- but there were only 42 supported parameters, would your code "silently ignore" this? Or, if parameter 47 *did* exist but expected to be set to a string, would you ignore the attempt to set it to a numeric?>> And, it would be silly to type out the name of the key in each >> invocation. Instead, you'd use: >> >> #define PARAM_BACKGROUND_COLOR ("bacKgRouNdCoLOR") >> #define PARAM_FOREGROUND_COLOR ("4GrouNDcoLOUr") >> etc. >> >> How is this any different from: >> >> #define PARAM_BACKGROUND_COLOR (1) >> #define PARAM_FOREGROUND_COLOR (2) >> etc.? > > It is different, because it is - IMHO - daft. Working with strings like this > is vastly less efficient, and more error prone. And if you've got a goodSo, if you opted to use an int as a tag/key, you'd insist on using values like 1, 2, 3, 4, ... instead of "BACK", "FORE", "BORD", for your tags? (i.e., each of these are 32b integers) And, force yourself to maintain a cheat sheet mapping *arbitrary* integers to these interpretations -- just because your grade school teacher taught you to start counting from '1'? Using strings is "daft" when you don't have the resources (memory/MIPS) to do so. Forcing yourself to use enums/integers when you *do* have the resources is "daft"! (why isn't the windows registry just one big set of binary tuples? why aren't UN*X configuration files encoded in some magical, highly space and time efficient manner -- with a suitable "decoder" to allow for the *infrequent* human use? why do databases give names to fields instead of just 'field1', 'field2', etc.?)> place to write out these #defines, then you are better off using an enum there > with numerical values: > > enum { > paramBackgroundColour, > paramForegroundColour, > etc. > } > > It's far more efficient, easier on the eye, easier to get IDE help in automatic > completion, and easier for the compiler to check (such as compiler warnings if > you use switch on an enum but forget some cases). And if you are using C++ > rather than C, you can use type-safe strong enums, as well as have better > scoping and namespace usage.And, all it ADDS is the requirement that you insert this extra step (and keep it consistently, everywhere you use it). I just got notified of an "error 27". What's that mean? What's the symbolic label associated with that numeric value? *Then*, what does that symbolic label actually *mean*? [Isn't this the same argument applied to "error codes"?]>> Enums, IMO, are a bad idea -- it's far too easy to slip a new one >> "in the middle" and effectively bodge the keys associated with all >> those that follow. I.e., a software revision changes: >> >> {PARAM_BACKGROUND_COLOR, PARAM_FOREGROUND_COLOR, ...} >> to >> {PARAM_BACKGROUND_COLOR, PARAM_BORDER_COLOR, PARAM_FOREGROUND_COLOR, ...} >> >> and now any "legacy" configuration files are effectively unreadable. > > If that is a concern (and it sometimes /is/ a concern), give the enum explicit > values: > > enum { > paramBackgroundColour = 1, > paramForegroundColour = 2, > etc. > }This is then another maintenance issue: ensuring that paramBorderColor gets MANUALLY assigned the correct value regardless of where it appears in the list of enumerations: enum { paramBackgroundColour = 1, paramForegroundColour = 2, paramBorderColor = 27, paramShadowColor = 3, // return to the value that would have been used // had not paramBorderColor been introduced or, alternatively: enum { paramBackgroundColour = 1, paramForegroundColour = 2, paramShadowColor, // 3 ... // manually skip over 24 labels (assuming none of them explicitly // alter their default values) to ensure we're at "27" when the // next label (border color) is encountered paramBorderColor, // 27>> (How do you refer to "old_PARAM_FOREGROUND_COLOR" in the code that >> attempts to patch-up a legacy configuration dataset in light of >> the *new* "PARAM_FOREGROUND_COLOR"? I.e., you end up having to create >> another enumeration for the legacy representation -- using DIFFERENT >> names!) > > And how is that worse than using different names with #define'd values?Because there is no "old_" value! The "old" and "new" are exactly the same! You just add new values to the end and obsolete values "in the middle". #define PARAM_BACKGROUND_COLOR (3) #define PARAM_FOREGROUND_COLOR (4) #define PARAM_BORDER_COLOR (5) #define PARAM_SHADOW_COLOR (6) becomes: #define PARAM_BACKGROUND_COLOR (3) #define PARAM_FOREGROUND_COLOR (4) #define PARAM_BORDER_COLOR (5) // no longer used #define PARAM_SHADOW_COLOR (6) when the border color parameter is obsoleted. So, if you encounter a "configuration struct" that references parameter 5, you know that it is referencing the obsolete border color parameter -- not the *new* "highlight color" that took its place in the list of numerical parameter identifier values.>> By contrast, if you manually enumerate them, you are keenly aware of >> the original (name,key) mapping. Add new names to the end of the >> list (who cares if "COLOR_X" and "COLOR_Y" have adjacent keys?!). >> And, you can simply put a comment after any keys that have been obsoleted >> over time: >> >> #define PARAM_VARIABLE_X (...) // obsolete as per rev x.y.z >> >> Strings have value in that you can *see* them in logs: >> log( "Updating parameter %s to value %s", param, value ) >> instead of: >> log( "updating parameter %d to value %d", param, value ) >> or (ick): >> log( "Updating parameter %s to value %s", >> param_to_string(param), value_to_string(value) ) >> >> Integers (and enums) suffer in poorly typed languages: >> update_parameter(PARAM_BACKGROUND_COLOR, VALUE_BROCOLLI); >> >> I.e., there's no way to ensure only the values valid for >> PARAM_BACKGROUND_COLOR are applied to PARAM_BACKGROUND_COLOR! >> The above function call may succeed (depends on whether >> VALUE_BROCOLLI happens to coincide with a value for a >> PARAM_BACKGROUND_COLOR) -- or not. I.e., you still have to >> rely on run-time checks; you can't do it all at compile time. >> >> In my current project, parameters are just fields in an RDBMS table. >> So, the RDBMS *enforces* "correct" (in the sense of "valid") values. >> Can't set Background_Color to "Blue" unless "Blue" is a valid choice >> for *that* field. And, surely can't set it to "Brocolli"! :> >> >> And, as the RDBMS is charged with maintaining the integrity of this >> data, there's no need to *check* it when reading it from the RDBMS >> (it was checked "on the way in"!) >> >> As everything is SQL, I *see* "Blue" not some arbitrary >> *encoding* of PARAM_BACKGROUND_COLOR_BLUE (which might differ >> from PARAM_FOREGROUND_COLOR_BLUE -- even though they are the >> same color!) > > I don't disagree that storing things as strings can be useful sometimes. But > we are not talking about big, wasteful PC software - we are talking about a > small number of parameters in a small embedded program. SQL is too big byWho said it was a "small embedded program"? Did I miss that, somewhere upthread?? Just because its "embedded" doesn't implicitly make it "small". How many Linux-based appliances still use fstab(5), ethers(5), inetd.conf(5), etc.? Surely they could replace those with "compiled" versions and elide all of that *text* from their configurations! After all, they already *have* run-time code to parse those configuration files and configure the system accordingly; why not just skip the "text processing" and store a bunch of magic integers in a file, someplace?> orders of magnitude for such a system. It would a different matter if we were > talking about an embedded linux system, for example - then you just write your > code in Python, use a JSON string to hold a dict of the parameters, and you've > got the system working in an hour of developer time. Different tools for > different tasks.Exactly. If I tried to replace the *thousands* of "configuration parameters" in my current system with enums/small integers, I (and anyone following after me) would spend all out time consulting cheat sheets to try to remember which parameter controls which set of options. And, of course, document all of that nicely as it evolves. Even the little /pro bono/ job I'm working on this month has enough "settings" to make that a daunting undertaking. Much easier to just say: result = write("/dev/display/refresh", "60") than to sort out which ioctl(2) sets the display refresh rate to 60Hz. Or: result = write("/dev/eia1/ctl", "odd") instead of trying to remember if the manifest constant for parity is PARODD or !PAREVEN. Likewise, the "result" need not be some cryptic value that the developer needs to look up and then translate into a *familiar* form (so all developers use similar reporting messages): if (result != nil) print("Error: " result) [This also seems to be a trend in CONFIGURING devices and apps in other OS's as well: "mixerctl outputs.headphones=+5" ] If I had to provide a set of manuals enumerating all sorts of "error codes", no one would ever attempt to alter the system from its default state. "How do I change the color of the background? *Why* didn't my attempt to change the background to "vilet" [sic] fail?">>>> E.g., you can pack a bunch of (potentially unrelated) parameters >>>> into a bitfield within that struct. Data coming *into* it gets >>>> massaged to manipulate the appropriate fields. Likewise for data >>>> coming *out*. >>> >>> That sounds like a complication that is unlikely to be worth the effort, >>> but it's possible. In many cases its enough to simply say "all >>> parameters are 32 bits - it's up to the app if this is 4 characters, a >>> float, an integer, etc.". >> >> Depends on resources available *and* how parameters are accessed. >> Surely easier to fetch a *set* of related small fields in one >> access than it is to have to fetch several individual fields >> (each consisting of lots of 0 bits!) > > If you have the resources to use string keys for the parameters, you have the > resources for using individual data items for your bits.You're assuming all of my comments apply to single implementation. I'm presenting options that have/can be used for a variety of scenarios. Many are inconsistent with each other (but the OP hadn't indicated which SPECIFIC capabilities/characteristics his system involves)> And if you have a number of related bits to track together, then use a flags > parameter - have the application module keep the flags in a uint and pass them > all together. > >> It also hides the representation from the caller. Should I store >> the time-of-last-update as a time_t? Or, as some sort of string? >> Or, as something that really only makes sense to the routine that >> does the updates? >> >>>> Finally, because this represents a monitor, of sorts, you can ensure >>>> atomic operations on that "data to be preserved" -- up to and including >>>> an action that automatically saves the struct to persistent store >>>> when power is failing (the rest of the time, "updates" happen to >>>> the "in-RAM" copy of the struct so you don't incur the costs of >>>> updating FLASH more often than necessary) >>> >>> Also true - it's always important to make sure your data is consistent >>> at all times.
Reply by ●August 13, 20162016-08-13
On Thu, 11 Aug 2016 14:48:57 +0200, pozz wrote:> Many times I work on embedded projects where it is required to save some > data in a non volatile way. Usualy I use external (to the main > microcontroller) serial/parallel EEPROM or internal Flash memory (with a > software layer for EEPROM emulation). > > However my goal isn't to discuss the hw alternatives, but the data > format and strategy. > > I know there are many serialization data format (XML, JSON, Protocol > Buffer, SQLite), but I don't think they are valid solutions for > medium/low end microcontrollers. They are complex to implement and they > aren't really necessary. > Data are written and read from the same platform, so the endianess and > byte order are not a problem. > > IMHO directly saving a C structure to the non-volatile memory is the > best method. When you will load bytes from the non volatile memory, you > will have magically your C structure in RAM filled with the saved > values. > > Is it the same strategy that you use?And then, when you upgrade your software, your data structure is automatically corrupted! Woo hoo! You can prevent this by assuming that memory will never be corrupted and only growing the structure at the end (which is oh so convenient when, for instance, you want to obsolete a parameter or grow it from one byte to two), or by making 0xff the default value for every byte (again, oh so convenient in the code). Personally, I use a list of data items arranged in records. Each record has a length byte, an ID byte (or word), and data. It's accompanied by software that lets you define the default value for each ID, so when the item isn't found the software automatically gets the factory default (I use C++, so the default value is done as a parameter in a template). It all goes into a block with a 16-bit CRC word at the end. It's extravagant in the use of overhead, but I have yet to come close to using 256 bytes, much less the amount of memory commonly found in a block of flash or an external EEPROM. -- Tim Wescott Control systems, embedded software and circuit design I'm looking for work! See my website if you're interested http://www.wescottdesign.com
Reply by ●August 13, 20162016-08-13
Il 13/08/2016 19:06, Tim Wescott ha scritto:> On Thu, 11 Aug 2016 14:48:57 +0200, pozz wrote: > >> Many times I work on embedded projects where it is required to save some >> data in a non volatile way. Usualy I use external (to the main >> microcontroller) serial/parallel EEPROM or internal Flash memory (with a >> software layer for EEPROM emulation). >> >> However my goal isn't to discuss the hw alternatives, but the data >> format and strategy. >> >> I know there are many serialization data format (XML, JSON, Protocol >> Buffer, SQLite), but I don't think they are valid solutions for >> medium/low end microcontrollers. They are complex to implement and they >> aren't really necessary. >> Data are written and read from the same platform, so the endianess and >> byte order are not a problem. >> >> IMHO directly saving a C structure to the non-volatile memory is the >> best method. When you will load bytes from the non volatile memory, you >> will have magically your C structure in RAM filled with the saved >> values. >> >> Is it the same strategy that you use? > > And then, when you upgrade your software, your data structure is > automatically corrupted! Woo hoo!You can include a "data layout version" that increases when one parameter is added, removed or changed when a new firmware version is created. During loading process, the "data layout version" is checked against the corresponding version of the current firmware. Immediately after an upgrade, the data version doesn't match the firmware version so a "data upgrade" process is triggered. Of course, for complex firmware and persistent data that changes frequently, this task could be tricky: you could have whatever previous data version (or the upgrade *must* be done incrementally from one version to the next one, without gaps).> You can prevent this by assuming that memory will never be corrupted and > only growing the structure at the end (which is oh so convenient when, > for instance, you want to obsolete a parameter or grow it from one byte > to two), or by making 0xff the default value for every byte (again, oh so > convenient in the code). > > Personally, I use a list of data items arranged in records. Each record > has a length byte, an ID byte (or word), and data. It's accompanied by > software that lets you define the default value for each ID, so when the > item isn't found the software automatically gets the factory default (I > use C++, so the default value is done as a parameter in a template).How do you save a variable-length data, for example a string? Do you save always the maximum lenght for that parameter? Otherwise, how do you manage an increase in the length of that parameter (when the length of a string increases)? You can't overwrite only the old value, you need to rewrite the "database" from the beginning.> It > all goes into a block with a 16-bit CRC word at the end. It's > extravagant in the use of overhead, but I have yet to come close to using > 256 bytes, much less the amount of memory commonly found in a block of > flash or an external EEPROM.
