Enable spaces when writing config values in Shell

[microbuilder]

The current shell command handlers for config files doesn't allow spaces to be used when saving config values. This is due to the way argv and argc are used.

This should be updated to treat any args after the third as spaces and save strings with spaced so that things like "Adafruit Mynewt" can be assigned to config values.

See: https://github.com/apache/incubator-mynewt-core/blob/master/sys/config/src/config_cli.c

[hathach]

no problem, do we need to do a fork the core repo to somewhere first, this one is probably need to be public to submit as a pull ?? not really sure though.

[microbuilder]

Ah yeah, you will probably need to fork from master to a personal repo on your account, make the changes locally, push them to your repo with a comment, and then from Github generate a pull request back to the core repository. You can just kill the personal repo after that.

We should also add a unit test for this I think since it is only an extra 10 minutes work: https://github.com/apache/incubator-mynewt-core/blob/master/sys/config/src/test/conf_test.c

[hathach]

Thank for the explanation :D

the conf_test.c is the test for the config module behavior, which we are using. Since we are using their API, we dont need to re-test it. We rather need to test if a config is last over a power. But I am not sure if testutil support nrf52 target test run or only simulation for now

[hathach]

Copy comment to this, since I realize the other issue is already closed.

Yeah, it is indeed, we will need to implement an lookup table much like what we did with SDEP/AT command. newt make it a bit worse when tokenize the adafruit/ble/devname into [adafruit, ble, devname] making matching the variable a bit more works. But it should be easy enough. I also intend to move all the handling code into adautil. Application only need to declare variable + their name then the lib will does the trick.

PS: I also feel that the state section/name declare macro can be defined as a single lookup table to ease the work as well. Will try to implement these in the adautil, if it proves to be good enough, maybe we can do a pull request later.

I'm a bit surprised that things like this aren't handled automatically in simple read/modify/write situations:

/**
 * Callback from config management to load data from Flash to local variable
 * @param argc
 * @param argv
 * @param val
 * @return
 */
static int cfg_set (int argc, char **argv, char *val)
{
  if ( (argc == 2) && !strcmp(argv[0], "ble") && !strcmp(argv[1], "devname") )
  {
    return CONF_VALUE_SET(val, CONF_STRING, ada_cfg.ble_devname);
  }

  return OS_ENOENT;
}

By including the data type along with the entry it should have been possible to automatically retrieve the values and assign it to `val`, etc.

Similarly for `cfg_export` and `cfg_get`, every time we add a new config value we need to handle it ourselves in code. That seems error prone and wastes `.text` space, although having a standard config API and system is still of course nice, especially since you can write directly to flash for chips like the nRF52 that have no internal EEPROM.

Perhaps the config code should be moved to a separate .c/.h file though to keep this in one place and make it easier to reference elsewhere?