use arduinoJson for fields

[henrygab]

This should reduce heap fragmentation (by constant appending of small strings).

Uses arduinoJson to simplify (and make safer) the generation of JSON. Uses custom converter for field structure to make easy to use.

Also explicitly makes most of the code in fields.cpp internal, to ensure safe to modify the internals. Only public APIs are put outside the namespace.

There's also some minor changes (removing auto references in map.cpp) in anticipation of enabling ESP32.

[henrygab]

@tobi01001 -- Awesome review, I really appreciate your ability to quickly pinpoint those additional areas for improvement! ~I hope you'll open a couple issues (so that you remain the owner of the issue) to track the noted enhancements, as~ they're great ideas. Thank you!

[henrygab]

Hi @jasoncoon -- Are you good with these changes?

[jasoncoon]

I'm working on adding Fibonacci1024, see thread on Twitter here: https://twitter.com/jasoncoon_/status/1461852385656160261

The /all response is being truncated. I assume I'll need to adjust the size allocated to the jsonDoc? Would it make sense to split this into multiple API calls? Perhaps one call to enumerate the fields, including only the basic info, then one additional request per select field to fully describe the options? Since the bulk of the data is the list of pattern and palette names.

[henrygab]

Hi Jason,

I'm interested! Please message me on Tindie with details on expected costs, etc?

You can use ArduinoJSON Assistant to help figure out the size of the buffer needed. I entered the longest possible strings in each field, then added ~100 bytes. Looks like I didn't add enough.

Would it make sense to split the API? Yes, at least for pattern / palette lists.

Maybe define a new data type, which is a string representing a relative URI, where the caller calls that URI as a REST API to get the additional data?

I'm sure there are "correct" ways to do this in REST, but I'm not deeply familiar with how REST APIs normally self-document....

[tobi01001]

I do believe that this is good but maybe not neccessary ;-)

The /all API is only used to fill the web app with the configuration / parameters. The only dynamic part is in the current values.

So rather than increasing the JSON Buffer I propose to:

• decouple fixed values (i.e. the configuration) from the dynamic / changing values
• /all only serves the configuration
• /allvalues serves current values of parameters

/all is fixed as long as there is no change in the software affecting the fields and its contents. So why not letting the ESP app writing the config to a file on the LittleFS (e.g. config_all.json). On SW update/magic number change (I would tend to use a CRC which is set to 0 on SW updates), the application could in setup() rewrite the file to the current config.

And all the app would have to do with the API call to /all is to serve that particular file instead of the StaticJSONBuffer... The filesystem and webserver is there anyway...

[henrygab]

Caveats which Jason should confirm, but I will presume as true for this post:

It's OK to break compatibilty withe old REST APIs.

If yes, this means it's OK to break compatibility if a 3rd party had used the REST APIs, such as using custom `curl` scripts with `crond`, etc. In essence, the only supported client is the on-device Web UI, so that updates needn't consider backwards-compatibility. I think Jason will be OK with this?

It's OK to take a permanent dependent on having LittleFS storage.

If yes, this means boards that don't have LittleFS (or SPIFFS) support will not work. I don't foresee this being a concern, but it's better to be sure.

---

### @tobi01001's solution is excellent In fact, if Jason confirms those are OK, then it'll be time to drop the "EEPROM" code altogether, in favor of reading/writing the parameters through LittleFS, using ArduinoJson.

[jasoncoon]

It's definitely OK with me to break compatibility with old REST APIs. Might need to check in with @balloob to see if there's a good way to prevent breaking the HomeAssistant integration: https://github.com/home-assistant/core/tree/dev/homeassistant/components/evil_genius_labs

Permanent LittleFS dependency is also fine with me.

It'd be great if this could be done in a way that wouldn't require having to update multiple places in the code and/or file when new patterns/palettes are added, updated, renamed, etc.

I also think the patterns and palettes list should probably be separate requests, as they are each so large.

[balloob]

Can we add a version number to the API so I can differentiate the different APIs?

[tobi01001]

@henrygab I actually did not mean the EEPROM settings... Personally I would avoid writing them to a json file in the file system. As they are being written quite frequently it may / will cause quite a lot of additional wear to the flash - unless you write and read only binary data. but That is then the same just with the overhead for the file system ;-)

For easier maintenance and for the sake of reliability I would suggest that (modifiable) settings to be stored are managed in a structure like e.g.:

structure

```cpp typedef struct { uint8_t settingsMagicByte = SETTINGS_MAGIC_BYTE; CRGB solidColor = CRGB::Blue; uint8_t speed = 30; uint8_t power = 1; uint8_t brightness = brightnessMap[brightnessIndex]; uint8_t cooling = 49; uint8_t sparking = 60; uint8_t currentPatternIndex = DEFAULT_PATTERN_INDEX; // Index number of which pattern is current uint8_t autoplay = 0; uint8_t autoplayDuration = 10; uint8_t showClock = 0; uint8_t clockBackgroundFade = 160; uint8_t utcOffsetIndex = 24; uint8_t currentPaletteIndex = 0; uint8_t twinkleSpeed = 4; uint8_t twinkleDensity = 5; uint8_t coolLikeIncandescent = 1; } strEEPROMSettings; ```

