Skip to content

Conversation

@adrianschroeter
Copy link
Contributor

before a x=32 (n times of 16) had not equal sized bars, but first was a single pixel and later a bar had 3 pixel width. This solves it to have always 2 pixel sized bars.

I have to admit that I did not test with other pixel dimensions.

@blazoncek
Copy link
Collaborator

It is easy to test, just resize segment.

@blazoncek blazoncek requested a review from softhack007 March 17, 2024 09:52
@blazoncek blazoncek added effect waiting for feedback addition information needed to better understand the issue labels Mar 17, 2024
@softhack007
Copy link
Member

I have to admit that I did not test with other pixel dimensions.

PR looks reasonable, however I'm currently "away from WLED" (holidays) and cannot test.

@adrianschroeter please test with segments of different width, and inform us about results. At least the following should be tested:

  • width=2
  • width=8
  • width=15
  • width=16
  • width=17
  • width=23
  • width=24
  • width=32

Also please try a few different numbers of bars (slider), in addition to the standard of 16 bars.

@blazoncek
Copy link
Collaborator

@adrianschroeter are you willing to pursue this PR further? If so, please rebase it for 0_15 branch.

blazoncek and others added 24 commits April 3, 2024 18:31
updated changelog (missing credit)
Update cdata.js to rebuild if package.json changes
usermod support for Adafruit MAX17048 module
millis()/1000 rollover after 18h was not handled, truncating to 16bits after division fixes it.
- add "clock" - CPU clock in MHz
- add "flash" - flash size in MB
Fix for wled#3879
Indentation fix
Indentation fix
Compile fix
- audioreactive always included for S3 & S2
Effect: modified KITT (Scanner)
- fix for wled#3896
- fix WS2815 current
- conditional AA setPixelColor()
blazoncek and others added 13 commits July 3, 2024 07:12
…d_improvement_new

Usermod Updated: Internal Temperature V2
- FX: Breathe, Meteor
- IR: use Segment
- UI: palette search, LED settings
- LED memory calculation (not UI)
- potential fix for wled#4040
- compiler warning in FX
Avoid reconfiguring the device during web server context, which can
trigger a yield().  Instead defer the device initialization to loop().
Fix typo in "Battery" usermod (Build Failed)
* fixed a few typo's in comments
* fixed 8266 specific warning about 'comparison of integer expressions of different signedness'

based on recommendations made by @willmmiles:
* make sure that audioSyncPacket is the same size  (44bytes) on all platforms
* use static buffer for receiving (avoids heap fragmentation)
* copy receive buffer to local audioSyncPacket struct - avoids alignment problems
* esp32 only: to stay in sync with UDP, Udp.flush() is needed when Udp.parsePacket() is _not_ followed by Udp.read()
Copy link
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

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

While the code change is aligned to "common practice" for avoiding inaccuracies of map, we still need confidence by testing with several segment width and "number of bars" settings.


for (int x=0; x < cols; x++) {
uint8_t band = map(x, 0, cols-1, 0, NUM_BANDS - 1);
uint8_t band = map(x, 0, cols, 0, NUM_BANDS);
Copy link
Member

@softhack007 softhack007 Jul 12, 2024

Choose a reason for hiding this comment

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

This is in line with the common workaround for know inaccuracies of map, by adding 1 to in_max and out_max.

However - as already stated - it would be good to test with common segment sizes before merging.
@adrianschroeter you can simply change the segment size to test your proposed modification.

willmmiles and others added 8 commits July 12, 2024 19:16
Add additional clarification as to the original source URL and the
specific local patches.
... protected against array overflow due to previous "if (packetSize <= UDPSOUND_MAX_PACKET)"
ESP8266 audioreactive UDP sound sync ported from MoonModules/WLED - receive only
use
-D WLED_BOOTUPDELAY=500
in platformio env definition to add 500ms of delay before hardware init.
boot-up delay to fix wifi not starting in some setups

use
`-D WLED_BOOTUPDELAY=500` (or some other delay you want, in milliseconds)
in platformio env definition to add 500ms of delay before hardware init.
before a x=32 (n times of 16) had not equal sized bars, but first was
a single pixel and later a bar had 3 pixel width. This solves it to
have always 2 pixel sized bars.

I have to admit that I did not test with other pixel dimensions.
@adrianschroeter
Copy link
Contributor Author

Hi, I finally rebased and tested your suggested segment sizes.

Apart from width = 2 it made all sense to me. The 17, 23, 24 had different sized bars, but I think that is expect.

width=2 showed only 2 led's to me (not 2x2). However, not sure if this resolution is so useful.

@softhack007 softhack007 removed the waiting for feedback addition information needed to better understand the issue label Jul 27, 2024
@softhack007
Copy link
Member

@adrianschroeter thanks for the testing 👍

I still see the PR is targeted at our 'main' branch. Please rebase it for 0_15, then we can merge.

@adrianschroeter
Copy link
Contributor Author

0_15 merge in new request

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.