Skip to content
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

Added wipe in and wipe out effects #4358

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Rinszky
Copy link

@Rinszky Rinszky commented Dec 7, 2024

This is a wipe in and wipe out effect to turn on and off the led smoothly with slide.
It was very important for me, and maybe it'll be beneficial for others, I saw a lot of topic about this how to turn on and off like this...
Before this, it was possible only with playlists, but the timing was problematic.

example1
example2
example3
example4

So Maintainer, please decide if you need this effect or not!
Should I change something to be better?
If you know a better name for this effect, let me know and I'll rename it!

Thank you for this cool product!!!

@DedeHai
Copy link
Collaborator

DedeHai commented Dec 7, 2024

thanks for contributing!
however, this is a whole lot of code for something that can be done already, although a bit convoluted.
if you want to solve it in an elegant way:
how about adding two checkmarks to the existing wipe effect, one to 'wipe on' and one to 'wipe off'?

@Rinszky
Copy link
Author

Rinszky commented Dec 7, 2024

thanks for contributing! however, this is a whole lot of code for something that can be done already, although a bit convoluted. if you want to solve it in an elegant way: how about adding two checkmarks to the existing wipe effect, one to 'wipe on' and one to 'wipe off'?

Not a bad idea, I'm totally new in this world, and it was easier for me to create a new, instead of modifying and maybe breaking something... If you know how to do that, I would appreciate for your help!

@blazoncek
Copy link
Collaborator

blazoncek commented Dec 7, 2024

FYI Effect blending styles PR (#3877) adds universal ability to "wipe in" and "wipe out" any effect among other styles.

EDIT: current PR is #4158

@dosipod
Copy link
Contributor

dosipod commented Dec 7, 2024

There seems to be a lot of other duplicate effects and some of them are useless so I see why not add another wipe effect that is as @Rinszky listed have been requested a lot and doing it with playlist is bad .

@dosipod
Copy link
Contributor

dosipod commented Dec 8, 2024

@Rinszky Now I see it I am also not sure why add 5 effects when only two can do .
Also something seems to be wrong with the wipe in ( or logic as you did not show your own demo after your change)
Once you select a wipe ( try with wipe in fast ) then it will light all the leds on peek and fixture but on the fixture
the leds will go off after a while and will come back again .Peek will stay on . So seems like a bug

The expected behavior we had in mind and done with playlist is one effect to lights all the leds one by one and then all the leds should stay on . The 2nd effect will turn off the leds one by one and then keep all of them off

image

@DedeHai
Copy link
Collaborator

DedeHai commented Dec 9, 2024

@dosipod can you confirm the new blending styles are suited to solve this? if so, there is not need to change the wipe effect as the blend-in is much more versatile.

@dosipod
Copy link
Contributor

dosipod commented Dec 9, 2024

@DedeHai I never tested that but if blaz said so then must be

@dosipod
Copy link
Contributor

dosipod commented Dec 9, 2024

I think I was traveling at the time but there is even video here of wipe in but not named so it was not clear to others what he was referencing https://discord.com/channels/473448917040758787/779395228624617512/1258428188527296614

// All LEDs are off, reset
SEGENV.aux1 = 0;
strip.setBrightness(0);
return 65535;
Copy link
Collaborator

@softhack007 softhack007 Dec 11, 2024

Choose a reason for hiding this comment

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

this will not cause a "forever black" - the effect will run again in 65535 millis = 65 seconds. Also manipulating strip properties in an effect (at segment level) is a very fragile thing and should be avoided.


uint16_t duration = 1000 - (SEGMENT.speed * 4); // Adjust speed (1-255) to duration (1000ms to 20ms)
uint16_t stepDuration = duration / SEGLEN;
uint32_t currentTime = millis();
Copy link
Collaborator

Choose a reason for hiding this comment

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

use strip.now instead of millis()


uint16_t duration = 1000 - (SEGMENT.speed * 4); // Adjust speed (1-255) to duration (1000ms to 20ms)
uint16_t stepDuration = duration / SEGLEN;
uint32_t currentTime = millis();
Copy link
Collaborator

Choose a reason for hiding this comment

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

see previous comment

@blazoncek
Copy link
Collaborator

Everyone, please see blending styles before continuing discussion on this PR.

@softhack007
Copy link
Collaborator

softhack007 commented Dec 11, 2024

@Rinszky If I understand your code correctly, your implementation of "wipe" is always filling / clearing the segment with a single color, right? This means we still have no good solution for animated effects.

To preserve some space in the effect table, it might be better to add "wiping" options (checkboxes) and wipe speed control (sliders) to the "Solid" effect. It would be possible to still have a "wipe-off" by using several presets for "wipe in" "normal light", "wipe out" an "lights out"

@softhack007 softhack007 added the rebase needed This PR needs to be re-based to the current development branch label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rebase needed This PR needs to be re-based to the current development branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants