globals.h has erroneous LED_PIN0 defines #681
Replies: 2 comments
-
I've considered this issue for a while, and I've now made up my mind that I'm turning it into a discussion. The issue with the issue is that the response to each point is "it depends on how you look at it". What is true is that the current build configuration across and within individual projects(/environments/configurations/...) is complicated, which has been a recurring topic of conversation within the overall NightDriverStrip project/repo, and will continue boiling up every now and then until someone comes up with a brilliant idea to sort the whole thing out. In the meantime, what is there now works. The learning curve one has to surmount before being comfortable with making changes is just uncomfortably steep. Who knows, maybe this post in discussion form will turn out to be the hook for the conversation that leads to a setup that is objectively better. |
Beta Was this translation helpful? Give feedback.
-
I saw this and sat on it because Rutger and I have discussed it before. I'll offer to comments on the question as actually asked before launching into one of my megaword rambles.
$ sort include/globals.h | uniq -c | sort -rn | less Most of those are untrue. Count the number of RING and FAN definitions in builds that don't obviously (which I respect is different than "obviously don't") contain either. Most such hunks happen to match the block before it. I've poked at this a few times as I've learned more about individual combinations, but there's little doubt that globals.h does not speak absolute truth. I suspect that the targets you're looking at are ones that don't actually HAVE remote support, but that had their blocks in globals.h copied and pasted, resulting in #defines that are set, but that have just never had hardware attached in that configuration or that never had the corresponding flag (which looks like it enables it, but it's more complicated than that) in platformio.ini. Now, to Rutger's invitation to "sort it out"... The approach I've taken in build files and globals.h is to break it into halves. (There are actually two additional halves that I'm leaving out for now. I think I can not mention them further here and change nothing in this distinction, but if this conversation goes anywhere, I reserve the right to add them back...) Any plan to sort it out should probably acknowledge that the current plan allows for many configurations that aren't actually provided. Replacement schemes shouldn't necessarily feel obligated to actually enumerate every possible combination, IMO. The overhead would be ridiculous. We don't necessarily need to make it possible to build Dave's PDP or Eave configurations for lilygo_amoled and m5stick and everything else without an edit; it should be enough to make it possible. It's not awesome because of PlatformIO's build syntax, but the way I've treated this is to treat PROJECTS and EFFECTS differently. They're currently midduled together and some of the configuration is done in plaatformio.ini and some is done in globals.h. For example, most of the matrix (mesmerizer) effects would actually work on the hardware configuration commonly called "spectrum" in this project. They might look better in a 2x2 grid than in a 3x1, but let's first agree that the basic math/logic that animates a gif or makes a bouncing ball into a pixel buffer and then ships that buffer to either a WS2812 serpentine or a HUB75 makese sense in the same way that any of the 1D strip "line" (not necessarily linear) effects { shooting star, laser, makes sense on any of {Lolind32, tinypico, wrover, wroom, ten kinds of m5sticks, or a gaggle of LilyGos. We have a few exceptions lik both MESMERIZER as an effect and as a project controlling specific hardware, but we can surely agree that LASERLINE, PDPGRID, or LANTERN can easily exist on even our most humble ESP32 hardware. Currently, things like pin numbers are engangled these effects. That's not great. We have the PANLEE project defining USE_SCREEN in p*ini, but PLATECOVER defines it in globals.h. My campaign platform was to cleave them more formally. I've been using BOARDS and EFFECTS. PDPGRID, CUBE, MAGICMIRROR, LASERLINE, etc. are effects. (Personally, I'd rename them as such and would be willing to spend the keystrokes to do so.) Looking at LASERLINE in globals, matrix_width and _height are the only traits that I see that are "obviously" traits of laserlin-edness. Maybe DEFAULT_EFFECT_INTERVAL. Everything else that's set to a non-default value LED_PIN0, TOGGLE_BUTTONs) are a trait of the controller it's attached to. Those should be part of whatever env_foo. My proposal is to have a boards.ini (name TBD) that defines the traits of the boards that WE care about - notably not every analog/digital signal on these things. This spelling isn't quite right, but remembering that multiple 'extends' doesn't really work in a sane way and 'interpolation' is dicey, we just end up with named sections. [board_heltec_wifi_v3] [board_m5stick-c] [board_m5stick-c-plus [board_mesmerizer] [board_n8r16] [board_waveshare_esp32s3_147_demo]
(I'm skeptical those last two are needed, but I'll go with it in the name of moving on.) Then, down in the environments, we just do the final merges: ... and so on. I'm not totally sure how we get past the squirreley limits of platformio's build syntax in all this. My elevator sales pitch here is: Strongly fence off the board flags (pins, flash size, etc.) and EFFECTS. RJL P.S. Step 0 in doing this responsibly is making a machine-verifiable way to collect the existing flags passed to all builds. I started such a tool some time ago and kind of lost wind with it. That process itself wasn't pretty. |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
I found some conflicting pin assignments in the globals.h file
I'll try to go through the ones that I found here:
1: In the "INSULATORS" and "CUBE" sections of globals.h, both LED_PIN0 and IR_REMOTE_PIN are defined to pin 26 - which would cause a conflict, right?
2: In Dave's videos where he uses the M5StickCplus, he said pin 32 should be assigned to LED_PIN0, (which is the very convenient yellow pin on the grove connector) - but this is inconsistently assigned to pin 26 in some places within globals.h. like FANSET, TREESET, HELMET, SINGLE_INSULATOR, etc.
#if USE_M5
#define LED_PIN0 32
So, I would argue that an #if USE_M5 condition be added throughout to make sure pin 32 is assigned to all m5 projects consistently.
And a couple other issues.....
3: in globals.h, lines 309 through 315 are repeated on lines 320 through 316. (here, the m5 pin assignment is set to 32 as well)
4: in globals.h M5STICKC devices are assigned pin 33, (or 26 in SINGLE_INSULATOR) for LED_PIN0, but all other m5 devices are defined as 32. - (why the difference?)
Beta Was this translation helpful? Give feedback.
All reactions