jasoncoon / esp8266-fastled-webserver

GNU General Public License v3.0
712 stars 360 forks source link

Re-work how UI-exposed properties work #227

Open henrygab opened 2 years ago

henrygab commented 2 years ago

here's a list of things to consider improving:

Fill in as things come to mind.

jasoncoon commented 2 years ago

Yeah, I would love to solve this problem. We wouldn't necessarily need to solve it in a generic way, the web app could just be made more aware of the different patterns and their related settings. But it is really nice to be able to add new fields and not have to update the web app.

tobi01001 commented 2 years ago

That is also something I would like to see.

What I did already (should I create a PR for this?) is to make the sectionfields in the webapp "buttons" where the content below is hidden when opening the web page. When you click, the properties expand (or fold). This does not solve the particular problem but contributes to a cleaner look and feel.

For the properties / pattern combination, one could flag the particular field and pattern with a e.g. 32bit value where each bit would belong to a certain property or set of properties....

henrygab commented 2 years ago
The primary problem is determining which effects are affected by which properties / variables.

I don't think there is currently any documentation indicating which properties are associated with which effects. I also haven't found a good code-mapping tool (e.g., what global variables are accessed by each function) that works in VSCode, at least for C++ code, and Visual Studio appears to require Enterprise edition for that functionality. Thus, this is currently not something that would be made easier for me via tools that I am aware of.


So, I think the first thing is for a volunteer to de-tangle and list (spreadsheet?) for each effect, what settable values affect how the effect operates.
Looking at the desired solution holistically

* The Web UI already shows the current effect, and (with websocket support) would get notified of changes. * The Web UI should be reactive, reflecting to the user which properties will affect any given effect. * The Web UI may want to show which properties **would** impact any given effect, even if it is not currently playing. * The existing JSON data from `fields.cpp` is used to generate the Web UI, and already includes a unique identifier for each property (`field->name`). * The Web UI uses `field->name` as the `id=` attribute for a property.


That holistic view suggests the desired API properties.

* The new API allows the Web UI to request "property information" on which properties are used by a given effect. * For input, the new API can accept the same identifier that is used to change the current effect. * For output, the new API can provide a JSON array that lists the settings that impact that effect, preferably using `field->name` as the identifier, since that's already used to uniquely identify properties.


Internal operation should be opaque.

How the data is stored and tracked internally should not impact the above properties. In fact, it an implementation detail, and one where a good answer really depends on the data ... how complex is this? There are currently 41 rows in the `fields` array: * Three common properties that affect everything (power, brightness, pattern) * Three clock properties that always apply, if `showClock` is enabled (showClock, clockBackgroundFade, utcOffsetIndex) * Two common properties that **_might_** affect any particular effect (palette, speed) * `solidColor` is listed in its own "section" currently(?!) * Two properties listed as "Fire & Water" (cooling, sparking) * Three properties listed as "Twinkles" (twinkleSpeed, twinkleDensity, coolLikeIncandescent) * 18(!) properties that likely only apply to "Pride Playground"


Strawman psuedo-code

Given the limited information above, the internal implementation might: * Define a bitfield enum for the groups of properties (+ a few individual ones) * Add that bitfield enum to PatternAndName, thus requiring each pattern to explicitly define which properties affect its behavior * Add that bitfield enum to `Field`, thus requiring each field to explicitly define which group(s) it belongs to * Generate JSON for any given pattern ```C++ namespace PropertyGroups { // would prefer to use scoped enum, but there's sharp bits when trying to use // scoped enums for bitflags. Thus, using a namespace to still avoid namespace // pollution, while having flexibility with setting/getting multiple values. enum EnumFoo : uint16_t { // internal only, never exposed directly AlwaysEnabledGroup = 0x0100, // power, brightness, showClock, clockBackgroundFade, utcOffsetIndex FireAndWaterGroup = 0x0200, // cooling, sparking TwinklesGroup = 0x0400, // twinkleSpeed, twinkleDensity, coolLikeIncandescent PridePlaygroundGroup = 0x0800, // ... 18 properties ... // and then the list of single properties Palette = 0x0001, Speed = 0x0002, SolidColor = 0x0004, }; void CreatePropertiesJsonForPatternIndex(size_t patternIndex, JsonVariant dst) { auto value = patterns[patternIndex].PropertyGroups; auto doc = dst.to(); for (auto f : fields) { if (0 != (f->PropertyGroups & value)) { // if any bits overlap, then the property is 'active' doc.add(f->name); } } }; }; ```


Lots to write, but not that complex when done. Just needs the work of back-tracking all 40-odd settable properties to see which patterns use each of them....
jasoncoon commented 2 years ago

I think you've pretty closely identified which fields are used by which patters, other than speed and palette maybe. I can definitely build a complete list.

Ideally all patterns would use speed in some way.

I'd like to simplify all of the patterns that really only differ by palette (twinkles, noise, etc). Ideally all patterns would use the selected palette, and we'd add a set of options similar to the pattern autoplay and duration for palettes. That and combine the two different palette lists ("standard" and gradient).

The only other difference between the noise patterns are x, y, and z speeds/directions. I'd love to add fields for those as well.

Whatever solution we arrive at, I'd like to keep it fairly simple, and ideally keep the fields used close to the definition of the pattern function.