Mirror/inav
1
0
Fork 0
mirror of https://github.com/iNavFlight/inav.git synced 2025-07-26 01:35:35 +03:00
Code Issues Wiki Activity

Revert "[PG] Avoid keeping 2 copies of each PG in RAM"

Browse source 
Unfortunately, making this work properly requires a lot of changes
to the targetConfiguration() system, but still leave us with all
the shortcomings of the PGs.

Working on a new and better way to store the settings seems like
a more productive use of time.

On the bright side, this uncovered a couple of bugs in the
`diff` logic that I will fix on the next commit.

Fixes #5458
This commit is contained in:
Alberto García Hierro 2020-02-28 12:25:05 +00:00
parent aeb8cc68fc
commit 144a7ccc38
4 changed files with 106 additions and 72 deletions
Download patch file Download diff file Expand all files Collapse all files

44
src/main/config/parameter_group.h
View file

@ -22,8 +22,6 @@
#include "build/build_config.h"
#define PG_MAX_SIZE 2048
typedef uint16_t pgn_t;
// parameter group registry flags
@ -47,6 +45,7 @@ typedef struct pgRegistry_s {
pgn_t pgn; // The parameter group number, the top 4 bits are reserved for version
uint16_t size; // Size of the group in RAM, the top 4 bits are reserved for flags
uint8_t *address; // Address of the group in RAM.
uint8_t *copy; // Address of the copy in RAM.
uint8_t **ptr; // The pointer to update after loading the record into ram.
union {
void *ptr; // Pointer to init template
@ -102,37 +101,22 @@ extern const uint8_t __pg_resetdata_end[];
} while (0) \
/**/
#ifdef UNIT_TEST
# define _PG_PROFILE_CURRENT_DECL(_type, _name) \
_type *_name ## _ProfileCurrent = &_name ## _Storage[0];
# define _PG_DEFAULT_DECL(_type, _name)
# define _PG_DEFAULT_I(_type, _name, _png)
#else
# define _PG_PROFILE_CURRENT_DECL(_type, _name) \
_type *_name ## _ProfileCurrent;
# define _PG_DEFAULT_DECL(_type, _name) \
_type* _name ## Default(void *blob);
# define _PG_DEFAULT_I(_type, _name, _pgn) \
_type* _name ## Default(void *blob) { pgResetCopy(blob, _pgn); return (_type*)blob; }
#endif
// Declare system config
#define PG_DECLARE(_type, _name) \
extern _type _name ## _System; \
extern _type _name ## _Copy; \
static inline const _type* _name(void) { return &_name ## _System; }\
static inline _type* _name ## Mutable(void) { return &_name ## _System; }\
_PG_DEFAULT_DECL(_type, _name); \
struct _dummy \
/**/
// Declare system config array
#define PG_DECLARE_ARRAY(_type, _size, _name) \
extern _type _name ## _SystemArray[_size]; \
extern _type _name ## _CopyArray[_size]; \
static inline const _type* _name(int _index) { return &_name ## _SystemArray[_index]; } \
static inline _type* _name ## Mutable(int _index) { return &_name ## _SystemArray[_index]; } \
static inline _type (* _name ## _array(void))[_size] { return &_name ## _SystemArray; } \
_PG_DEFAULT_DECL(_type, _name); \
struct _dummy \
/**/
@ -145,20 +129,17 @@ extern const uint8_t __pg_resetdata_end[];
/**/
#define _PG_ASSERT_MAX_SIZE(_type, _name, _count) unsigned char __pg_ ## _name ## _too_big[sizeof(_type) * _count <= PG_MAX_SIZE ? 0 : -1]
// Register system config
#define PG_REGISTER_I(_type, _name, _pgn, _version, _reset) \
_type _name ## _System; \
_PG_ASSERT_MAX_SIZE(_type, _name, 1); \
_PG_DEFAULT_I(_type, _name, _pgn); \
_type _name ## _Copy; \
/* Force external linkage for g++. Catch multi registration */ \
extern const pgRegistry_t _name ## _Registry; \
const pgRegistry_t _name ##_Registry PG_REGISTER_ATTRIBUTES = { \
.pgn = _pgn | (_version << 12), \
.size = sizeof(_type) | PGR_SIZE_SYSTEM_FLAG, \
.address = (uint8_t*)&_name ## _System, \
.copy = (uint8_t*)&_name ## _Copy, \
.ptr = 0, \
_reset, \
} \
@ -181,13 +162,13 @@ extern const uint8_t __pg_resetdata_end[];
// Register system config array
#define PG_REGISTER_ARRAY_I(_type, _size, _name, _pgn, _version, _reset) \
_type _name ## _SystemArray[_size]; \
_PG_ASSERT_MAX_SIZE(_type, _name, _size); \
_PG_DEFAULT_I(_type, _name, _pgn); \
_type _name ## _CopyArray[_size]; \
extern const pgRegistry_t _name ##_Registry; \
const pgRegistry_t _name ## _Registry PG_REGISTER_ATTRIBUTES = { \
.pgn = _pgn | (_version << 12), \
.size = (sizeof(_type) * _size) | PGR_SIZE_SYSTEM_FLAG, \
.address = (uint8_t*)&_name ## _SystemArray, \
.copy = (uint8_t*)&_name ## _CopyArray, \
.ptr = 0, \
_reset, \
} \
@ -210,16 +191,25 @@ extern const uint8_t __pg_resetdata_end[];
/**/
#endif
#ifdef UNIT_TEST
# define _PG_PROFILE_CURRENT_DECL(_type, _name) \
_type *_name ## _ProfileCurrent = &_name ## _Storage[0];
#else
# define _PG_PROFILE_CURRENT_DECL(_type, _name) \
_type *_name ## _ProfileCurrent;
#endif
// register profile config
#define PG_REGISTER_PROFILE_I(_type, _name, _pgn, _version, _reset) \
STATIC_UNIT_TESTED _type _name ## _Storage[MAX_PROFILE_COUNT]; \
_PG_ASSERT_MAX_SIZE(_type, _name, MAX_PROFILE_COUNT); \
STATIC_UNIT_TESTED _type _name ## _CopyStorage[MAX_PROFILE_COUNT]; \
_PG_PROFILE_CURRENT_DECL(_type, _name) \
extern const pgRegistry_t _name ## _Registry; \
const pgRegistry_t _name ## _Registry PG_REGISTER_ATTRIBUTES = { \
.pgn = _pgn | (_version << 12), \
.size = sizeof(_type) | PGR_SIZE_PROFILE_FLAG, \
.address = (uint8_t*)&_name ## _Storage, \
.copy = (uint8_t*)&_name ## _CopyStorage, \
.ptr = (uint8_t **)&_name ## _ProfileCurrent, \
_reset, \
} \

120
src/main/fc/cli.c
View file

@ -450,14 +450,18 @@ static bool valuePtrEqualsDefault(const setting_t *value, const void *ptr, const
return result;
}
static void dumpPgValue(uint8_t *pgBlob, const setting_t *value, uint8_t dumpMask)
static void dumpPgValue(const setting_t *value, uint8_t dumpMask)
{
char name[SETTING_MAX_NAME_LENGTH];
const char *format = "set %s = ";
const char *defaultFormat = "#set %s = ";
const void *valuePointer = settingGetValuePointer(value);
const void *defaultValuePointer = settingGetDefaultValuePointer(pgBlob, value);
// During a dump, the PGs have been backed up to their "copy"
// regions and the actual values have been reset to its
// defaults. This means that settingGetValuePointer() will
// return the default value while settingGetCopyValuePointer()
// will return the actual value.
const void *valuePointer = settingGetCopyValuePointer(value);
const void *defaultValuePointer = settingGetValuePointer(value);
const bool equalsDefault = valuePtrEqualsDefault(value, valuePointer, defaultValuePointer);
if (((dumpMask & DO_DIFF) == 0) || !equalsDefault) {
settingGetName(value, name);
@ -472,13 +476,13 @@ static void dumpPgValue(uint8_t *pgBlob, const setting_t *value, uint8_t dumpMas
}
}
static void dumpAllValues(uint8_t *pgBlob, uint16_t valueSection, uint8_t dumpMask)
static void dumpAllValues(uint16_t valueSection, uint8_t dumpMask)
{
for (unsigned i = 0; i < SETTINGS_TABLE_COUNT; i++) {
const setting_t *value = settingGet(i);
bufWriterFlush(cliWriter);
if (SETTING_SECTION(value) == valueSection) {
dumpPgValue(pgBlob, value, dumpMask);
dumpPgValue(value, dumpMask);
}
}
}
@ -2604,7 +2608,7 @@ static void cliProfile(char *cmdline)
}
}
static void cliDumpProfile(uint8_t *pgBlob, uint8_t profileIndex, uint8_t dumpMask)
static void cliDumpProfile(uint8_t profileIndex, uint8_t dumpMask)
{
if (profileIndex >= MAX_PROFILE_COUNT) {
// Faulty values
@ -2613,8 +2617,8 @@ static void cliDumpProfile(uint8_t *pgBlob, uint8_t profileIndex, uint8_t dumpMa
setConfigProfile(profileIndex);
cliPrintHashLine("profile");
cliPrintLinef("profile %d\r\n", getConfigProfile() + 1);
dumpAllValues(pgBlob, PROFILE_VALUE, dumpMask);
dumpAllValues(pgBlob, CONTROL_RATE_VALUE, dumpMask);
dumpAllValues(PROFILE_VALUE, dumpMask);
dumpAllValues(CONTROL_RATE_VALUE, dumpMask);
}
static void cliBatteryProfile(char *cmdline)
@ -2632,7 +2636,7 @@ static void cliBatteryProfile(char *cmdline)
}
}
static void cliDumpBatteryProfile(uint8_t *pgBlob, uint8_t profileIndex, uint8_t dumpMask)
static void cliDumpBatteryProfile(uint8_t profileIndex, uint8_t dumpMask)
{
if (profileIndex >= MAX_BATTERY_PROFILE_COUNT) {
// Faulty values
@ -2641,7 +2645,7 @@ static void cliDumpBatteryProfile(uint8_t *pgBlob, uint8_t profileIndex, uint8_t
setConfigBatteryProfile(profileIndex);
cliPrintHashLine("battery_profile");
cliPrintLinef("battery_profile %d\r\n", getConfigBatteryProfile() + 1);
dumpAllValues(pgBlob, BATTERY_CONFIG_VALUE, dumpMask);
dumpAllValues(BATTERY_CONFIG_VALUE, dumpMask);
}
#ifdef USE_CLI_BATCH
@ -3060,10 +3064,31 @@ static void cliResource(char *cmdline)
}
#endif
static void backupConfigs(void)
{
// make copies of configs to do differencing
PG_FOREACH(pg) {
if (pgIsProfile(pg)) {
memcpy(pg->copy, pg->address, pgSize(pg) * MAX_PROFILE_COUNT);
} else {
memcpy(pg->copy, pg->address, pgSize(pg));
}
}
}
static void restoreConfigs(void)
{
PG_FOREACH(pg) {
if (pgIsProfile(pg)) {
memcpy(pg->address, pg->copy, pgSize(pg) * MAX_PROFILE_COUNT);
} else {
memcpy(pg->address, pg->copy, pgSize(pg));
}
}
}
static void printConfig(const char *cmdline, bool doDiff)
{
uint8_t pgBlob[PG_MAX_SIZE] __attribute__((aligned(4)));
uint8_t dumpMask = DUMP_MASTER;
const char *options;
if ((options = checkCommand(cmdline, "master"))) {
@ -3082,6 +3107,15 @@ static void printConfig(const char *cmdline, bool doDiff)
dumpMask = dumpMask | DO_DIFF;
}
const int currentProfileIndexSave = getConfigProfile();
const int currentBatteryProfileIndexSave = getConfigBatteryProfile();
backupConfigs();
// reset all configs to defaults to do differencing
resetConfigs();
// restore the profile indices, since they should not be reset for proper comparison
setConfigProfile(currentProfileIndexSave);
setConfigBatteryProfile(currentBatteryProfileIndexSave);
if (checkCommand(options, "showdefaults")) {
dumpMask = dumpMask | SHOW_DEFAULTS; // add default values as comments for changed values
}
@ -3114,63 +3148,64 @@ static void printConfig(const char *cmdline, bool doDiff)
cliPrintHashLine("mixer");
cliDumpPrintLinef(dumpMask, primaryMotorMixer(0)->throttle == 0.0f, "\r\nmmix reset\r\n");
printMotorMix(dumpMask, primaryMotorMixer(0), primaryMotorMixerDefault(pgBlob));
printMotorMix(dumpMask, primaryMotorMixer_CopyArray, primaryMotorMixer(0));
// print custom servo mixer if exists
cliPrintHashLine("servo mix");
cliDumpPrintLinef(dumpMask, customServoMixers(0)->rate == 0, "smix reset\r\n");
printServoMix(dumpMask, customServoMixers(0), customServoMixersDefault(pgBlob));
printServoMix(dumpMask, customServoMixers_CopyArray, customServoMixers(0));
// print servo parameters
cliPrintHashLine("servo");
printServo(dumpMask, servoParams(0), servoParamsDefault(pgBlob));
printServo(dumpMask, servoParams_CopyArray, servoParams(0));
#ifdef USE_LOGIC_CONDITIONS
cliPrintHashLine("logic");
printLogic(dumpMask, logicConditions(0), logicConditionsDefault(pgBlob));
printLogic(dumpMask, logicConditions_CopyArray, logicConditions(0));
#endif
#ifdef USE_GLOBAL_FUNCTIONS
cliPrintHashLine("gf");
printGlobalFunctions(dumpMask, globalFunctions(0), globalFunctionsDefault(pgBlob));
printGlobalFunctions(dumpMask, globalFunctions_CopyArray, globalFunctions(0));
#endif
cliPrintHashLine("feature");
printFeature(dumpMask, featureConfig(), featureConfigDefault(pgBlob));
printFeature(dumpMask, &featureConfig_Copy, featureConfig());
#ifdef BEEPER
cliPrintHashLine("beeper");
printBeeper(dumpMask, beeperConfig(), beeperConfigDefault(pgBlob));
printBeeper(dumpMask, &beeperConfig_Copy, beeperConfig());
#endif
cliPrintHashLine("map");
printMap(dumpMask, rxConfig(), rxConfigDefault(pgBlob));
printMap(dumpMask, &rxConfig_Copy, rxConfig());
cliPrintHashLine("serial");
printSerial(dumpMask, serialConfig(), serialConfigDefault(pgBlob));
printSerial(dumpMask, &serialConfig_Copy, serialConfig());
#ifdef USE_LED_STRIP
cliPrintHashLine("led");
printLed(dumpMask, ledStripConfig()->ledConfigs, ledStripConfigDefault(pgBlob)->ledConfigs);
printLed(dumpMask, ledStripConfig_Copy.ledConfigs, ledStripConfig()->ledConfigs);
cliPrintHashLine("color");
printColor(dumpMask, ledStripConfig()->colors, ledStripConfigDefault(pgBlob)->colors);
printColor(dumpMask, ledStripConfig_Copy.colors, ledStripConfig()->colors);
cliPrintHashLine("mode_color");
printModeColor(dumpMask, ledStripConfig(), ledStripConfigDefault(pgBlob));
printModeColor(dumpMask, &ledStripConfig_Copy, ledStripConfig());
#endif
cliPrintHashLine("aux");
printAux(dumpMask, modeActivationConditions(0), modeActivationConditionsDefault(pgBlob));
printAux(dumpMask, modeActivationConditions_CopyArray, modeActivationConditions(0));
cliPrintHashLine("adjrange");
printAdjustmentRange(dumpMask, adjustmentRanges(0), adjustmentRangesDefault(pgBlob));
printAdjustmentRange(dumpMask, adjustmentRanges_CopyArray, adjustmentRanges(0));
cliPrintHashLine("rxrange");
printRxRange(dumpMask, rxChannelRangeConfigs(0), rxChannelRangeConfigsDefault(pgBlob));
printRxRange(dumpMask, rxChannelRangeConfigs_CopyArray, rxChannelRangeConfigs(0));
#ifdef USE_TEMPERATURE_SENSOR
cliPrintHashLine("temp_sensor");
printTempSensor(dumpMask, tempSensorConfig(0), tempSensorConfigDefault(pgBlob));
printTempSensor(dumpMask, tempSensorConfig_CopyArray, tempSensorConfig(0));
#endif
#if defined(USE_NAV) && defined(NAV_NON_VOLATILE_WAYPOINT_STORAGE) && defined(NAV_NON_VOLATILE_WAYPOINT_CLI)
@ -3180,24 +3215,28 @@ static void printConfig(const char *cmdline, bool doDiff)
#ifdef USE_OSD
cliPrintHashLine("osd_layout");
printOsdLayout(dumpMask, osdConfig(), osdConfigDefault(pgBlob), -1, -1);
printOsdLayout(dumpMask, &osdConfig_Copy, osdConfig(), -1, -1);
#endif
cliPrintHashLine("master");
dumpAllValues(pgBlob, MASTER_VALUE, dumpMask);
dumpAllValues(MASTER_VALUE, dumpMask);
if (dumpMask & DUMP_ALL) {
// dump all profiles
const int currentProfileIndexSave = getConfigProfile();
const int currentBatteryProfileIndexSave = getConfigBatteryProfile();
for (int ii = 0; ii < MAX_PROFILE_COUNT; ++ii) {
cliDumpProfile(pgBlob, ii, dumpMask);
cliDumpProfile(ii, dumpMask);
}
for (int ii = 0; ii < MAX_BATTERY_PROFILE_COUNT; ++ii) {
cliDumpBatteryProfile(pgBlob, ii, dumpMask);
cliDumpBatteryProfile(ii, dumpMask);
}
setConfigProfile(currentProfileIndexSave);
setConfigBatteryProfile(currentBatteryProfileIndexSave);
cliPrintHashLine("restore original profile selection");
cliPrintLinef("profile %d", getConfigProfile() + 1);
cliPrintLinef("battery_profile %d", getConfigBatteryProfile() + 1);
cliPrintLinef("profile %d", currentProfileIndexSave + 1);
cliPrintLinef("battery_profile %d", currentBatteryProfileIndexSave + 1);
cliPrintHashLine("save configuration\r\nsave");
#ifdef USE_CLI_BATCH
@ -3205,17 +3244,17 @@ static void printConfig(const char *cmdline, bool doDiff)
#endif
} else {
// dump just the current profiles
cliDumpProfile(pgBlob, getConfigProfile(), dumpMask);
cliDumpBatteryProfile(pgBlob, getConfigBatteryProfile(), dumpMask);
cliDumpProfile(getConfigProfile(), dumpMask);
cliDumpBatteryProfile(getConfigBatteryProfile(), dumpMask);
}
}
if (dumpMask & DUMP_PROFILE) {
cliDumpProfile(pgBlob, getConfigProfile(), dumpMask);
cliDumpProfile(getConfigProfile(), dumpMask);
}
if (dumpMask & DUMP_BATTERY_PROFILE) {
cliDumpBatteryProfile(pgBlob, getConfigBatteryProfile(), dumpMask);
cliDumpBatteryProfile(getConfigBatteryProfile(), dumpMask);
}
#ifdef USE_CLI_BATCH
@ -3224,6 +3263,9 @@ static void printConfig(const char *cmdline, bool doDiff)
cliPrintLine("batch end");
}
#endif
// restore configs from copies
restoreConfigs();
}
static void cliDump(char *cmdline)

