[WIP] Enhance config system #626
Conversation
There was a problem hiding this comment.
I am concerned about making changes to the config parser without having tests 1st. We need to have high confidence of being able to parse existing model files.
I'd probably prefer to 1st see a PR implementing tests validating the existing system (with whatever minimal changes are needed to write tests), and then have a 2nd PR with the new system.
|
sure. of course, we need tests. My thinking is create several configuration files and read it in. then write it out, read it in again. and validate between stage. |
howard0su
force-pushed
the
config
branch
4 times, most recently
from
da78683
to
503a1d4
Compare
|
do you mind I first get the code of config.* and related tests in? then Display.c, then tx.c, then module.c with the corresponding tests? @PhracturedBlue |
howard0su
force-pushed
the
config
branch
5 times, most recently
from
3e6007c
to
dd682ef
Compare
|
@PhracturedBlue config.* contains new infra to support configure system. it has most part I want and please review. I will work on converting the existing code to new model piece by piece. |
| } | ||
|
|
||
| int write_int2(void* ptr, const struct struct_map *map, int map_size, | ||
| const u16* defaults, int defaults_size, FILE* fh) { |
There was a problem hiding this comment.
can you rename this? from its name i'd expect it to write a single int, but it actually writes a whole struct of them. 'write_ints_from_map' or something more descriptive
There was a problem hiding this comment.
Sure. I will do it later. assign_int -> parse_ini_map, write_int ->write_ini_map, write_int2 ->write_ini_map_defaults
howard0su
force-pushed
the
config
branch
2 times, most recently
from
7d083ba
to
6133a33
Compare
| static const char * const MODEL_TYPE_VAL[MODELTYPE_LAST] = { "heli", "plane", "multi" }; | ||
|
|
||
| #define MATCH_SECTION(s) (strcasecmp(section, s) == 0) | ||
| #define MATCH_START(x,y) (strncasecmp(x, y, sizeof(y)-1) == 0) |
There was a problem hiding this comment.
Lint Error:
33: Missing space after ,
| static const char * const MIXER_MUXTYPE_VAL[MUX_LAST] = { | ||
| "replace", "multiply", "add", "max", "min", "delay", | ||
| #if HAS_EXTENDED_AUDIO | ||
| "beep","voice", |
There was a problem hiding this comment.
Lint Error:
75: Missing space after ,
|
|
||
| static const char MIXER_CURVETYPE[] = "curvetype"; | ||
| static const char * const MIXER_CURVETYPE_VAL[CURVE_MAX+1] = { | ||
| "none", "fixed", "min/max", "zero/max", "greater-than-0", "less-than-0", "absval", |
There was a problem hiding this comment.
Lint Error:
81: Weird number of spaces at line-start. Are you using a 2-space indent?
| { | ||
| int i = 0; | ||
| while(1) { | ||
| if(s1[i] == s2[i] |
There was a problem hiding this comment.
Lint Error:
199: Missing space before ( in while(
200: Missing space before ( in if(
| || (s2[i] == ' ' && s1[i] == '_') | ||
| || (s1[i] == ' ' && s2[i] == '_')) | ||
| { | ||
| if(s1[i] == '\0') |
There was a problem hiding this comment.
Lint Error:
206: Missing space before ( in if(
| const char *tmp; | ||
| char cmp[10]; | ||
| for (i = 0; i <= NUM_SOURCES; i++) { | ||
| if(mapstrcasecmp(INPUT_SourceNameReal(cmp, i), ptr) == 0) { |
There was a problem hiding this comment.
Lint Error:
222: Missing space before ( in if(
| #define SWITCH_NOSTOCK ((1 << INP_HOLD0) | (1 << INP_HOLD1) | \ | ||
| (1 << INP_FMOD0) | (1 << INP_FMOD1)) | ||
| if ((Transmitter.ignore_src & SWITCH_NOSTOCK) == SWITCH_NOSTOCK) { | ||
| if(mapstrcasecmp("FMODE0", ptr) == 0 || |
There was a problem hiding this comment.
Lint Error:
227: Missing space before ( in if(
| mapstrcasecmp("HOLD1", ptr) == 0) | ||
| break; | ||
| } | ||
| #endif //HAS_SWITCHES_NOSTOCK |
There was a problem hiding this comment.
Lint Error:
233: At least two spaces is best between code and comments
233: Should have a space between // and comment
| } | ||
| } | ||
| for (i = 0; i < 4; i++) { | ||
| if(mapstrcasecmp(tx_stick_names[i], ptr) == 0) { |
There was a problem hiding this comment.
Lint Error:
238: Missing space before ( in if(
| } | ||
| i = 0; | ||
| while((tmp = INPUT_MapSourceName(i++, &val))) { | ||
| if(mapstrcasecmp(tmp, ptr) == 0) { |
There was a problem hiding this comment.
Lint Error:
243: Missing space before ( in while(
244: Missing space before ( in if(
| { | ||
| u8 i; | ||
| for (i = 0; i <= NUM_TX_BUTTONS; i++) { | ||
| if(strcasecmp(INPUT_ButtonName(i), value) == 0) { |
There was a problem hiding this comment.
Lint Error:
256: Missing space before ( in if(
| const char **popts = opts; | ||
| int idx = 0; | ||
| while(*popts) { | ||
| if(mapstrcasecmp(*popts, key) == 0) { |
There was a problem hiding this comment.
Lint Error:
268: Missing space before ( in while(
269: Missing space before ( in if(
| int start = exact_atoi(popts[0]); | ||
| int end = exact_atoi(popts[1]); | ||
| int is_num = ((start != 0 || end != 0) && (popts[2] == 0 || (popts[3] == 0 && exact_atoi(popts[2]) != 0))) ? 1 : 0; | ||
| if(is_num) { |
There was a problem hiding this comment.
Lint Error:
274: Missing space before ( in if(
| } | ||
| int val = 0; | ||
| while(popts[val]) { | ||
| if(mapstrcasecmp(popts[val], value) == 0) { |
There was a problem hiding this comment.
Lint Error:
279: Missing space before ( in while(
280: Missing space before ( in if(
| while(*popts) { | ||
| popts++; | ||
| } | ||
| popts++; //Go to next option |
There was a problem hiding this comment.
Lint Error:
289: Should have a space between // and comment
290: Missing space before ( in while(
293: At least two spaces is best between code and comments
293: Should have a space between // and comment
| }; | ||
| static const char * parse_partial_int_list(const char *ptr, void *vals, int *max_count, int type) | ||
| { | ||
| //const char *origptr = ptr; |
There was a problem hiding this comment.
Lint Error:
306: Should have a space between // and comment
| int sign = 0; | ||
|
|
||
| while(1) { | ||
| if(*ptr == ',' || *ptr == '\0') { |
There was a problem hiding this comment.
Lint Error:
311: Missing space before ( in while(
312: Missing space before ( in if(
| --*max_count; | ||
| if (*max_count == 0 || *ptr == '\0') | ||
| return ptr; | ||
| } else if(*ptr == '-') { |
There was a problem hiding this comment.
Lint Error:
335: Missing space before ( in if(
We need reduce the size of the ROM of overall configuration subsystem.
Here is the ideas:
while I am doing refectoring, add the test cases to cover.