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 8 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
86 changes: 66 additions & 20 deletions wled00/FX_fcn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@
*/

//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
Expand All @@ -56,8 +56,8 @@
#define DATA_PINS LEDPIN
#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,62 @@ void WS2812FX::finalizeInit(void) {
//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);

// check if we have enough pins left to configure an output of this type
// should never happen due to static assert above
if (pinsIndex + busPins > defNumPins) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO this should be asserted as well as build time misconfiguration should not be allowed.

DEBUG_PRINTLN(F("LED outputs misaligned with defined pins. Some pins will remain unused."));
break;
}

for (unsigned j = 0; j < busPins && j < OUTPUT_MAX_PINS; j++) {
defPin[j] = defDataPins[pinsIndex + 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), read/only pins, etc.
if (pinManager.isPinAllocated(defPin[j]) || pinManager.isReadOnlyPin(defPin[j])) {
defPin[j] = 1; // start with GPIO1 and work upwards
while (
(
pinManager.isPinAllocated(defPin[j]) ||
pinManager.isReadOnlyPin(defPin[j]) ||
// Check if pin is defined for current bus
pinManager.isPinDefined(defPin[j], defDataPins, pinsIndex + j, pinsIndex + busPins)
)
&&
defPin[j] < WLED_NUM_PINS
)
{
defPin[j]++;
}
}
}
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 @@ -673,7 +673,7 @@ void BusNetwork::cleanup(void) {

//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
9 changes: 3 additions & 6 deletions wled00/bus_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ class BusPwm : public Bus {
void cleanup(void) { deallocatePins(); }

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 Expand Up @@ -346,10 +346,7 @@ struct BusConfig {
{
refreshReq = (bool) GET_BIT(busType,7);
type = busType & 0x7F; // bit 7 may be/is hacked to include refresh info (1=refresh in off state, 0=no refresh)
size_t nPins = 1;
if (Bus::isVirtual(type)) nPins = 4; //virtual network bus. 4 "pins" store IP address
else if (Bus::is2Pin(type)) nPins = 2;
else if (Bus::isPWM(type)) nPins = Bus::numPWMPins(type);
size_t nPins = Bus::getNumberOfPins(type);
for (size_t i = 0; i < nPins; i++) pins[i] = ppins[i];
}

Expand Down
20 changes: 20 additions & 0 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 @@ -569,6 +572,19 @@
#endif
#endif

// List of read only pins. Cannot be used for LED outputs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we rely on HAL? Do we really need #define? It may make the code larger but then requires no maintenance.

#if defined(CONFIG_IDF_TARGET_ESP32S2)
#define READ_ONLY_PINS 46
#elif defined(CONFIG_IDF_TARGET_ESP32S3)
// none for S3
#elif defined(CONFIG_IDF_TARGET_ESP32C3)
// none for C3
#elif defined(ESP32)
#define READ_ONLY_PINS 34,35,36,37,38,39
#else
// none for ESP8266
#endif

#ifdef WLED_ENABLE_DMX
#if (LEDPIN == 2)
#undef LEDPIN
Expand All @@ -577,6 +593,10 @@
#endif
#endif

#ifndef DEFAULT_LED_TYPE
#define DEFAULT_LED_TYPE TYPE_WS2812_RGB
#endif

#ifndef DEFAULT_LED_COUNT
#define DEFAULT_LED_COUNT 30
#endif
Expand Down
21 changes: 21 additions & 0 deletions wled00/pin_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,27 @@ bool PinManagerClass::isPinOk(byte gpio, bool output) const
return false;
}

bool PinManagerClass::isReadOnlyPin(byte gpio)
{
#ifdef READ_ONLY_PINS
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer to query HAL here.
Like:

#ifdef ARDUINO_ARCH_ESP32
  if (gpio < WLED_NUM_PINS) return digitalPinCanOutput(gpio);
#endif

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.

I have tested digitalPinCanOutput but unfortunately it does not return FALSE only for read/only pins but also for other types of pins (e.g. ones that don't have pads). While this doesn't matter for finalizeInit(), the GUI displays read/only pins in orange, which cannot be achieved from what I can see without hardcoding the pins currently. We could of course just leave the pins hardcoded there, but since they might be useful in other places I figured centralizing them in const.h was a better option.

EDIT: I guess a possible solution in the GUI is to first color the pins orange if they're not a valid output, and then color them red if they're not a valid pin. That way read/only pins will stay orange while the other invalid pins will stay red.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't forget that GUI also has reserved pins, those are not populated using read-only information.

const unsigned pins[] = {READ_ONLY_PINS};
const unsigned numPins = ((sizeof pins) / (sizeof pins[0]));

if (gpio <= WLED_NUM_PINS) {
for (unsigned i = 0; i < numPins; i++) if (gpio == pins[i]) return true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

If not using HAL this could be simplified (as I've learned recently) as:

for (const auto pin : pins) if (gpio == pin) return true;

}
#endif

return false;
}

bool PinManagerClass::isPinDefined(byte gpio, const unsigned *pins, unsigned start, unsigned end) {
for (unsigned i = start; i < end; i++) {
if (pins[i] == gpio) return true;
}
return false;
}

PinOwner PinManagerClass::getPinOwner(byte gpio) const
{
if (!isPinOk(gpio, false)) return PinOwner::None;
Expand Down
3 changes: 3 additions & 0 deletions wled00/pin_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ 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);
static bool isPinDefined(byte gpio, const unsigned* pins, unsigned start = 0, unsigned end = WLED_NUM_PINS);

PinOwner getPinOwner(byte gpio) const;

Expand Down
20 changes: 9 additions & 11 deletions wled00/xml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,17 +191,15 @@ 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
Expand Down