6
src/main/fc/settings.c
View file

@ -247,10 +247,10 @@ void *settingGetValuePointer(const setting_t *val)
return pg->address + getValueOffset(val);
}
const void * settingGetDefaultValuePointer(void *pgBlob, const setting_t *val)
const void * settingGetCopyValuePointer(const setting_t *val)
{
pgResetCopy(pgBlob, settingGetPgn(val));
return ((uint8_t *)pgBlob) + getValueOffset(val);
const pgRegistry_t *pg = pgFind(settingGetPgn(val));
return pg->copy + getValueOffset(val);
}
setting_min_t settingGetMin(const setting_t *val)

8
src/main/fc/settings.h
View file

@ -92,9 +92,11 @@ pgn_t settingGetPgn(const setting_t *val);
// Returns a pointer to the actual value stored by
// the setting_t. The returned value might be modified.
void * settingGetValuePointer(const setting_t *val);
// Returns a pointer to the default value. Note that
// pgBlob must be at least PG_MAX_SIZE.
const void * settingGetDefaultValuePointer(void *pgBlob, const setting_t *val);
// Returns a pointer to the backed up copy of the value. Note that
// this will contain random garbage unless a copy of the parameter
// group for the value has been manually performed. Currently, this
// is only used by cli.c during config dumps.
const void * settingGetCopyValuePointer(const setting_t *val);
// Returns the minimum valid value for the given setting_t. setting_min_t
// depends on the target and build options, but will always be a signed
// integer (e.g. intxx_t,)