From c43d09c8b18f02a5a9142f000b6449948c9335bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Kristan?= Date: Fri, 7 Feb 2025 15:02:27 +0100 Subject: [PATCH] Move _data and allocation to derived class - as suggested by @TripleWhy - minimum length guard Conflicts: wled00/bus_manager.cpp wled00/bus_manager.h --- wled00/bus_manager.cpp | 46 ++++++++++++++++++------------------------ wled00/bus_manager.h | 34 +++++++++++++++---------------- 2 files changed, 36 insertions(+), 44 deletions(-) diff --git a/wled00/bus_manager.cpp b/wled00/bus_manager.cpp index 69cc63bed..68a58d5a4 100644 --- a/wled00/bus_manager.cpp +++ b/wled00/bus_manager.cpp @@ -91,7 +91,7 @@ void Bus::calculateCCT(uint32_t c, uint8_t &ww, uint8_t &cw) { } else { cct = (approximateKelvinFromRGB(c) - 1900) >> 5; // convert K (from RGB value) to relative format } - + //0 - linear (CCT 127 = 50% warm, 50% cold), 127 - additive CCT blending (CCT 127 = 100% warm, 100% cold) if (cct < _cctBlend) ww = 255; else ww = ((255-cct) * 255) / (255 - _cctBlend); @@ -118,16 +118,6 @@ uint32_t Bus::autoWhiteCalc(uint32_t c) const { return RGBW32(r, g, b, w); } -uint8_t *Bus::allocateData(size_t size) { - freeData(); // should not happen, but for safety - return _data = (uint8_t *)(size>0 ? calloc(size, sizeof(uint8_t)) : nullptr); -} - -void Bus::freeData() { - if (_data) free(_data); - _data = nullptr; -} - BusDigital::BusDigital(const BusConfig &bc, uint8_t nr) : Bus(bc.type, bc.start, bc.autoWhite, bc.count, bc.reversed, (bc.refreshReq || bc.type == TYPE_TM1814)) @@ -153,8 +143,10 @@ BusDigital::BusDigital(const BusConfig &bc, uint8_t nr) _hasRgb = hasRGB(bc.type); _hasWhite = hasWhite(bc.type); _hasCCT = hasCCT(bc.type); - if (bc.doubleBuffer && !allocateData(bc.count * Bus::getNumberOfChannels(bc.type))) return; - //_buffering = bc.doubleBuffer; + if (bc.doubleBuffer) { + _data = (uint8_t*)d_calloc(_len, Bus::getNumberOfChannels(_type)); + if (!_data) DEBUG_PRINTLN(F("Bus: Buffer allocation failed!")); + } uint16_t lenToCreate = bc.count; if (bc.type == TYPE_WS2812_1CH_X3) lenToCreate = NUM_ICS_WS2812_1CH_3X(bc.count); // only needs a third of "RGB" LEDs for NeoPixelBus _busPtr = PolyBus::create(_iType, _pins, lenToCreate + _skip, nr); @@ -285,7 +277,7 @@ void BusDigital::show() { } } } - PolyBus::show(_busPtr, _iType, !_data); // faster if buffer consistency is not important (use !_buffering this causes 20% FPS drop) + PolyBus::show(_busPtr, _iType, !_data); // faster if buffer consistency is not important // restore bus brightness to its original value // this is done right after show, so this is only OK if LED updates are completed before show() returns // or async show has a separate buffer (ESP32 RMT and I2S are ok) @@ -434,10 +426,11 @@ void BusDigital::begin() { void BusDigital::cleanup() { DEBUG_PRINTLN(F("Digital Cleanup.")); PolyBus::cleanup(_busPtr, _iType); + free(_data); + _data = nullptr; _iType = I_NONE; _valid = false; _busPtr = nullptr; - if (_data != nullptr) freeData(); PinManager::deallocatePin(_pins[1], PinOwner::BusDigital); PinManager::deallocatePin(_pins[0], PinOwner::BusDigital); } @@ -461,7 +454,7 @@ void BusDigital::cleanup() { #else #ifdef SOC_LEDC_TIMER_BIT_WIDE_NUM // C6/H2/P4: 20 bit, S2/S3/C2/C3: 14 bit - #define MAX_BIT_WIDTH SOC_LEDC_TIMER_BIT_WIDE_NUM + #define MAX_BIT_WIDTH SOC_LEDC_TIMER_BIT_WIDE_NUM #else // ESP32: 20 bit (but in reality we would never go beyond 16 bit as the frequency would be to low) #define MAX_BIT_WIDTH 14 @@ -509,7 +502,6 @@ BusPwm::BusPwm(const BusConfig &bc) _hasRgb = hasRGB(bc.type); _hasWhite = hasWhite(bc.type); _hasCCT = hasCCT(bc.type); - _data = _pwmdata; // avoid malloc() and use stack _valid = true; DEBUG_PRINTF_P(PSTR("%successfully inited PWM strip with type %u, frequency %u, bit depth %u and pins %u,%u,%u,%u,%u\n"), _valid?"S":"Uns", bc.type, _frequency, _depth, _pins[0], _pins[1], _pins[2], _pins[3], _pins[4]); } @@ -583,7 +575,7 @@ void BusPwm::show() { // if _needsRefresh is true (UI hack) we are using dithering (credit @dedehai & @zalatnaicsongor) // https://github.com/Aircoookie/WLED/pull/4115 and https://github.com/zalatnaicsongor/WLED/pull/1) const bool dithering = _needsRefresh; // avoid working with bitfield - const unsigned maxBri = (1<<_depth); // possible values: 16384 (14), 8192 (13), 4096 (12), 2048 (11), 1024 (10), 512 (9) and 256 (8) + const unsigned maxBri = (1<<_depth); // possible values: 16384 (14), 8192 (13), 4096 (12), 2048 (11), 1024 (10), 512 (9) and 256 (8) const unsigned bitShift = dithering * 4; // if dithering, _depth is 12 bit but LEDC channel is set to 8 bit (using 4 fractional bits) #endif // use CIE brightness formula (linear + cubic) to approximate human eye perceived brightness @@ -599,7 +591,7 @@ void BusPwm::show() { [[maybe_unused]] unsigned hPoint = 0; // phase shift (0 - maxBri) // we will be phase shifting every channel by previous pulse length (plus dead time if required) - // phase shifting is only mandatory when using H-bridge to drive reverse-polarity PWM CCT (2 wire) LED type + // phase shifting is only mandatory when using H-bridge to drive reverse-polarity PWM CCT (2 wire) LED type // CCT additive blending must be 0 (WW & CW will not overlap) otherwise signals *will* overlap // for all other cases it will just try to "spread" the load on PSU // Phase shifting requires that LEDC timers are synchronised (see setup()). For PWM CCT (and H-bridge) it is @@ -678,7 +670,7 @@ void BusPwm::deallocatePins() { BusOnOff::BusOnOff(const BusConfig &bc) : Bus(bc.type, bc.start, bc.autoWhite, 1, bc.reversed) -, _onoffdata(0) +, _data(0) { if (!Bus::isOnOff(bc.type)) return; @@ -691,7 +683,6 @@ BusOnOff::BusOnOff(const BusConfig &bc) _hasRgb = false; _hasWhite = false; _hasCCT = false; - _data = &_onoffdata; // avoid malloc() and use stack _valid = true; DEBUG_PRINTF_P(PSTR("%successfully inited On/Off strip with pin %u\n"), _valid?"S":"Uns", _pin); } @@ -703,17 +694,17 @@ void BusOnOff::setPixelColor(unsigned pix, uint32_t c) { uint8_t g = G(c); uint8_t b = B(c); uint8_t w = W(c); - _data[0] = bool(r|g|b|w) && bool(_bri) ? 0xFF : 0; + _data = bool(r|g|b|w) && bool(_bri) ? 0xFF : 0; } uint32_t BusOnOff::getPixelColor(unsigned pix) const { if (!_valid) return 0; - return RGBW32(_data[0], _data[0], _data[0], _data[0]); + return RGBW32(_data, _data, _data, _data); } void BusOnOff::show() { if (!_valid) return; - digitalWrite(_pin, _reversed ? !(bool)_data[0] : (bool)_data[0]); + digitalWrite(_pin, _reversed ? !(bool)_data : (bool)_data); } size_t BusOnOff::getPins(uint8_t* pinArray) const { @@ -752,7 +743,8 @@ BusNetwork::BusNetwork(const BusConfig &bc) _hasCCT = false; _UDPchannels = _hasWhite + 3; _client = IPAddress(bc.pins[0],bc.pins[1],bc.pins[2],bc.pins[3]); - _valid = (allocateData(_len * _UDPchannels) != nullptr) && bc.count > 0; + _data = (uint8_t*)d_calloc(_len, _UDPchannels); + _valid = (_data != nullptr); DEBUG_PRINTF_P(PSTR("%successfully inited virtual strip with type %u and IP %u.%u.%u.%u\n"), _valid?"S":"Uns", bc.type, bc.pins[0], bc.pins[1], bc.pins[2], bc.pins[3]); } @@ -801,9 +793,11 @@ std::vector BusNetwork::getLEDTypes() { } void BusNetwork::cleanup() { + DEBUG_PRINTLN(F("Virtual Cleanup.")); + free(_data); + _data = nullptr; _type = I_NONE; _valid = false; - freeData(); } diff --git a/wled00/bus_manager.h b/wled00/bus_manager.h index 65724003b..60b96048d 100644 --- a/wled00/bus_manager.h +++ b/wled00/bus_manager.h @@ -83,20 +83,19 @@ class Bus { : _type(type) , _bri(255) , _start(start) - , _len(len) + , _len(std::max(len,(uint16_t)1)) , _reversed(reversed) , _valid(false) , _needsRefresh(refresh) - , _data(nullptr) // keep data access consistent across all types of buses { _autoWhiteMode = Bus::hasWhite(type) ? aw : RGBW_MODE_MANUAL_ONLY; }; - virtual ~Bus() {} //throw the bus under the bus (derived class needs to freeData()) + virtual ~Bus() {} //throw the bus under the bus virtual void begin() {}; virtual void show() = 0; - virtual bool canShow() const { return true; } + virtual bool canShow() const { return true; } virtual void setStatusPixel(uint32_t c) {} virtual void setPixelColor(unsigned pix, uint32_t c) = 0; virtual void setBrightness(uint8_t b) { _bri = b; }; @@ -191,7 +190,6 @@ class Bus { bool _hasCCT;// : 1; //} __attribute__ ((packed)); uint8_t _autoWhiteMode; - uint8_t *_data; // global Auto White Calculation override static uint8_t _gAWM; // _cct has the following menaings (see calculateCCT() & BusManager::setSegmentCCT()): @@ -206,8 +204,6 @@ class Bus { static uint8_t _cctBlend; uint32_t autoWhiteCalc(uint32_t c) const; - uint8_t *allocateData(size_t size = 1); - void freeData(); }; @@ -237,14 +233,15 @@ class BusDigital : public Bus { static std::vector getLEDTypes(); private: - uint8_t _skip; - uint8_t _colorOrder; - uint8_t _pins[2]; - uint8_t _iType; + uint8_t _skip; + uint8_t _colorOrder; + uint8_t _pins[2]; + uint8_t _iType; uint16_t _frequencykHz; - uint8_t _milliAmpsPerLed; + uint8_t _milliAmpsPerLed; uint16_t _milliAmpsMax; - void * _busPtr; + uint8_t *_data; + void *_busPtr; static uint16_t _milliAmpsTotal; // is overwitten/recalculated on each show() @@ -274,13 +271,13 @@ class BusPwm : public Bus { uint16_t getFrequency() const override { return _frequency; } size_t getBusSize() const override { return sizeof(BusPwm); } void show() override; - inline void cleanup() { deallocatePins(); _data = nullptr; } + inline void cleanup() { deallocatePins(); } static std::vector getLEDTypes(); private: uint8_t _pins[OUTPUT_MAX_PINS]; - uint8_t _pwmdata[OUTPUT_MAX_PINS]; + uint8_t _data[OUTPUT_MAX_PINS]; #ifdef ARDUINO_ARCH_ESP32 uint8_t _ledcStart; #endif @@ -301,13 +298,13 @@ class BusOnOff : public Bus { size_t getPins(uint8_t* pinArray) const override; size_t getBusSize() const override { return sizeof(BusOnOff); } void show() override; - inline void cleanup() { PinManager::deallocatePin(_pin, PinOwner::BusOnOff); _data = nullptr; } + inline void cleanup() { PinManager::deallocatePin(_pin, PinOwner::BusOnOff); } static std::vector getLEDTypes(); private: uint8_t _pin; - uint8_t _onoffdata; + uint8_t _data; }; @@ -331,6 +328,7 @@ class BusNetwork : public Bus { uint8_t _UDPtype; uint8_t _UDPchannels; bool _broadcastLock; + uint8_t *_data; }; @@ -351,7 +349,7 @@ struct BusConfig { uint16_t milliAmpsMax; BusConfig(uint8_t busType, uint8_t* ppins, uint16_t pstart, uint16_t len = 1, uint8_t pcolorOrder = COL_ORDER_GRB, bool rev = false, uint8_t skip = 0, byte aw=RGBW_MODE_MANUAL_ONLY, uint16_t clock_kHz=0U, bool dblBfr=false, uint8_t maPerLed=LED_MILLIAMPS_DEFAULT, uint16_t maMax=ABL_MILLIAMPS_DEFAULT) - : count(len) + : count(std::max(len,(uint16_t)1)) , start(pstart) , colorOrder(pcolorOrder) , reversed(rev)