Skip to content

Commit

Permalink
Orange flash fix (#3196) for transitions
Browse files Browse the repository at this point in the history
  • Loading branch information
@blazoncek
blazoncek committed Jan 14, 2024
1 parent 45dc5e2 commit 21d21ea
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
7 changes: 7 additions & 0 deletions wled00/wled.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,13 @@ void WLED::beginStrip()
else if (bri == 0) bri = 128;
} else {
// fix for #3196
if (bootPreset > 0) {
bool oldTransition = fadeTransition; // workaround if transitions are enabled

This comment has been minimized.

Sign in to view

Copy link
@DedeHai

DedeHai Nov 16, 2024

Collaborator

if these lines are moved further above and always executed, the glitch is also fixed when turnOnAtBoot is enabled.

I tested this code and found no downside. Am I missing something or would this be a better fix?

strip.setShowCallback(handleOverlayDraw);

  // fix for #3196
  bool oldTransition = fadeTransition;    // workaround if transitions are enabled
  fadeTransition = false;                 // ignore transitions temporarily
  strip.setColor(0, BLACK);               // set all segments black
  fadeTransition = oldTransition;         // restore transitions

  if (turnOnAtBoot) {
    if (briS > 0) bri = briS;
    else if (bri == 0) bri = 128;
  } else {
    // fix for #3196
    col[0] = col[1] = col[2] = col[3] = 0;  // needed for colorUpdated() if bootPreset is set
    briLast = briS; bri = 0;
    strip.fill(BLACK);
    strip.show();
  }

This comment has been minimized.

Sign in to view

Copy link
@blazoncek

blazoncek Nov 16, 2024

Author Collaborator

This will defeat default orange color.
Current solution is correct as far as original behaviour is expected.

  • turn on at boot: will light up orange
  • preset: starts with a black background

This comment has been minimized.

Sign in to view

Copy link
@DedeHai

DedeHai Nov 17, 2024

Collaborator

I was a bit too quick with the comment, sorry for that. What I actually tested was the code you introduced in DedeHai@ba3a61f
So moving this initialisation of the segments before the if (turnOnAtBoot):

// set all segments black (no transition)
      for (unsigned i = 0; i < strip.getSegmentsNum(); i++) {
        Segment &seg = strip.getSegment(i);
        if (seg.isActive()) seg.colors[0] = BLACK;
      }

This still goes to orange but instead of going full color immediately, it transitions from black or 'fades in' if fade is enabled. Looks nicer IMHO and also gets rid of the 'orange flash' if both options turnOnAtBoot as well as bootup preset are enabled.

This comment has been minimized.

Sign in to view

Copy link
@blazoncek

blazoncek Nov 17, 2024

Author Collaborator

Unfortunately I do not understand where the problem (you are describing) is.

If turnOnAtBoot is true then Orange should light up, using default transition time.
If it is false, then, if the boot preset is set, all segments have to be cleared immediately without transition (because constructor sets them Orange).

If you have both set (turnOnAtBoot=true & bootPreset>0) then strip should light up Orange using default transition (caused by turnOnAtBoot) but also boot preset should be applied asynchronously. This (preset) happens in next loop iteration. We do not care what's inside boot preset (it can be anything, even not WLED state related).

Following this logic, the code is ok.

This comment has been minimized.

Sign in to view

Copy link
@DedeHai

DedeHai Nov 17, 2024

Collaborator

your logic is fine, although I think the orange does not follow transition time, it just goes full bright, then transition starts.
Fixing the "orange flash" if turnOnAtBoot=true is more of a user-experience fix than anything else. It's not inherently clear that enabling this option is the cause, at least to me when reading this option in the settings I'd think "yes, this is what I want, but also I want to apply a preset" when not knowing the inner workings of presets (and that they already have the "on" option set by default). Its a bit convoluted and using the suggested change would just make it work regardless, which I think would be an improvement in user-friendliness rather than an fix (as it is not broken).

This comment has been minimized.

Sign in to view

Copy link
@blazoncek

blazoncek Nov 17, 2024

Author Collaborator

Do whatever you choose to do. IMO the current implementation follows the same principle that was there in 0.11 (and earlier) and is the correct one. I would not change it.

Perhaps it needs to be better described in KB or in settings alone.

This comment has been minimized.

Sign in to view

Copy link
@DedeHai

DedeHai Nov 22, 2024

Collaborator

I found another "orange weirdness": when setting up two segments, the second segment still starts up orange, the first one starts black, even if turnOnAtBoot=false. It is quite inconsistent behaviour with no real logic behind it IMHO.
I could not find the part of the code that initilizes the orange color, can you give me a hint on where to check that?
fadeTransition = false; // ignore transitions temporarily
strip.setColor(0, BLACK); // set all segments black
fadeTransition = oldTransition; // restore transitions
col[0] = col[1] = col[2] = col[3] = 0; // needed for colorUpdated()
}
briLast = briS; bri = 0;
strip.fill(BLACK);
strip.show();
Expand Down
3 changes: 1 addition & 2 deletions wled00/wled.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

// version code in format yymmddb (b = daily build)
#define VERSION 2401130
#define VERSION 2401140

//uncomment this if you have a "my_config.h" file you'd like to use
//#define WLED_USE_MY_CONFIG
Expand Down Expand Up @@ -364,7 +364,6 @@ WLED_GLOBAL char serverDescription[33] _INIT("WLED"); // Name of module - use d
#else
WLED_GLOBAL char serverDescription[33] _INIT(SERVERNAME); // use predefined name
#endif
//WLED_GLOBAL bool syncToggleReceive _INIT(false); // UIs which only have a single button for sync should toggle send+receive if this is true, only send otherwise
WLED_GLOBAL bool simplifiedUI _INIT(false); // enable simplified UI
WLED_GLOBAL byte cacheInvalidate _INIT(0); // used to invalidate browser cache

Expand Down

0 comments on commit 21d21ea

Please sign in to comment.