Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch LED types from Bus classes (dynamic UI) #4129

Merged
merged 29 commits into from
Sep 13, 2024
Merged

Fetch LED types from Bus classes (dynamic UI) #4129

merged 29 commits into from
Sep 13, 2024

Conversation

blazoncek
Copy link
Collaborator

The idea was sparked by @netmindz (#4056) to allow dynamic creation of enabled (compiled) LED types in LED settings UI instead of relying on editing HTML file to represent them.

This PR implements @netmindz principles while also improves JavaScript handling of implemented bus types in UI (no longer relies on magic numbers).

As a bonus BusPwm is enhanced to support "dithering" and "phase shifting" on ESP32. LEDC supports 4 fractional bits that enhance resolution by 1 bit using temporal statistical distribution (see ESP32 technical reference manual, LEDC chapter).
Phase shifting can be used to distribute load on PSU and, more importantly, drive H-bridge used for reverse-polarity PWM CCT LED strips (thanks goes to @DedeHai for debugging signals on the oscilloscope and adjusting calculations).

The downside of this PR is increased stack buffer size for oappend() function as LED types are pushed into it. When the number of LED types reaches the limit of usable stack that information will have to be split into a separate HTTP/XHR request.

What remains to be implemented is:

  • passing RAM and power requirements for each LED type (partially implemented by is16bit() function)
  • detection of parallel I2S vs. RMT buffers on ESP32 (RAM and available outputs)
  • better handling of available HW (avoid const.h and use HAL)
  • possible additional true "virtual" or usermod buses (see Bus config #4123)
  • pass information if LED type requires GPIO and how many (currently length of "t" JSON key determines it)
  • there is only one "virtual" type (currently used for network DDP/ArtNet)
  • if color orders are supported, off refresh is required, etc (could be stored in capabilities "c" JSON key)

Closes #4112 #4117

blazoncek and others added 20 commits August 22, 2024 17:15
- provide LED types from BusManager for settings
Credit: @netmindz for the idea.
- move macros to constexpr methods
- introduce type capabilities for UI
- add phase shifting (POC) to PWM
- replace PWM CIE LUT with calculated curve
CIE & phase shifting credit @DedeHai
- LEDC: allocate same timer for CCT PWM
- use SOC constants
BusManager update
- use allocateMultiplePins for BusPwm
- phase shifting correction (limit to PWM CCT)
Clarifications
Add semaphoring
Determine CCT overlap
- fixed maxBri value
- fixed overflow in dead time subtraction
- deadtime and offset now also work if signals are inverted (_reversed)
- renamed variables
- some tuning
fixed offsets and inverted signal plus dead time
@blazoncek blazoncek added enhancement javascript Pull requests that update Javascript code labels Sep 5, 2024
@willmmiles
Copy link
Collaborator

Re 43cc4ec, a quick thought offhand regarding oappend: I think there's really little value in using a stack buffer at all, since the two places where it's used immediately turn around and copy the contents to a heap allocated buffer anyways.

With that in mind and since this limitation keeps coming up, I sketched a proof-of-concept "remove oappend" patch last week -- there's some more cleanup to be done, the API change would need to be applied to all usermods, and it probably wants to be extended to use a rope-like data structure like DynamicBufferList, but it should give the general idea for a possible future direction.

Copy link
Collaborator

@willmmiles willmmiles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High level thoughts:

  • This is an excellent step in the right direction
  • Big thumbs up for moving from macros to constexpr
  • It really makes me want to take a stab at modularizing Bus type handling/config management/construction

wled00/bus_manager.h Outdated Show resolved Hide resolved
wled00/bus_manager.h Show resolved Hide resolved
wled00/bus_manager.h Outdated Show resolved Hide resolved
wled00/bus_manager.cpp Show resolved Hide resolved
@blazoncek
Copy link
Collaborator Author

a quick thought offhand regarding oappend

oappend is there since ever. If you intend to work on it it may be easiest just to replace char[] buffer with String so it can grow as needed.

@willmmiles
Copy link
Collaborator

a quick thought offhand regarding oappend

oappend is there since ever. If you intend to work on it it may be easiest just to replace char[] buffer with String so it can grow as needed.

Every time I see String::operator+, I weep for the allocator. ;) I went with a Print interface instead, since it's got a rich API, can be backed by different allocator strategies depending on the context, and AsyncWebServer already had a Stream response available.

wled00/data/settings_leds.htm Show resolved Hide resolved
wled00/data/settings_leds.htm Outdated Show resolved Hide resolved
wled00/const.h Show resolved Hide resolved
@blazoncek
Copy link
Collaborator Author

@netmindz & @willmmiles would you think this is good to be merged? @softhack007 do you have any second thoughts?
This (principle), along with #4134, may make expanding settings (including and where most beneficial - audioreactive settings) much easier.
@Aircoookie please review (or at least look at) this PR and #4134 if they clash somehow with what you planned for settings rewrite.

return &(_mappings[n]);
}

uint8_t getPixelColorOrder(uint16_t pix, uint8_t defaultColorOrder) const;

private:
uint8_t _count;
ColorOrderMapEntry _mappings[WLED_MAX_COLOR_ORDER_MAPPINGS];
std::vector<ColorOrderMapEntry> _mappings;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the switch to std::vector intended to reduce memory usage?

One of the challenges is that std::vector has an exponential allocation strategy. Depending on the relationship between WLED_MAX_COLOR_ORDER_MAPPINGS and the number of entries actually used, it may end up using more memory. I checked the code for std::vector for ESP32 -- this implementation of push_back uses the classic "double the array length if it needs more memory" strategy. So for WLED_MAX_COLOR_ORDER_MAPPINGS == 10 (esp32), when you push the 9th element on to the list, the buffer will expand to 16 -- leaving 6 elements you pay for in RAM, but will never use.

What you can do to mitigate this is any of:

  • You could ensure that the limits are powers of 2
  • When loading the vector, figure out how many elements you need, then call reserve() up front to allocate exactly the right amount of memory
  • Alternately, you can preallocate the maximum memory you plan to use with reserve() at construct time; won't save any RAM but also guarantees you'll never allocate more than you need.
  • You could call shrink_to_fit() after filling the vector, or make a copy and discard the original std::vector object. (Cost is the same, a complete copy and temporarily holding more RAM than it needs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the switch to std::vector intended to reduce memory usage?

No. It is to allow variable size (in the future) without the expense of pre-allocated memory. The question was "why 10 and why only 10?".
reserve() could be done, yes. That'll go into cfg.cpp and set.cpp. Thanks.

@blazoncek blazoncek self-assigned this Sep 12, 2024
@blazoncek blazoncek merged commit df24fd7 into 0_15 Sep 13, 2024
40 checks passed
@blazoncek
Copy link
Collaborator Author

@jw2013 please rebase your branch(es). I'm going to delete bus-config soon.

@blazoncek blazoncek deleted the bus-config branch September 15, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants