You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
@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:
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.
When receiving a palette index selection via the REST API, interpret the index accordingly.
When interpreting the palette index saved in EEPROM, interpret the index accordingly.
The text was updated successfully, but these errors were encountered:
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:
Do NOT include the gradient palettes in the same array as the normal palettes
Generate a separate array for the gradient palettes in JSON data
Extend REST API to accept either a gradient or a standard palette index
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
Include build version information in all JSON data ... in case behaviors change over time
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.).
@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 toCRGBPalette16
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 CRGBThis could be done in a number of ways. The current strawman:
CRGBPalette16
palettes, and then follow with the..._gp
gradient palettes. In essence, pretend both the...16
and..._gp
palette arrays are concatenated.The text was updated successfully, but these errors were encountered: