Skip to content

Recommit patch 732#735

Merged
rbergen merged 2 commits into
PlummersSoftwareLLC:mainfrom
robertlipe:732_redo
Jul 24, 2025
Merged

Recommit patch 732#735
rbergen merged 2 commits into
PlummersSoftwareLLC:mainfrom
robertlipe:732_redo

Conversation

@robertlipe

Copy link
Copy Markdown
Collaborator

Description

Reconstituted change from #732

Contributing requirements

  • I read the contribution guidelines in CONTRIBUTING.md.
  • I understand the BlinkenPerBit metric, and maximized it in this PR.
  • I selected main as the target branch.
  • All code herein is subjected to the license terms in COPYING.txt.

…esmerizer.

buddybuild still in progress. Will update in ~1 hour.
@robertlipe

Copy link
Copy Markdown
Collaborator Author

A build-all succeeded. I'm officially happy with this now.

I'd probably be happier to move spectrum-electrow to the I2S_PINS scheme, but I don't have one, so I'm trying to touch it as lightly as possible. This at least puts it back on the same path with the same values.

@robertlipe

robertlipe commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator Author

Confirmed building everywhere.

Platformio even lies about how slow it is. Scrolling up I can see that it refetched and reinstalled every library and then recompiled all our objects for all 41 of our committed targets (never mind that it had done so an hour before for a build-all instead of a buddy-build or a pio-run or something) so every one of these builds is 5+ minutes.

We can't parse an error message out of the demo build in 7.2 seconds. Hrmph.



========================= [SUCCESS] Took 66.12 seconds =========================

Environment               Status    Duration
------------------------  --------  -------------
demo                      SUCCESS   00:00:07.208
m5demo                    SUCCESS   00:00:09.151
m5plusdemo                SUCCESS   00:00:09.664
m5stackdemo               SUCCESS   00:01:03.991
heltecdemo                SUCCESS   00:00:07.611
heltecv2demo              SUCCESS   00:01:03.357
heltecv3demo              SUCCESS   00:01:07.794
helteclorav3demo          SUCCESS   00:01:07.033
lilygo-tdisplay-s3-demo   SUCCESS   00:01:06.415
demo_amoled               SUCCESS   00:01:23.637
ledstrip                  SUCCESS   00:00:08.506
ledstrip_feather          SUCCESS   00:00:09.400
ledstrip_feather_hexagon  SUCCESS   00:01:17.548
ledstrip_feather_wrover   SUCCESS   00:01:06.944
ledstriplite              SUCCESS   00:01:09.799
ledstrip_pico             SUCCESS   00:01:00.208
spectrum                  SUCCESS   00:00:11.106
spectrum2                 SUCCESS   00:01:11.232
helmet                    SUCCESS   00:01:16.952
spectrum_elecrow          SUCCESS   00:00:10.752
spectrumstack             SUCCESS   00:01:17.1000
spectrumlite              SUCCESS   00:01:45.739
spectrum_wrover_kit       SUCCESS   00:00:56.503
mesmerizer                SUCCESS   00:00:08.532
mesmerizer_lolin          SUCCESS   00:01:00.582
laserline                 SUCCESS   00:01:02.686
lantern                   SUCCESS   00:01:01.339
pdpgrid                   SUCCESS   00:01:04.813
chieftain                 SUCCESS   00:00:07.862
umbrella                  SUCCESS   00:00:07.537
generic                   SUCCESS   00:00:07.797
panlee                    SUCCESS   00:00:08.428
ttgo                      SUCCESS   00:00:55.517
xmastrees                 SUCCESS   00:00:09.893
insulators                SUCCESS   00:01:02.918
magicmirror               SUCCESS   00:01:05.439
atomlight                 SUCCESS   00:00:54.629
spirallamp                SUCCESS   00:01:05.857
platecover                SUCCESS   00:01:02.949
fanset                    SUCCESS   00:01:04.486
cube                      SUCCESS   00:01:06.122
========================= 41 succeeded in 00:32:25.937 ======================

@rbergen

rbergen commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator

This is why I delegate the "confirmation builds" to the GitHub Actions workflows in my fork. You could do the same. :)

I've noticed that the USE_I2S_AUDIO(_PINS) defines are not set or checked anywhere else. As a general principle I don't like adding "dead defines", but I assume there is a solemn intention to use these very shortly? Disclaimer: I did not review the other 2 PRs you opened, yet.

@rbergen

rbergen commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator

By the way, #733 and #734 suffer from the same problem #732 did, so I think they will have to undergo the same (or similar) treatment.

@robertlipe

robertlipe commented Jul 24, 2025 via email

Copy link
Copy Markdown
Collaborator Author

@rbergen

rbergen commented Jul 24, 2025

Copy link
Copy Markdown
Collaborator

I did mean to say "execute GitHub Actions on [the branches in] your fork".

Your laptop is probably doing that because of how your git remotes are set up, but I can't be sure without looking over your shoulder, so to speak.

Concerning the PR branches: again I can't be sure, but it looks like the branches for #733 and #734 are based on the branch for #732, thereby incorporating its changes as well.

Concerning the otherwise unused defines: you're getting a Most Valued Contributor pass on those, then. :) Which I know sounds extremely elitarian/arrogant on my part, but isn't: it is a genuine expression of appreciation for the work you do here.

Which brings me to the final point: the conversion of magic numbers to defines is solid and CI is happy, so I'll merge this after posting this comment.

@rbergen
rbergen merged commit 8abf080 into PlummersSoftwareLLC:main Jul 24, 2025
43 checks passed
@robertlipe

robertlipe commented Jul 24, 2025 via email

Copy link
Copy Markdown
Collaborator Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants