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

Configure different kinds of busses at compile #4107

Merged
merged 16 commits into from
Sep 14, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 73 additions & 21 deletions wled00/FX_fcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,20 @@
*/

//factory defaults LED setup
//#define PIXEL_COUNTS 30, 30, 30, 30
//#define DATA_PINS 16, 1, 3, 4
//#define PIXEL_COUNTS 30
//#define DATA_PINS 2 (8266/C3) or 16
//#define DEFAULT_LED_TYPE TYPE_WS2812_RGB

#ifndef PIXEL_COUNTS
#define PIXEL_COUNTS DEFAULT_LED_COUNT
#endif

#ifndef DATA_PINS
#define DATA_PINS LEDPIN
#define DATA_PINS DEFAULT_LED_PIN
#endif

#ifndef DEFAULT_LED_TYPE
#define DEFAULT_LED_TYPE TYPE_WS2812_RGB
#ifndef LED_TYPES
#define LED_TYPES DEFAULT_LED_TYPE
#endif

#ifndef DEFAULT_LED_COLOR_ORDER
Expand All @@ -69,6 +69,18 @@
#error "Max segments must be at least max number of busses!"
#endif

static constexpr unsigned sumPinsRequired(const unsigned* current, size_t count) {
return (count > 0) ? (Bus::getNumberOfPins(*current) + sumPinsRequired(current+1,count-1)) : 0;
}

static constexpr bool validatePinsAndTypes(const unsigned* types, unsigned numTypes, unsigned numPins ) {
// Pins provided < pins required -> always invalid
// Pins provided = pins required -> always valid
// Pins provided > pins required -> valid if excess pins are a product of last type pins since it will be repeated
return (sumPinsRequired(types, numTypes) > numPins) ? false :
(numPins - sumPinsRequired(types, numTypes)) % Bus::getNumberOfPins(types[numTypes-1]) == 0;
}


