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

ESP32 Different pins cause boot loop #621

Open
mdigibou opened this Issue Jul 2, 2018 · 15 comments

Comments

Projects
None yet
3 participants
@mdigibou

mdigibou commented Jul 2, 2018

Hardware: NodeMCU32s or Wemos32, really doesn't matter much. I use the same sketch across a number of different ESP32 dev boards.

I've been working on a modular sketch where I can specify a code in the beginning of the sketch and it will generate a different output for a different product (all of which are controlled by the same phone BLE software)

Well, I hit a number of just strange and arbitrary issues that I had to tinker with to get it to work, but I isolated one for certain. Notice in DEVICE_TYPE == 2, I have those pins commented out? Well if I use those pins (like i initially planned) the device gets stuck with all kinds of RMT errors and bootloop errors. But if I use the same pins as defined in DEVICE_TYPE == 1 / 2, the sketch processes fine and runs great.

Basically, the pin definition crashes FastLED.show() which I'm calling from a task. If you guys need to see any more of my code, let me know but I have 100% isolated the boot looping issue to those 3 pins.
I read in another one of the issues that anything above Pin 32 causes issues, but notice in DEVICE_TYPE == 1, I do have a single pin above 32 in there (of course the stupid ESP32 pinout isn't in order so maybe that's something to do with it?)

Either way, It would really be nice to have some kind of compilation level error handled for this rather than chasing down Core 0 panics.

`#if DEVICE_TYPE == 0
#define LED_TYPE WS2812B
#define COLOR_ORDER GRB
#define MAXIMUM_IDLE_MINUTE_TIMER 300

const uint8_t MAX_BRIGHTNESS = 255,
MIN_BRIGHTNESS = 20,
BPM_ADD = 30;

//Definitely check the pinout on these. I was not consistent in wiring the first few sets.

//Left Eye
#define ZONE_1_PIN 25
#define ZONE_1_NUM 12
CRGB Zone1[ZONE_1_NUM];

//Right Eye
#define ZONE_2_PIN 26
#define ZONE_2_NUM 12
CRGB Zone2[ZONE_2_NUM];
#elif DEVICE_TYPE == 1
#define LED_TYPE WS2812B
#define COLOR_ORDER GRB
#define MAXIMUM_IDLE_MINUTE_TIMER 30

const uint8_t MAX_BRIGHTNESS = 255,
MIN_BRIGHTNESS = 70,
BPM_ADD = 0;

//Nose Panel
#define ZONE_1_PIN 14
#define ZONE_1_NUM 32
CRGB Zone1[ZONE_1_NUM];

//Front Bumper
#define ZONE_2_PIN 27
#define ZONE_2_NUM 48
CRGB Zone2[ZONE_2_NUM];

//Driver Body
#define ZONE_3_PIN 26
#define ZONE_3_NUM 89
CRGB Zone3[ZONE_3_NUM];

//Passenger Body
#define ZONE_4_PIN 25
#define ZONE_4_NUM 89
CRGB Zone4[ZONE_4_NUM];

//Rear Bumper
#define ZONE_5_PIN 33
#define ZONE_5_NUM 42
CRGB Zone5[ZONE_5_NUM];
#elif DEVICE_TYPE == 2
#define LED_TYPE WS2812B
#define COLOR_ORDER GRB
#define MAXIMUM_IDLE_MINUTE_TIMER 60

const uint8_t MAX_BRIGHTNESS = 140,
MIN_BRIGHTNESS = 40,
BPM_ADD = 0;
//Driver Interior
#define ZONE_1_PIN 14//32
#define ZONE_1_NUM 24
CRGB Zone1[ZONE_1_NUM];

//Passenger Interior
#define ZONE_2_PIN 27//35
#define ZONE_2_NUM 24
CRGB Zone2[ZONE_2_NUM];

//Rear Interior
#define ZONE_3_PIN 26//34
#define ZONE_3_NUM 12
CRGB Zone3[ZONE_3_NUM];
#endif`

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 3, 2018

See #617 - pins above 32 aren’t currently supported - not sure why you weren’t getting a compile error for them - you should’ve been (I think the library was giving an error at one point then someone reset the max pins define without actually adding support)

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 3, 2018

Oh - wait, this is with the rmt- that’s why it doesnt catch an error at compile time - @samguyer - thoughts on this with the rmt?

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 6, 2018

Hi! I am working on rewriting the fastpin.h definition for ESP32, but I've noticed that even now is does not have pin instantiations for pins above 32, but that does not cause any compilation error.

@focalintent Can you give me some guidance on what the fastpin interface needs to provide in order to work properly?

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 6, 2018

