From 5468d2fe6e432482cb3112661b2c8c9d814f9811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Garci=CC=81a=20Hierro?= Date: Mon, 9 Jul 2018 00:30:58 +0100 Subject: [PATCH 1/2] Add support for retriving setting types and listing all of them This allows an external caller to retrieve and change any setting as well as listing all the available ones without any a priori knowledge. --- src/main/config/parameter_group_ids.h | 4 + src/main/fc/cli.c | 9 +- src/main/fc/fc_msp.c | 113 +++++++++++++++++++++++++- src/main/fc/settings.c | 39 +++++++++ src/main/fc/settings.h | 15 +++- src/main/msp/msp_protocol_v2_common.h | 11 ++- 6 files changed, 180 insertions(+), 11 deletions(-) diff --git a/src/main/config/parameter_group_ids.h b/src/main/config/parameter_group_ids.h index a3e23b392b..dd37d7466d 100644 --- a/src/main/config/parameter_group_ids.h +++ b/src/main/config/parameter_group_ids.h @@ -113,3 +113,7 @@ #define PG_RESERVED_FOR_TESTING_2 4094 #define PG_RESERVED_FOR_TESTING_3 4093 +#define PG_ID_INVALID 0 +#define PG_ID_FIRST PG_CF_START +#define PG_ID_LAST PG_INAV_END + diff --git a/src/main/fc/cli.c b/src/main/fc/cli.c index 908dbcfb0c..30beb2b319 100755 --- a/src/main/fc/cli.c +++ b/src/main/fc/cli.c @@ -352,14 +352,17 @@ static void printValuePointer(const setting_t *var, const void *valuePointer, ui } break; case MODE_LOOKUP: - if (var->config.lookup.tableIndex < LOOKUP_TABLE_COUNT) { - cliPrintf(settingLookupTables[var->config.lookup.tableIndex].values[value]); + { + const char *name = settingLookupValueName(var, value); + if (name) { + cliPrintf(name); } else { settingGetName(var, buf); - cliPrintLinef("VALUE %s OUT OF RANGE", buf); + cliPrintLinef("VALUE %d OUT OF RANGE FOR %s", (int)value, buf); } break; } + } } static bool valuePtrEqualsDefault(const setting_t *value, const void *ptr, const void *ptrDefault) diff --git a/src/main/fc/fc_msp.c b/src/main/fc/fc_msp.c index aad30560fd..a3dd091d6c 100644 --- a/src/main/fc/fc_msp.c +++ b/src/main/fc/fc_msp.c @@ -35,6 +35,8 @@ #include "common/time.h" #include "common/utils.h" +#include "config/parameter_group_ids.h" + #include "drivers/accgyro/accgyro.h" #include "drivers/bus_i2c.h" #include "drivers/compass/compass.h" @@ -2549,9 +2551,10 @@ static mspResult_e mspFcProcessInCommand(uint16_t cmdMSP, sbuf_t *src) return MSP_RESULT_ACK; } -static const setting_t *mspReadSettingName(sbuf_t *src) +static const setting_t *mspReadSetting(sbuf_t *src) { char name[SETTING_MAX_NAME_LENGTH]; + uint16_t id; uint8_t c; size_t s = 0; while (1) { @@ -2560,6 +2563,14 @@ static const setting_t *mspReadSettingName(sbuf_t *src) } name[s++] = c; if (c == '\0') { + if (s == 1) { + // Payload starts with a zero, setting index + // as uint16_t follows + if (sbufReadU16Safe(&id, src)) { + return settingGet(id); + } + return NULL; + } break; } if (s == SETTING_MAX_NAME_LENGTH) { @@ -2572,7 +2583,7 @@ static const setting_t *mspReadSettingName(sbuf_t *src) static bool mspSettingCommand(sbuf_t *dst, sbuf_t *src) { - const setting_t *setting = mspReadSettingName(src); + const setting_t *setting = mspReadSetting(src); if (!setting) { return false; } @@ -2587,7 +2598,7 @@ static bool mspSetSettingCommand(sbuf_t *dst, sbuf_t *src) { UNUSED(dst); - const setting_t *setting = mspReadSettingName(src); + const setting_t *setting = mspReadSetting(src); if (!setting) { return false; } @@ -2679,6 +2690,94 @@ static bool mspSetSettingCommand(sbuf_t *dst, sbuf_t *src) return true; } +static bool mspSettingInfoCommand(sbuf_t *dst, sbuf_t *src) +{ + const setting_t *setting = mspReadSetting(src); + if (!setting) { + return false; + } + + // Parameter Group ID + sbufWriteU16(dst, settingGetPgn(setting)); + + // Type, section and mode + sbufWriteU8(dst, SETTING_TYPE(setting)); + sbufWriteU8(dst, SETTING_SECTION(setting)); + sbufWriteU8(dst, SETTING_MODE(setting)); + + // Min as int32_t + int32_t min = settingGetMin(setting); + sbufWriteU32(dst, (uint32_t)min); + // Max as uint32_t + uint32_t max = settingGetMax(setting); + sbufWriteU32(dst, max); + + // Absolute setting index + sbufWriteU16(dst, settingGetIndex(setting)); + + // If the setting is profile based, send the current one + // and the count, both as uint8_t. For MASTER_VALUE, we + // send two zeroes, so the MSP client can assume there + // will always be two bytes. + switch (SETTING_SECTION(setting)) { + case MASTER_VALUE: + sbufWriteU8(dst, 0); + sbufWriteU8(dst, 0); + break; + case PROFILE_VALUE: + FALLTHROUGH; + case CONTROL_RATE_VALUE: + sbufWriteU8(dst, getConfigProfile()); + sbufWriteU8(dst, MAX_PROFILE_COUNT); + break; + case BATTERY_CONFIG_VALUE: + sbufWriteU8(dst, getConfigBatteryProfile()); + sbufWriteU8(dst, MAX_BATTERY_PROFILE_COUNT); + break; + } + + // If the setting uses a table, send each possible string (null terminated) + if (SETTING_MODE(setting) == MODE_LOOKUP) { + for (int ii = (int)min; ii <= (int)max; ii++) { + const char *name = settingLookupValueName(setting, ii); + sbufWriteDataSafe(dst, name, strlen(name) + 1); + } + } + + // Finally, include the setting value. This way resource constrained callers + // (e.g. a script in the radio) don't need to perform another call to retrieve + // the value. + const void *ptr = settingGetValuePointer(setting); + size_t size = settingGetValueSize(setting); + sbufWriteDataSafe(dst, ptr, size); + + return true; +} + +static bool mspParameterGroupsCommand(sbuf_t *dst, sbuf_t *src) +{ + uint16_t first; + uint16_t last; + uint16_t start; + uint16_t end; + + if (sbufReadU16Safe(&first, src)) { + last = first; + } else { + first = PG_ID_FIRST; + last = PG_ID_LAST; + } + + for (int ii = first; ii <= last; ii++) { + if (settingsGetParameterGroupIndexes(ii, &start, &end)) { + sbufWriteU16(dst, ii); + sbufWriteU16(dst, start); + sbufWriteU16(dst, end); + } + } + return true; +} + bool mspFCProcessInOutCommand(uint16_t cmdMSP, sbuf_t *dst, sbuf_t *src, mspResult_e *ret) { switch (cmdMSP) { @@ -2705,6 +2804,14 @@ bool mspFCProcessInOutCommand(uint16_t cmdMSP, sbuf_t *dst, sbuf_t *src, mspResu *ret = mspSetSettingCommand(dst, src) ? MSP_RESULT_ACK : MSP_RESULT_ERROR; break; + case MSP2_COMMON_SETTING_INFO: + *ret = mspSettingInfoCommand(dst, src) ? MSP_RESULT_ACK : MSP_RESULT_ERROR; + break; + + case MSP2_COMMON_PG_LIST: + *ret = mspParameterGroupsCommand(dst, src) ? MSP_RESULT_ACK : MSP_RESULT_ERROR; + break; + #if defined(USE_OSD) case MSP2_INAV_OSD_LAYOUTS: if (sbufBytesRemaining(src) >= 1) { diff --git a/src/main/fc/settings.c b/src/main/fc/settings.c index 8c66ebb1f3..d9fd778d89 100644 --- a/src/main/fc/settings.c +++ b/src/main/fc/settings.c @@ -77,6 +77,16 @@ const setting_t *settingFind(const char *name) return NULL; } +const setting_t *settingGet(unsigned index) +{ + return index < SETTINGS_TABLE_COUNT ? &settingsTable[index] : NULL; +} + +unsigned settingGetIndex(const setting_t *val) +{ + return val - settingsTable; +} + size_t settingGetValueSize(const setting_t *val) { switch (SETTING_TYPE(val)) { @@ -154,6 +164,17 @@ setting_max_t settingGetMax(const setting_t *val) return settingMinMaxTable[SETTING_INDEXES_GET_MAX(val)]; } +const char * settingLookupValueName(const setting_t *val, unsigned v) +{ + if (SETTING_MODE(val) == MODE_LOOKUP && val->config.lookup.tableIndex < LOOKUP_TABLE_COUNT) { + const lookupTableEntry_t *table = &settingLookupTables[val->config.lookup.tableIndex]; + if (v < table->valueCount) { + return table->values[v]; + } + } + return NULL; +} + const char * settingGetString(const setting_t *val) { if (SETTING_TYPE(val) == VAR_STRING) { @@ -180,3 +201,21 @@ setting_max_t settingGetStringMaxLength(const setting_t *val) } return 0; } + +bool settingsGetParameterGroupIndexes(pgn_t pg, uint16_t *start, uint16_t *end) +{ + unsigned acc = 0; + for (int ii = 0; ii < SETTINGS_PGN_COUNT; ii++) { + if (settingsPgn[ii] == pg) { + if (start) { + *start = acc; + } + if (end) { + *end = acc + settingsPgnCounts[ii] - 1; + } + return true; + } + acc += settingsPgnCounts[ii]; + } + return false; +} diff --git a/src/main/fc/settings.h b/src/main/fc/settings.h index 6b26ee6445..0140be5816 100644 --- a/src/main/fc/settings.h +++ b/src/main/fc/settings.h @@ -81,6 +81,11 @@ bool settingNameIsExactMatch(const setting_t *val, char *buf, const char *cmdlin // Returns a setting_t with the exact name (case sensitive), or // NULL if no setting with that name exists. const setting_t *settingFind(const char *name); +// Returns the setting at the given index, or NULL if +// the index is greater than the total count. +const setting_t *settingGet(unsigned index); +// Returns the setting index for the given setting. +unsigned settingGetIndex(const setting_t *val); // Returns the size in bytes of the setting value. size_t settingGetValueSize(const setting_t *val); pgn_t settingGetPgn(const setting_t *val); @@ -100,6 +105,10 @@ setting_min_t settingGetMin(const setting_t *val); // depends on the target and build options, but will always be an unsigned // integer (e.g. uintxx_t,) setting_max_t settingGetMax(const setting_t *val); +// Returns the string in the table which corresponds to the value v +// for the given setting. If the setting mode is not MODE_LOOKUP or +// if the value is out of range, it returns NULL. +const char * settingLookupValueName(const setting_t *val, unsigned v); // Returns the setting value as a const char * iff the setting is of type // VAR_STRING. Otherwise it returns NULL. const char * settingGetString(const setting_t *val); @@ -109,4 +118,8 @@ const char * settingGetString(const setting_t *val); void settingSetString(const setting_t *val, const char *s, size_t size); // Returns the max string length (without counting the '\0' terminator) // for setting of type VAR_STRING. Otherwise it returns 0. -setting_max_t settingGetStringMaxLength(const setting_t *val); \ No newline at end of file +setting_max_t settingGetStringMaxLength(const setting_t *val); + +// Retrieve the setting indexes for the given PG. If the PG is not +// found, these function returns false. +bool settingsGetParameterGroupIndexes(pgn_t pg, uint16_t *start, uint16_t *end); \ No newline at end of file diff --git a/src/main/msp/msp_protocol_v2_common.h b/src/main/msp/msp_protocol_v2_common.h index 0f57056f42..98ebdf5e3f 100644 --- a/src/main/msp/msp_protocol_v2_common.h +++ b/src/main/msp/msp_protocol_v2_common.h @@ -15,10 +15,13 @@ * along with INAV. If not, see . */ -#define MSP2_COMMON_TZ 0x1001 //out message Gets the TZ offset for the local time (returns: minutes(i16)) -#define MSP2_COMMON_SET_TZ 0x1002 //in message Sets the TZ offset for the local time (args: minutes(i16)) -#define MSP2_COMMON_SETTING 0x1003 //in/out message Returns the value for a setting -#define MSP2_COMMON_SET_SETTING 0x1004 //in message Sets the value for a setting +#define MSP2_COMMON_TZ 0x1001 //out message Gets the TZ offset for the local time (returns: minutes(i16)) +#define MSP2_COMMON_SET_TZ 0x1002 //in message Sets the TZ offset for the local time (args: minutes(i16)) +#define MSP2_COMMON_SETTING 0x1003 //in/out message Returns the value for a setting +#define MSP2_COMMON_SET_SETTING 0x1004 //in message Sets the value for a setting #define MSP2_COMMON_MOTOR_MIXER 0x1005 #define MSP2_COMMON_SET_MOTOR_MIXER 0x1006 + +#define MSP2_COMMON_SETTING_INFO 0x1007 //in/out message Returns info about a setting (PG, type, flags, min/max, etc..). +#define MSP2_COMMON_PG_LIST 0x1008 //in/out message Returns a list of the PG ids used by the settings \ No newline at end of file From aa8e46b2de4f3c4332c55fdaca9aefbd4c6fa36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Garci=CC=81a=20Hierro?= Date: Mon, 9 Jul 2018 22:36:01 +0100 Subject: [PATCH 2/2] Use new setting accessor functions for retriving settings This reduces flash usage by 96 bytes, making the cost of the whole PR around 300 bytes. Also, the settings tables in settings_generated.c are now made static to make sure no code outside of settings.c can reference them. --- src/main/cms/cms.c | 12 +++--------- src/main/fc/cli.c | 14 +++++++------- src/main/fc/settings.c | 16 +++++++++++----- src/main/fc/settings.h | 7 +++---- src/utils/settings.rb | 6 +++--- 5 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/main/cms/cms.c b/src/main/cms/cms.c index e85b37e745..375b7a82a6 100644 --- a/src/main/cms/cms.c +++ b/src/main/cms/cms.c @@ -428,7 +428,7 @@ static int cmsDrawMenuEntry(displayPort_t *pDisplay, const OSD_Entry *p, uint8_t if (IS_PRINTVALUE(p, screenRow) && p->data) { buff[0] = '\0'; const OSD_SETTING_t *ptr = p->data; - const setting_t *var = &settingsTable[ptr->val]; + const setting_t *var = settingGet(ptr->val); int32_t value; const void *valuePointer = settingGetValuePointer(var); switch (SETTING_TYPE(var)) { @@ -476,13 +476,7 @@ static int cmsDrawMenuEntry(displayPort_t *pDisplay, const OSD_Entry *p, uint8_t break; case MODE_LOOKUP: { - const char *str = NULL; - if (var->config.lookup.tableIndex < LOOKUP_TABLE_COUNT) { - const lookupTableEntry_t *tableEntry = &settingLookupTables[var->config.lookup.tableIndex]; - if (value < tableEntry->valueCount) { - str = tableEntry->values[value]; - } - } + const char *str = settingLookupValueName(var, value); strncpy(buff, str ? str : "INVALID", sizeof(buff) - 1); } break; @@ -1008,7 +1002,7 @@ STATIC_UNIT_TESTED uint16_t cmsHandleKey(displayPort_t *pDisplay, uint8_t key) case OME_Setting: if (p->data) { const OSD_SETTING_t *ptr = p->data; - const setting_t *var = &settingsTable[ptr->val]; + const setting_t *var = settingGet(ptr->val); setting_min_t min = settingGetMin(var); setting_max_t max = settingGetMax(var); float step = ptr->step ?: 1; diff --git a/src/main/fc/cli.c b/src/main/fc/cli.c index 30beb2b319..cd78165788 100755 --- a/src/main/fc/cli.c +++ b/src/main/fc/cli.c @@ -428,8 +428,8 @@ static void dumpPgValue(const setting_t *value, uint8_t dumpMask) static void dumpAllValues(uint16_t valueSection, uint8_t dumpMask) { - for (uint32_t i = 0; i < SETTINGS_TABLE_COUNT; i++) { - const setting_t *value = &settingsTable[i]; + for (unsigned i = 0; i < SETTINGS_TABLE_COUNT; i++) { + const setting_t *value = settingGet(i); bufWriterFlush(cliWriter); if (SETTING_SECTION(value) == valueSection) { dumpPgValue(value, dumpMask); @@ -456,7 +456,7 @@ static void cliPrintVarRange(const setting_t *var) break; case MODE_LOOKUP: { - const lookupTableEntry_t *tableEntry = &settingLookupTables[var->config.lookup.tableIndex]; + const lookupTableEntry_t *tableEntry = settingLookupTable(var); cliPrint("Allowed values:"); for (uint32_t i = 0; i < tableEntry->valueCount ; i++) { if (i > 0) @@ -2259,7 +2259,7 @@ static void cliGet(char *cmdline) char name[SETTING_MAX_NAME_LENGTH]; for (uint32_t i = 0; i < SETTINGS_TABLE_COUNT; i++) { - val = &settingsTable[i]; + val = settingGet(i); if (settingNameContains(val, name, cmdline)) { cliPrintf("%s = ", name); cliPrintVar(val, 0); @@ -2291,7 +2291,7 @@ static void cliSet(char *cmdline) if (len == 0 || (len == 1 && cmdline[0] == '*')) { cliPrintLine("Current settings:"); for (uint32_t i = 0; i < SETTINGS_TABLE_COUNT; i++) { - val = &settingsTable[i]; + val = settingGet(i); settingGetName(val, name); cliPrintf("%s = ", name); cliPrintVar(val, len); // when len is 1 (when * is passed as argument), it will print min/max values as well, for gui @@ -2313,7 +2313,7 @@ static void cliSet(char *cmdline) } for (uint32_t i = 0; i < SETTINGS_TABLE_COUNT; i++) { - val = &settingsTable[i]; + val = settingGet(i); // ensure exact match when setting to prevent setting variables with shorter names if (settingNameIsExactMatch(val, name, cmdline, variableNameLength)) { const setting_type_e type = SETTING_TYPE(val); @@ -2344,7 +2344,7 @@ static void cliSet(char *cmdline) } break; case MODE_LOOKUP: { - const lookupTableEntry_t *tableEntry = &settingLookupTables[settingsTable[i].config.lookup.tableIndex]; + const lookupTableEntry_t *tableEntry = settingLookupTable(val); bool matched = false; for (uint32_t tableValueIndex = 0; tableValueIndex < tableEntry->valueCount && !matched; tableValueIndex++) { matched = sl_strcasecmp(tableEntry->values[tableValueIndex], eqptr) == 0; diff --git a/src/main/fc/settings.c b/src/main/fc/settings.c index d9fd778d89..43375b02cc 100644 --- a/src/main/fc/settings.c +++ b/src/main/fc/settings.c @@ -164,13 +164,19 @@ setting_max_t settingGetMax(const setting_t *val) return settingMinMaxTable[SETTING_INDEXES_GET_MAX(val)]; } -const char * settingLookupValueName(const setting_t *val, unsigned v) +const lookupTableEntry_t * settingLookupTable(const setting_t *val) { if (SETTING_MODE(val) == MODE_LOOKUP && val->config.lookup.tableIndex < LOOKUP_TABLE_COUNT) { - const lookupTableEntry_t *table = &settingLookupTables[val->config.lookup.tableIndex]; - if (v < table->valueCount) { - return table->values[v]; - } + return &settingLookupTables[val->config.lookup.tableIndex]; + } + return NULL; +} + +const char * settingLookupValueName(const setting_t *val, unsigned v) +{ + const lookupTableEntry_t *table = settingLookupTable(val); + if (table && v < table->valueCount) { + return table->values[v]; } return NULL; } diff --git a/src/main/fc/settings.h b/src/main/fc/settings.h index 0140be5816..172f960ce4 100644 --- a/src/main/fc/settings.h +++ b/src/main/fc/settings.h @@ -13,8 +13,6 @@ typedef struct lookupTableEntry_s { const uint8_t valueCount; } lookupTableEntry_t; -extern const lookupTableEntry_t settingLookupTables[]; - #define SETTING_TYPE_OFFSET 0 #define SETTING_SECTION_OFFSET 4 #define SETTING_MODE_OFFSET 6 @@ -69,8 +67,6 @@ typedef struct { } __attribute__((packed)) setting_t; -extern const setting_t settingsTable[]; - static inline setting_type_e SETTING_TYPE(const setting_t *s) { return s->type & SETTING_TYPE_MASK; } static inline setting_section_e SETTING_SECTION(const setting_t *s) { return s->type & SETTING_SECTION_MASK; } static inline setting_mode_e SETTING_MODE(const setting_t *s) { return s->type & SETTING_MODE_MASK; } @@ -105,6 +101,9 @@ setting_min_t settingGetMin(const setting_t *val); // depends on the target and build options, but will always be an unsigned // integer (e.g. uintxx_t,) setting_max_t settingGetMax(const setting_t *val); +// Returns the lookup table for the given setting. If the setting mode +// is not MODE_LOOKUP, it returns NULL; +const lookupTableEntry_t * settingLookupTable(const setting_t *val); // Returns the string in the table which corresponds to the value v // for the given setting. If the setting mode is not MODE_LOOKUP or // if the value is out of range, it returns NULL. diff --git a/src/utils/settings.rb b/src/utils/settings.rb index c42901c87f..b84c6f35b7 100644 --- a/src/utils/settings.rb +++ b/src/utils/settings.rb @@ -468,7 +468,7 @@ class Generator buf << "};\n" end - buf << "const lookupTableEntry_t settingLookupTables[] = {\n" + buf << "static const lookupTableEntry_t settingLookupTables[] = {\n" table_names.each do |name| vn = table_variable_name(name) buf << "\t{ #{vn}, sizeof(#{vn}) / sizeof(char*) },\n" @@ -476,7 +476,7 @@ class Generator buf << "};\n" # Write min/max values table - buf << "const uint32_t settingMinMaxTable[] = {\n" + buf << "static const uint32_t settingMinMaxTable[] = {\n" @value_encoder.values.each do |v| buf << "\t#{v},\n" end @@ -492,7 +492,7 @@ class Generator end # Write setting_t values - buf << "const setting_t settingsTable[] = {\n" + buf << "static const setting_t settingsTable[] = {\n" last_group = nil foreach_enabled_member do |group, member|