///////////////////////////////////////////////////////////////////////////////
// Segment class implementation
Expand Down Expand Up @@ -1215,28 +1227,68 @@ void WS2812FX::finalizeInit() {
//if busses failed to load, add default (fresh install, FS issue, ...)
if (BusManager::getNumBusses() == 0) {
DEBUG_PRINTLN(F("No busses, init default"));
const unsigned defDataPins[] = {DATA_PINS};
const unsigned defCounts[] = {PIXEL_COUNTS};
const unsigned defNumPins = ((sizeof defDataPins) / (sizeof defDataPins[0]));
const unsigned defNumCounts = ((sizeof defCounts) / (sizeof defCounts[0]));
// if number of pins is divisible by counts, use number of counts to determine number of buses, otherwise use pins
const unsigned defNumBusses = defNumPins > defNumCounts && defNumPins%defNumCounts == 0 ? defNumCounts : defNumPins;
const unsigned pinsPerBus = defNumPins / defNumBusses;
constexpr unsigned defDataTypes[] = {LED_TYPES};
constexpr unsigned defDataPins[] = {DATA_PINS};
constexpr unsigned defCounts[] = {PIXEL_COUNTS};
constexpr unsigned defNumTypes = ((sizeof defDataTypes) / (sizeof defDataTypes[0]));
constexpr unsigned defNumPins = ((sizeof defDataPins) / (sizeof defDataPins[0]));
constexpr unsigned defNumCounts = ((sizeof defCounts) / (sizeof defCounts[0]));

static_assert(validatePinsAndTypes(defDataTypes, defNumTypes, defNumPins),
"The default pin list defined in DATA_PINS does not match the pin requirements for the default buses defined in LED_TYPES");

unsigned prevLen = 0;
for (unsigned i = 0; i < defNumBusses && i < WLED_MAX_BUSSES+WLED_MIN_VIRTUAL_BUSSES; i++) {
uint8_t defPin[5]; // max 5 pins
for (unsigned j = 0; j < pinsPerBus; j++) defPin[j] = defDataPins[i*pinsPerBus + j];
// when booting without config (1st boot) we need to make sure GPIOs defined for LED output don't clash with hardware
// i.e. DEBUG (GPIO1), DMX (2), SPI RAM/FLASH (16&17 on ESP32-WROVER/PICO), etc
if (pinManager.isPinAllocated(defPin[0])) {
defPin[0] = 1; // start with GPIO1 and work upwards
while (pinManager.isPinAllocated(defPin[0]) && defPin[0] < WLED_NUM_PINS) defPin[0]++;
unsigned pinsIndex = 0;
for (unsigned i = 0; i < WLED_MAX_BUSSES+WLED_MIN_VIRTUAL_BUSSES; i++) {
uint8_t defPin[OUTPUT_MAX_PINS];
// if we have less types than requested outputs and they do not align, use last known type to set current type
unsigned dataType = defDataTypes[(i < defNumTypes) ? i : defNumTypes -1];
unsigned busPins = Bus::getNumberOfPins(dataType);

// if we need more pins than available all outputs have been configured
if (pinsIndex + busPins > defNumPins) break;

// Assign all pins first so we can check for conflicts on this bus
for (unsigned j = 0; j < busPins && j < OUTPUT_MAX_PINS; j++) defPin[j] = defDataPins[pinsIndex + j];

for (unsigned j = 0; j < busPins && j < OUTPUT_MAX_PINS; j++) {
bool validPin = true;
// When booting without config (1st boot) we need to make sure GPIOs defined for LED output don't clash with hardware
// i.e. DEBUG (GPIO1), DMX (2), SPI RAM/FLASH (16&17 on ESP32-WROVER/PICO), read/only pins, etc.
// Pin should not be already allocated, read/only or defined for current bus
while (pinManager.isPinAllocated(defPin[j]) || !pinManager.isPinOk(defPin[j],true)) {
if (validPin) {
DEBUG_PRINTLN(F("Some of the provided pins cannot be used to configure this LED output."));
defPin[j] = 1; // start with GPIO1 and work upwards
validPin = false;
} else if (defPin[j] < WLED_NUM_PINS) {
defPin[j]++;
} else {
DEBUG_PRINTLN(F("No available pins left! Can't configure output."));
return;
}
// is the newly assigned pin already defined? try next in line until there are no clashes
bool clash;
do {
clash = false;
for (const auto pin : defDataPins) {
if (pin == defPin[j]) {
defPin[j]++;
if (defPin[j] < WLED_NUM_PINS) clash = true;
}
}
} while (clash);
}
}
pinsIndex += busPins;

unsigned start = prevLen;
// if we have less counts than pins and they do not align, use last known count to set current count
unsigned count = defCounts[(i < defNumCounts) ? i : defNumCounts -1];
// analog always has length 1
if (Bus::isPWM(dataType)) count = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

BusOnOff also has a length of 1.

prevLen += count;
BusConfig defCfg = BusConfig(DEFAULT_LED_TYPE, defPin, start, count, DEFAULT_LED_COLOR_ORDER, false, 0, RGBW_MODE_MANUAL_ONLY, 0, useGlobalLedBuffer);
BusConfig defCfg = BusConfig(dataType, defPin, start, count, DEFAULT_LED_COLOR_ORDER, false, 0, RGBW_MODE_MANUAL_ONLY, 0, useGlobalLedBuffer);
if (BusManager::add(defCfg) == -1) break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion wled00/bus_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ void BusNetwork::cleanup() {

//utility to get the approx. memory usage of a given BusConfig
uint32_t BusManager::memUsage(BusConfig &bc) {
if (Bus::isOnOff(bc.type) || Bus::isPWM(bc.type)) return 5;
if (Bus::isOnOff(bc.type) || Bus::isPWM(bc.type)) return OUTPUT_MAX_PINS;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reviewing original code, BusOnOff uses only 1 byte of data (bug in original code). Does not hurt to report 5, though.


unsigned len = bc.count + bc.skipAmount;
unsigned channels = Bus::getNumberOfChannels(bc.type);
Expand Down
4 changes: 2 additions & 2 deletions wled00/bus_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,8 @@ class BusPwm : public Bus {
static std::vector<LEDType> getLEDTypes();

private:
uint8_t _pins[5];
uint8_t _pwmdata[5];
uint8_t _pins[OUTPUT_MAX_PINS];
uint8_t _pwmdata[OUTPUT_MAX_PINS];
#ifdef ARDUINO_ARCH_ESP32
uint8_t _ledcStart;
#endif
Expand Down
21 changes: 10 additions & 11 deletions wled00/const.h
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,9 @@
#define NTP_PACKET_SIZE 48 // size of NTP receive buffer
#define NTP_MIN_PACKET_SIZE 48 // min expected size - NTP v4 allows for "extended information" appended to the standard fields

// Maximum number of pins per output. 5 for RGBCCT analog LEDs.
#define OUTPUT_MAX_PINS 5

//maximum number of rendered LEDs - this does not have to match max. physical LEDs, e.g. if there are virtual busses
#ifndef MAX_LEDS
#ifdef ESP8266
Expand Down Expand Up @@ -564,15 +567,6 @@
#define WLED_MAX_NODES 150
#endif

//this is merely a default now and can be changed at runtime
#ifndef LEDPIN
#if defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3) //|| (defined(ARDUINO_ARCH_ESP32) && defined(BOARD_HAS_PSRAM)) || defined(ARDUINO_ESP32_PICO)
#define LEDPIN 2 // GPIO2 (D4) on Wemos D1 mini compatible boards, safe to use on any board
#else
#define LEDPIN 16 // aligns with GPIO2 (D4) on Wemos D1 mini32 compatible boards (if it is unusable it will be reassigned in WS2812FX::finalizeInit())
#endif
#endif

#ifdef WLED_ENABLE_DMX
#if (LEDPIN == 2)
#undef LEDPIN
Expand All @@ -581,9 +575,14 @@
#endif
#endif

#ifndef DEFAULT_LED_COUNT
#define DEFAULT_LED_COUNT 30
// Defaults pins, type and counts to configure LED output
#if defined(ESP8266) || defined(CONFIG_IDF_TARGET_ESP32C3)
#define DEFAULT_LED_PIN 2 // GPIO2 (D4) on Wemos D1 mini compatible boards, safe to use on any board
#else
#define DEFAULT_LED_PIN 16 // aligns with GPIO2 (D4) on Wemos D1 mini32 compatible boards (if it is unusable it will be reassigned in WS2812FX::finalizeInit())
#endif
#define DEFAULT_LED_TYPE TYPE_WS2812_RGB
#define DEFAULT_LED_COUNT 30

#define INTERFACE_UPDATE_COOLDOWN 1000 // time in ms to wait between websockets, alexa, and MQTT updates

Expand Down
2 changes: 1 addition & 1 deletion wled00/data/settings_leds.htm
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@
if (i.type === "number" && fields.includes(i.name)) { //select all pin select elements
let v = parseInt(i.value);
let sel = addDropdown(i.name,0);
for (var j = -1; j <= d.max_gpio; j++) {
for (var j = -1; j < d.max_gpio; j++) {
if (d.rsvd.includes(j)) continue;
let foundPin = d.um_p.indexOf(j);
let txt = (j === -1) ? "unused" : `${j}`;
Expand Down
12 changes: 6 additions & 6 deletions wled00/data/settings_um.htm
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
GetV();
for (let r of d.rsvd) { pins.push(r); pinO.push("rsvd"); } // reserved pins
if (d.um_p[0]==-1) d.um_p.shift(); // remove filler
d.Sf.SDA.max = d.Sf.SCL.max = d.Sf.MOSI.max = d.Sf.SCLK.max = d.Sf.MISO.max = d.max_gpio;
//for (let i of d.getElementsByTagName("input")) if (i.type === "number" && i.name.replace("[]","").substr(-3) === "pin") i.max = d.max_gpio;
d.Sf.SDA.max = d.Sf.SCL.max = d.Sf.MOSI.max = d.Sf.SCLK.max = d.Sf.MISO.max = d.max_gpio-1;
//for (let i of d.getElementsByTagName("input")) if (i.type === "number" && i.name.replace("[]","").substr(-3) === "pin") i.max = d.max_gpio-1;
pinDD(); // convert INPUT to SELECT for pins
});
// error event
Expand Down Expand Up @@ -80,7 +80,7 @@
for (var i=0; i<pins.length; i++) {
if (k==pinO[i]) continue;
if (o.value==pins[i] && pinO[i]==="if") { o.style.color="lime"; break; }
if (o.value==pins[i] || o.value<-1 || o.value>d.max_gpio) { o.style.color="red"; break; } else o.style.color=d.ro_gpio.some((e)=>e==parseInt(o.value,10))?"orange":"#fff";
if (o.value==pins[i] || o.value<-1 || o.value>d.max_gpio-1) { o.style.color="red"; break; } else o.style.color=d.ro_gpio.some((e)=>e==parseInt(o.value,10))?"orange":"#fff";
}
} else {
switch (o.name) {
Expand All @@ -94,7 +94,7 @@
for (var i=0; i<pins.length; i++) {
//if (k==pinO[i]) continue; // same owner
if (o.value==pins[i] && pinO[i]==="if") { o.style.color="tomato"; break; }
if (o.value==pins[i] || o.value<-1 || o.value>d.max_gpio) { o.style.color="red"; break; } else o.style.color=d.ro_gpio.some((e)=>e==parseInt(o.value,10))?"orange":"#fff";
if (o.value==pins[i] || o.value<-1 || o.value>d.max_gpio-1) { o.style.color="red"; break; } else o.style.color=d.ro_gpio.some((e)=>e==parseInt(o.value,10))?"orange":"#fff";
}
}
*/
Expand Down Expand Up @@ -148,7 +148,7 @@
case "number":
c = `value="${o}"`;
if (f.substr(-3)==="pin") {
c += ` max="${d.max_gpio}" min="-1" class="s"`;
c += ` max="${d.max_gpio-1}" min="-1" class="s"`;
t = "int";
} else {
c += ' step="any" class="xxl"';
Expand All @@ -170,7 +170,7 @@
if (i.type === "number" && (i.name.includes("pin") || ["SDA","SCL","MOSI","MISO","SCLK"].includes(i.name))) { //select all pin select elements
let v = parseInt(i.value);
let sel = addDD(i.name,0);
for (var j = -1; j <= d.max_gpio; j++) {
for (var j = -1; j < d.max_gpio; j++) {
if (d.rsvd.includes(j)) continue;
let foundPin = pins.indexOf(j);
let txt = (j === -1) ? "unused" : `${j}`;
Expand Down
8 changes: 8 additions & 0 deletions wled00/pin_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,14 @@ bool PinManagerClass::isPinOk(byte gpio, bool output) const
return false;
}

bool PinManagerClass::isReadOnlyPin(byte gpio)
{
#ifdef ARDUINO_ARCH_ESP32
if (gpio < WLED_NUM_PINS) return digitalPinCanOutput(gpio);
Copy link
Author

@PaoloTK PaoloTK Sep 14, 2024

Choose a reason for hiding this comment

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

This should be !digitalPinCanOutput().

Unfortunately even with that fixed it still doesn't work because this function also returns FALSE on other pins than R/O (for example 28 to 31 on ESP32).

While as far as the finalizeInit() code this doesn't matter I think we should not return TRUE for pins that are not read/only on a function called isReadOnlyPin(). This could lead to bugs in the future (e.g. usermod wanting to use this function to check if a pin can be used to read data).

There are two ways I can think of to get around this problem:

  1. Hardcode the read only pins (which I know you're not a fan of).
  2. Remove the orange coloring in the GUI for read/only pins. We don't need to check if a pin is r/o in finalizeInit() and if the r/o pins are red (since they fail the digitalPinCanOutput() check) in the GUI instead of orange I don't think it's a big deal. As far as I can tell there's no other place in the codebase that does a r/o check, but please correct me if I'm wrong.

EDIT: I'll try implement what I suggested above: first color digitalPinCanOutput() pins orange, then color digitalPinIsValid() red.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately even with that fixed it still doesn't work because this function also returns FALSE on other pins than R/O (for example 28 to 31 on ESP32).

This is used in pin manager since ever:

if (output) return digitalPinCanOutput(gpio);

If you do not trust HAL (you may have reasons) then use isPinOk(gpio) != isPinOk(gpio,false) to determine RO pins. This way there will only be one place to configure GPIOs and that is isPinOk().

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately even with that fixed it still doesn't work because this function also returns FALSE on other pins than R/O (for example 28 to 31 on ESP32).

This is used in pin manager since ever:

if (output) return digitalPinCanOutput(gpio);

If you do not trust HAL (you may have reasons) then use isPinOk(gpio) != isPinOk(gpio,false) to determine RO pins. This way there will only be one place to configure GPIOs and that is isPinOk().

Sorry, I had already implemented the logic fix before reading your message. Both options work the same in my testing so your call on which you prefer:

pinManager.isPinOk(i) != pinManager.isPinOk(i,false)

digitalPinIsValid(gpio) && !digitalPinCanOutput(gpio)

#endif
return false;
}

PinOwner PinManagerClass::getPinOwner(byte gpio) const
{
if (!isPinOk(gpio, false)) return PinOwner::None;
Expand Down
2 changes: 2 additions & 0 deletions wled00/pin_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ class PinManagerClass {
bool isPinAllocated(byte gpio, PinOwner tag = PinOwner::None) const;
// will return false for reserved pins
bool isPinOk(byte gpio, bool output = true) const;

static bool isReadOnlyPin(byte gpio);

PinOwner getPinOwner(byte gpio) const;

Expand Down
32 changes: 10 additions & 22 deletions wled00/xml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,32 +191,20 @@ void appendGPIOinfo() {

// add info for read-only GPIO
oappend(SET_F("d.ro_gpio=["));
#if defined(CONFIG_IDF_TARGET_ESP32S2)
oappendi(46);
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
// none for S3
#elif defined(CONFIG_IDF_TARGET_ESP32C3)
// none for C3
#elif defined(ESP32)
oappend(SET_F("34,35,36,37,38,39"));
#else
// none for ESP8266
#endif
bool firstPin = true;
for (unsigned i = 0; i < WLED_NUM_PINS; i++) {
if (pinManager.isReadOnlyPin(i)) {
// No comma before the first pin
if (!firstPin) oappend(SET_F(","));
oappendi(i);
firstPin = false;
}
}
oappend(SET_F("];"));

// add info about max. # of pins
oappend(SET_F("d.max_gpio="));
#if defined(CONFIG_IDF_TARGET_ESP32S2)
oappendi(46);
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
oappendi(48);
#elif defined(CONFIG_IDF_TARGET_ESP32C3)
oappendi(21);
#elif defined(ESP32)
oappendi(39);
#else
oappendi(16);
#endif
oappendi(WLED_NUM_PINS);
oappend(SET_F(";"));
}

Expand Down