It does cause a compilation error if you try to use pin 32 or above for a bit-banged SPI chipset or if you just try to use/instantiate one on your own - e.g. in a bit of code:

FastPin<32>::hi();

What do you mean on what the interface needs to provide? The FastPin interface in there for esp32 has all the methods that it needs - it's just all the i/o registers only refer to the i/o registers for the low 32 pins, 0-31.

setInput/setOutput - flag whether the pin is for input or output
hi/lo/set - set the pin hi/lo or to a specific value
toggle - reverse the pin state (if it's high, set it low, if it's low set it high)
strobe - just calls toggle twice

the variations on hi/lo/fastset that take a port register are mostly used for some optimizations on some platforms, but having them just call through like they are is fine.

hival/loval are another optimization for setting pin values when interrupts are disabled, they get the current value of the GPIO register tweaked for setting the instantiated pin hi or low

port gives the regular GPIO register
sport gives the GPIO register that sets the pin when a 1 value is written to it (but does nothing otherwise - it's for platforms that have a dedicated register for setting a pin high)
cport gives the GPIO register that clears the pin when a 1 value is written to it (but does nothing otherwise) -- the sport/cport allow you to set/clear a pin without having to do an read and or/and on the existing register value.
mask gives the bitmask for the given pin

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

@focalintent I think my question is more about what parts of the interface are needed and by what other code. The RMT implementation of the clockless support does not need any of the FastPin methods because only the RMT peripheral accesses the pins.

Here are my main questions:

What methods are needed by other parts of FastLED (presumably, the clocked chips)? Are the 32 bit masks used anywhere outside of the ESP platform support?

Given that the ESP chip is pretty fast, do I need to worry a lot about performance? In other words, can I just call the convenience methods in the ESP core that do these things?

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 7, 2018

Please implement the full class - I don't know off the top of my head which of the methods are used by what other parts of the code - but what i don't want to do is start tracking with platforms aren't implementing what parts of fastpin - and for now you can probably just call the convenience functions (though that will likely slow down the speed of the bit-bang'd spi output a fair bit), so i would prefer that it be done right at the register level (also there's a backlog of other led chipset types that will likely require some level of pin level twiddling and timing again)

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 7, 2018

And - looking at the bitbang'd spi code - it makes use of a bunch of the variations in the FastPin class.

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

OK, will do. And the whole 32-bit mask thing? It seems to be only a part of the inner workings of the FastPin. Does that sound right?

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

Ignore that last comment -- I see where it's using the masks in the bitbanging code.

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 7, 2018

the mask method on FastPin is used by the bitbang'd SPI output so it's still needed (how it's implemented internally is ¯_(ツ)_/¯ - it doesn't have to be the template parameter, though I found having it as a template parameter made a bunch of things easier in the implementation of the templated class) -- I would imagine the pins above pin32 are just on a separate set of GPIO registers, right? Like _GPB1 instead of _GPB0 or some such?

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

Pretty much. Here is the definition of the digitalWrite function from the ESP core:

extern void IRAM_ATTR __digitalWrite(uint8_t pin, uint8_t val)
{
    if(val) {
        if(pin < 32) {
            GPIO.out_w1ts = ((uint32_t)1 << pin);
        } else if(pin < 34) {
            GPIO.out1_w1ts.val = ((uint32_t)1 << (pin - 32));
        }
    } else {
        if(pin < 32) {
            GPIO.out_w1tc = ((uint32_t)1 << pin);
        } else if(pin < 34) {
            GPIO.out1_w1tc.val = ((uint32_t)1 << (pin - 32));
        }
    }
}
@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

I tried to understand what you did for the ARM chips, which have way more that 32 pins, but all of the macro stuff kind of blew my mind!

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 7, 2018

For your amusement: I have two laptops open in front of me right now. On one machine I'm hacking FastLED; on the other I'm running static analysis experiments at Veracode. :-)

@focalintent

This comment has been minimized.

Member

focalintent commented Jul 7, 2018

Yeah - trying to access register variables as template parameters got a bit ugly/tricky really fast - especially to get them to be effectively treated as constants at compile time. (In an ideal world, if the implementation of FastPin is done properly, it will be a write to a register (the MASK value usually) and then a store of that register value to a constant address. (On AVR this actually compiles down to a single instruction - which is a constant write directly to an i/o register))

@samguyer

This comment has been minimized.

Contributor

samguyer commented Jul 9, 2018

OK, I submitted a pull request with the new definitions. I also added an instantiation of FastPin in the clockless template, so that it will also check the pin choice. Let me know if you need anything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment