jasoncoon/esp8266-fastled-webserver

Enable use of gradient palettes as a normal palette

Opened this issue · 1 comments

@HACKER-3000's original PR #189 included each of the gradient palettes (by convention, variable names ending in _gp) in the palette list. However, this appeared to convert each of the gradient palettes to CRGBPalette16 at startup, increasing both ROM and RAM usage.

To ensure this issue is tracked when that PR is merged (sans the gradient palettes), this issue was opened as it makes sense to support the gradient palettes as palette selections. Of course, the goal is to do so without keeping a second copy of each gradient palette in memory.

Given that the gradient palettes can be auto-converted to CRGBPalette16 on-the-fly, it seems more useful to simply expose both types of palettes to the web UI, and similarly accept a gradient palette as the current palette. There is not actually any need to convert the gradient palette to a CRGB

This could be done in a number of ways. The current strawman:

  1. When generating the JSON listing the available palettes, put first the existing CRGBPalette16 palettes, and then follow with the ..._gp gradient palettes. In essence, pretend both the ...16 and ..._gp palette arrays are concatenated.
  2. When receiving a palette index selection via the REST API, interpret the index accordingly.
  3. When interpreting the palette index saved in EEPROM, interpret the index accordingly.

Looking at this issue again. To increase API stability, it would be beneficial to have a palette's index be fixed, rather than having them shift. With the existing palettes, this is easy by simply appending to the end of the list. If these lists get concatenated, then this will no longer be true, as adding any normal palette will shift the indices of all the gradient palettes.

Obviously, there can be a requirement to request the list of palettes, and scan for a string match. But this makes scripting and automation more complex ... automation solutions would likely prefer to avoid this.

Thus, in thinking this through more, I would suggest a slightly altered solution:

  1. Do NOT include the gradient palettes in the same array as the normal palettes
  2. Generate a separate array for the gradient palettes in JSON data
  3. Extend REST API to accept either a gradient or a standard palette index
  4. Extend EEPROM to store either a gradient or a standard palette index ... initially by using most significant bit ... thus limiting palettes and gradient palettes to maximum ~ 125 entries each
  5. Include build version information in all JSON data ... in case behaviors change over time
  6. Extend Javascript that currently parses palettes to support both types of palettes

Note: Initially, I'd recommend that it should technically be an error to provide both a gradient and normal palette index via REST API. Specifically, the behavior in this situation would be subject to change between versions (e.g., select the first one parsed; select the last one parsed; always prefer a gradient palette index; both stored and used separately; etc.).