-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Usermod bus type #4123
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
base: main
Are you sure you want to change the base?
Usermod bus type #4123
Changes from all commits
8708de4
b91a695
c48aaf6
5c76ec1
64b62ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -236,6 +236,7 @@ AR_lib_deps = kosme/arduinoFFT @ 2.0.1 | |
| ;; | ||
| ;; please note that you can NOT update existing ESP32 installs with a "V4" build. Also updating by OTA will not work properly. | ||
| ;; You need to completely erase your device (esptool erase_flash) first, then install the "V4" build from VSCode+platformio. | ||
| default_partitions = tools/WLED_ESP32_4MB_1MB_FS.csv | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please keep this usermod PR out of the main Even a one-line change here can perturb shared CI/build environments, and this PR does not look like the dual-framework exception that needs core build-system edits. This should stay in a custom override/build config unless a maintainer explicitly wants it in-tree. As per coding guidelines, modifications to 🤖 Prompt for AI Agents |
||
| platform = espressif32@ ~6.3.2 | ||
| platform_packages = platformio/framework-arduinoespressif32 @ 3.20009.0 ;; select arduino-esp32 v2.0.9 (arduino-esp32 2.0.10 thru 2.0.14 are buggy so avoid them) | ||
| build_flags = -g | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,83 @@ | ||||||||||||||||||||||||||||
| #pragma once | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #include "wled.h" | ||||||||||||||||||||||||||||
| #include "bus_usermod.h" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| class UMB_AT8870_I2C_PSPWM : public UsermodBus | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| uint16_t getId() override { return USERMOD_ID_BUS; } | ||||||||||||||||||||||||||||
| void loop() override {} | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| void initBus(BusUsermod* bus) override | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| allocateBusData(bus, bus->getLength() * 4); | ||||||||||||||||||||||||||||
| setBusRGB(bus, true); | ||||||||||||||||||||||||||||
| setBusWhite(bus, true); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| void setBusPixelColor(BusUsermod* bus, uint16_t pix, uint32_t c) override | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| uint8_t* data = getBusData(bus); | ||||||||||||||||||||||||||||
| uint16_t i = pix * 4; | ||||||||||||||||||||||||||||
| data[i++] = R(c); | ||||||||||||||||||||||||||||
| data[i++] = G(c); | ||||||||||||||||||||||||||||
| data[i++] = B(c); | ||||||||||||||||||||||||||||
| data[i++] = W(c); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+21
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing bounds check before buffer access.
🛡️ Proposed bounds check void setBusPixelColor(BusUsermod* bus, uint16_t pix, uint32_t c) override
{
+ if (pix >= bus->getLength()) return;
uint8_t* data = getBusData(bus);
uint16_t i = pix * 4;🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| uint32_t getBusPixelColor(const BusUsermod* bus, uint16_t pix) const override | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| uint8_t* data = getBusData(bus); | ||||||||||||||||||||||||||||
| uint16_t i = pix * 4; | ||||||||||||||||||||||||||||
| return RGBW32(data[i], data[i+1], data[i+2], data[i+3]); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+31
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same bounds check needed for Reading from an out-of-range pixel index will access uninitialized or out-of-bounds memory. 🛡️ Proposed bounds check uint32_t getBusPixelColor(const BusUsermod* bus, uint16_t pix) const override
{
+ if (pix >= bus->getLength()) return 0;
uint8_t* data = getBusData(bus);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| void showBus(BusUsermod* bus) override | ||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||
| const uint8_t* pins = getBusPins(bus); | ||||||||||||||||||||||||||||
| uint32_t c = getBusPixelColor(bus, 0); | ||||||||||||||||||||||||||||
| if ( bus->hasWhite() ) { | ||||||||||||||||||||||||||||
| c = autoWhiteCalc(bus, c); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| uint8_t pwm_duty; | ||||||||||||||||||||||||||||
| uint16_t pwm_delay; | ||||||||||||||||||||||||||||
| uint8_t bri = getBusBrightness(bus); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Wire.beginTransmission(pins[0]); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Wire.write(16); // 16 == start of PWM addresses | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| pwm_delay = 0; // PWM delay for red = 0 | ||||||||||||||||||||||||||||
| pwm_duty = (R(c) * bri) / 255; // PWM duty for red, scaled by brightness | ||||||||||||||||||||||||||||
| Wire.write(pwm_duty); | ||||||||||||||||||||||||||||
| Wire.write(pwm_delay); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| pwm_delay += pwm_duty; // PWM delay for green, start after red | ||||||||||||||||||||||||||||
| if ( pwm_delay > 254 ) pwm_delay -= 255; | ||||||||||||||||||||||||||||
| pwm_duty = (G(c) * bri) / 255; // PWM duty for green, scaled by brightness | ||||||||||||||||||||||||||||
| Wire.write(pwm_duty); | ||||||||||||||||||||||||||||
| Wire.write(pwm_delay); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| pwm_delay += pwm_duty; // PWM delay for blue, start after green | ||||||||||||||||||||||||||||
| if ( pwm_delay > 254 ) pwm_delay -= 255; | ||||||||||||||||||||||||||||
| pwm_duty = (B(c) * bri) / 255; // PWM duty for blue, scaled by brightness | ||||||||||||||||||||||||||||
| Wire.write(pwm_duty); | ||||||||||||||||||||||||||||
| Wire.write(pwm_delay); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| pwm_delay += pwm_duty; // PWM delay for white, start after blue | ||||||||||||||||||||||||||||
| if ( pwm_delay > 254 ) pwm_delay -= 255; | ||||||||||||||||||||||||||||
| pwm_duty = (W(c) * bri) / 255; // PWM duty for white, scaled by brightness | ||||||||||||||||||||||||||||
| Wire.write(pwm_duty); | ||||||||||||||||||||||||||||
| Wire.write(pwm_delay); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Wire.endTransmission(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,8 @@ class Bus { | |
| type == TYPE_SK6812_RGBW || type == TYPE_TM1814 || type == TYPE_UCS8904 || | ||
| type == TYPE_FW1906 || type == TYPE_WS2805 || type == TYPE_SM16825 || // digital types with white channel | ||
| (type > TYPE_ONOFF && type <= TYPE_ANALOG_5CH && type != TYPE_ANALOG_3CH) || // analog types with white channel | ||
| type == TYPE_NET_DDP_RGBW || type == TYPE_NET_ARTNET_RGBW; // network types with white channel | ||
| type == TYPE_NET_DDP_RGBW || type == TYPE_NET_ARTNET_RGBW || | ||
| type == TYPE_USERMOD; // network types with white channel | ||
|
Comment on lines
+122
to
+123
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t hardcode every usermod bus as RGBW.
🤖 Prompt for AI Agents |
||
| } | ||
| static constexpr bool hasCCT(uint8_t type) { | ||
| return type == TYPE_WS2812_2CH_X3 || type == TYPE_WS2812_WWA || | ||
|
|
@@ -132,7 +133,7 @@ class Bus { | |
| static constexpr bool is2Pin(uint8_t type) { return (type >= TYPE_2PIN_MIN && type <= TYPE_2PIN_MAX); } | ||
| static constexpr bool isOnOff(uint8_t type) { return (type == TYPE_ONOFF); } | ||
| static constexpr bool isPWM(uint8_t type) { return (type >= TYPE_ANALOG_MIN && type <= TYPE_ANALOG_MAX); } | ||
| static constexpr bool isVirtual(uint8_t type) { return (type >= TYPE_VIRTUAL_MIN && type <= TYPE_VIRTUAL_MAX); } | ||
| static constexpr bool isVirtual(uint8_t type) { return ((type >= TYPE_VIRTUAL_MIN && type <= TYPE_VIRTUAL_MAX) || type == TYPE_USERMOD); } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In this header, 🤖 Prompt for AI Agents |
||
| static constexpr bool is16bit(uint8_t type) { return type == TYPE_UCS8903 || type == TYPE_UCS8904 || type == TYPE_SM16825; } | ||
| static constexpr int numPWMPins(uint8_t type) { return (type - 40); } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
|
|
||
| #include <Arduino.h> | ||
| #include <IPAddress.h> | ||
|
|
||
| #include "pin_manager.h" | ||
| #include "bus_wrapper.h" | ||
| #include "bus_manager.h" | ||
| #include "bus_usermod.h" | ||
|
|
||
| #include "Wire.h" | ||
|
|
||
| extern BusManager busses; | ||
| extern UsermodManager usermods; | ||
|
|
||
| void BusUsermod::setPixelColor(uint16_t pix, uint32_t c) { | ||
| if (!_valid || !_usermod || pix >= _len) return; | ||
| return _usermod->setBusPixelColor(this, pix, c); | ||
| } | ||
|
|
||
| uint32_t BusUsermod::getPixelColor(uint16_t pix) const { | ||
| if (!_valid || !_usermod || pix >= _len) return 0; | ||
| return _usermod->getBusPixelColor(this, pix); | ||
| } | ||
|
|
||
| void BusUsermod::show() { | ||
| if (!_valid || !_usermod) return; | ||
| return _usermod->showBus(this); | ||
| } | ||
|
|
||
| BusUsermod::BusUsermod(BusConfig &bc) | ||
| : Bus(bc.type, bc.start, bc.autoWhite, bc.count) | ||
| { | ||
| // safe pin configuration for the actual Usermod | ||
| _pins[0] = bc.pins[0]; | ||
| _pins[1] = bc.pins[1]; | ||
| _pins[2] = bc.pins[2]; | ||
| _pins[3] = bc.pins[3]; | ||
| _pins[4] = bc.pins[4]; | ||
|
|
||
| _valid = 1; | ||
| // error: 'dynamic_cast' not permitted with '-fno-rtti' | ||
| // _usermod = dynamic_cast<UsermodBus*> (usermods.lookup(USERMOD_ID_BUS)); | ||
| _usermod = static_cast<UsermodBus*> (usermods.lookup(USERMOD_ID_BUS)); | ||
|
|
||
| if ( _usermod ) { | ||
| _usermod->initBus(this); | ||
| } | ||
|
Comment on lines
+41
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Binding the backing usermod here is too early, and the cast is not type-safe. At this point the usermod has been registered but not yet run through Based on learnings, hardcoded usermod IDs are intentionally used as part of a strategy to avoid modifying core code. 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| void BusUsermod::cleanup(void) { | ||
| _type = I_NONE; | ||
| _valid = false; | ||
| freeData(); | ||
| } | ||
|
|
||
| uint8_t BusUsermod::getPins(uint8_t* pinArray) const { | ||
| if (!_valid) return 0; | ||
| unsigned numPins = 5; | ||
| for (unsigned i = 0; i < numPins; i++) pinArray[i] = _pins[i]; | ||
| return numPins; | ||
|
Comment on lines
+56
to
+60
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honor the Other bus implementations let callers query just the pin count without supplying a buffer. This version unconditionally dereferences Suggested fix uint8_t BusUsermod::getPins(uint8_t* pinArray) const {
if (!_valid) return 0;
unsigned numPins = 5;
- for (unsigned i = 0; i < numPins; i++) pinArray[i] = _pins[i];
+ if (pinArray) for (unsigned i = 0; i < numPins; i++) pinArray[i] = _pins[i];
return numPins;
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
|
|
||
| void UsermodBus::setup() { | ||
| uint8_t busNr; | ||
| for ( busNr = 0; busNr < busses.getNumBusses(); busNr++ ) { | ||
| BusUsermod* bus = static_cast<BusUsermod*> (busses.getBus(busNr)); | ||
| if ( bus->getType() == TYPE_USERMOD ) { | ||
| if ( bus->_usermod ) continue; | ||
| bus->_usermod = this; | ||
| initBus(bus); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,59 @@ | ||||||
| #ifndef BusUsermod_h | ||||||
| #define BusUsermod_h | ||||||
|
|
||||||
| class JsonObject; | ||||||
|
|
||||||
| #include "usermod.h" | ||||||
|
|
||||||
| class UsermodBus; | ||||||
|
|
||||||
| class BusUsermod : public Bus { | ||||||
| public: | ||||||
| BusUsermod(BusConfig &bc); | ||||||
| ~BusUsermod() { cleanup(); } | ||||||
|
|
||||||
| void setPixelColor(uint16_t pix, uint32_t c) override; | ||||||
| uint32_t getPixelColor(uint16_t pix) const override; | ||||||
| void show(void) override; | ||||||
| void cleanup(void); | ||||||
|
|
||||||
| uint8_t getPins(uint8_t* pinArray) const override; | ||||||
|
|
||||||
| protected: | ||||||
| UsermodBus* _usermod; | ||||||
| uint8 _pins[5]; // used as configuration hints for the Usermod | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use
🔧 Proposed fix- uint8 _pins[5]; // used as configuration hints for the Usermod
+ uint8_t _pins[5]; // used as configuration hints for the Usermod📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| friend class UsermodBus; | ||||||
| }; | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
| class UsermodBus : public Usermod { | ||||||
| public: | ||||||
|
|
||||||
| void setup() override; | ||||||
|
|
||||||
| protected: | ||||||
| virtual void showBus(BusUsermod* bus) = 0; | ||||||
| virtual void setBusPixelColor(BusUsermod* bus, uint16_t pix, uint32_t c) = 0; | ||||||
| virtual uint32_t getBusPixelColor(const BusUsermod* bus, uint16_t pix) const = 0; | ||||||
| virtual void initBus(BusUsermod* bus) = 0; | ||||||
|
|
||||||
| // helper functions, as C++ classes do not inherit friend class permissions | ||||||
| inline uint8_t* allocateBusData(BusUsermod* bus, size_t size = 1) { return bus->allocateData(size); } | ||||||
| inline uint8_t* getBusData(const BusUsermod* bus) const { return bus->_data; } | ||||||
| inline const uint8_t* getBusPins(const BusUsermod* bus) const { return bus->_pins; } | ||||||
| inline void setBusRGB(BusUsermod* bus, const bool hasRgb) { bus->_hasRgb = hasRgb; } | ||||||
| inline void setBusWhite(BusUsermod* bus, const bool hasWhite) { bus->_hasWhite = hasWhite; } | ||||||
| inline void setBusCCT(BusUsermod* bus, const bool hasCCT) { bus->_hasCCT = hasCCT; } | ||||||
| inline uint8_t getBusBrightness(const BusUsermod* bus) const { return bus->_bri; } | ||||||
| inline uint32_t autoWhiteCalc(const BusUsermod* bus, uint32_t c) const { return bus->autoWhiteCalc(c); } | ||||||
|
|
||||||
| friend class BusUsermod; | ||||||
| }; | ||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
| #endif | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -81,8 +81,8 @@ | |||||||||||||||||
| let nm = LC.name.substring(0,2); | ||||||||||||||||||
| let n = LC.name.substring(2); | ||||||||||||||||||
| let t = parseInt(d.Sf["LT"+n].value, 10); // LED type SELECT | ||||||||||||||||||
| // ignore IP address | ||||||||||||||||||
| if (nm=="L0" || nm=="L1" || nm=="L2" || nm=="L3") { | ||||||||||||||||||
| // ignore IP address and virtual usermod | ||||||||||||||||||
| if (nm=="L0" || nm=="L1" || nm=="L2" || nm=="L3" || nm=="L4") { | ||||||||||||||||||
| if (t>=80) return; | ||||||||||||||||||
| } | ||||||||||||||||||
| //check for pin conflicts | ||||||||||||||||||
|
|
@@ -259,7 +259,7 @@ | |||||||||||||||||
| gId("p1d"+n).innerText = p1d; | ||||||||||||||||||
| gId("off"+n).innerText = off; | ||||||||||||||||||
| // secondary pins show/hide (type string length is equivalent to number of pins used; except for network and on/off) | ||||||||||||||||||
| let pins = Math.min(gT(t).t.length,1) + 3*isNet(t); // fixes network pins to 4 | ||||||||||||||||||
| let pins = Math.min(gT(t).t.length,1) + 4*isVir(t) - 1*isNet(t); // fixes network pins to 4, other virtual to 5 | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new fifth virtual config field is still treated like a GPIO later in This branch exposes Suggested follow-up- if (nm=="L0" || nm=="L1" || nm=="L2" || nm=="L3") {
+ if (nm=="L0" || nm=="L1" || nm=="L2" || nm=="L3" || nm=="L4") {
if (isVir(t)) {
LC.max = 255;
LC.min = 0;
LC.style.color="#fff";
return; // do not check conflicts
} else {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| for (let p=1; p<5; p++) { | ||||||||||||||||||
| var LK = d.Sf["L"+p+n]; | ||||||||||||||||||
| if (!LK) continue; | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this change is obsolete - the V4 environment has evolved, and the default partition should not be set here.