This way, the settings management incl. EEPROM write and read is just:

Handling

```cpp strEEPROMSettings EEPROMSettings; ... EEPROM.begin(sizeof(EEPROMSettings)) ... void readSettings() { // check for "magic number" so we know settings have been written to EEPROM // and it's not just full of random bytes if (EEPROM.read(0) != SETTINGS_MAGIC_BYTE) { return; } EEPROM.get(0, EEPROMSettings); setUtcOffsetIndex(EEPROMSettings.utcOffsetIndex); } void writeAndCommitSettings() { EEPROMSettings.settingsMagicByte = SETTINGS_MAGIC_BYTE; EEPROM.put(0, EEPROMSettings); EEPROM.commit(); } ```

EDIT: If the magic byte would be replaced by a CRC 16, one could easily check integrity of the structure / saved settings. So if a new setting is added or removed, there would be a CRC mismatch and one could stay on default values. This will not directly work for changes to the index of default patterns or default palettes but one could just delete the CRC in such case... /EDIT

The EEPROM uses a reserved section in the flash where a complete sector needs to be erased and rewritten. Reliability could further be increased by using e.g. EEPROM_Rotate which is basically a drop-in replacement of the EEPROM class and which will cycle through different sectors...

Long story short: As the EEPROM is only emulated in the flash, I see no benefit (other than maybe readability and code maintenance) to switch to LittleFS with file sytem overhead for the settings to be saved and reread quite often...

@jasoncoon If Fields (w/o values) are all written / stored in a file, I think there is no need to separate the "options" for patterns and palettes - unless one would only need to read those. But this could go into a separate API call. The webserver does serve all the other files (which are bigger in size) wihtout issues and so it can also serve this one easily (and quick).

[jasoncoon]

@tobi01001 just so I'm clear, we're talking about the fields (w/o values) being written/stored in a file automatically/dynamically, in setup, on startup? Not maintained separately in a static file that would need to be kept in sync with the patterns, palettes, etc in code?

[tobi01001]

Yes, exactly. A file which is generated from the fields itself in case the configuration changed (or the file is not yet available).

I did this already but as I am personally on ArduinoJson V5 and AsyncWebserver (and very limited time currently) I cannot come up with a PR on the fly... ;-)

In my case with AsyncWebServer this would be as easy as:

Update File

```cpp void getAllJSON(JsonArray &arr) { for (uint8_t i = 0; i < getFieldCount(); i++) { Field field = fields[i]; JsonObject& obj = arr.createNestedObject(); obj[F("name")] = field.name; obj[F("label")] = field.label; obj[F("type")] = (int)field.type; if (field.type == NumberFieldType) { obj[F("min")] = field.min; obj[F("max")] = field.max; } if (field.getOptions) { JsonArray &ar = obj.createNestedArray(F("options")); field.getOptions(ar); } } } void updateConfigFile(void) { DynamicJsonBuffer buffer; JsonArray& root = buffer.createArray(); getAllJSON(root); File config_Json = LittleFS.open(F("/config/config_all.json"), "w"); root.printTo(config_Json); config_Json.close(); } ```

Serve the file

```cpp ... server.on("/all", HTTP_GET, [](AsyncWebServerRequest *request) { if(!LittleFS.exists("/config/config_all.json")) { updateConfigFile(); } request->send(LittleFS, "/config/config_all.json", "application/json"); }); server.on("/allvalues", HTTP_GET, [](AsyncWebServerRequest *request) { AsyncJsonResponse * response = new AsyncJsonResponse(); JsonObject &root = response->getRoot(); JsonArray &arr = root.createNestedArray("values"); if(!getAllValuesJSONArray(arr)) { JsonObject &obj = arr.createNestedObject(); obj[F("ValueError")] = F("Did not read any values!"); } response->setLength(); request->send(response); }); ```

And the app.js needs to perform a second request to e.g /allvalues to update the web app to the current values with something like:

app.js

```js $.get(urlBase + "all", function(data) { }) .fail(function(errorThrown) { console.log(errorThrown); }) .done(function(name, value, test) { $("#status").html("Done..."); $.get(urlBase + "allvalues", function(data) { $("#status").html("Loading, current values..."); for(i=0; i

[henrygab]

Having two methods to read/parse/store configuration (EEPROM and JSON) is going to be error-prone, and increases maintenance. I still think it makes sense to store any user-configurable data in /config.json, and to stop using the EEPROM. Reading / updating the configuration is not a performance-sensitive operation, so I think the benefits in maintainability will outweigh the downsides.

Any objection if I work towards Tobi's solution, ditch all the EEPROM code, and store only in the file system?

[henrygab]

OK, I will open a new issue to track that work. Thanks!

[henrygab]

Can we add a version number to the API so I can differentiate the different APIs?

Yes, that's a fine idea.