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

Add pulse mode #28

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

Add pulse mode #28

wants to merge 1 commit into from

Conversation

r41d
Copy link
Contributor

@r41d r41d commented Aug 31, 2017

Pulse mode transitions forth and back between black and the current color.
Looking for remarks before possibly adapting it for the other 2 sketches.


if (pulse) {
if ( millis() % pulseTickEveryXms == 0) {
pulseCurrentBrightness += pulseDir;
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe I'm misunderstanding something here, but wouldn't this only trigger if we manage to hit this if in a matching millisecond? I don't think we can guarantee that the loop will happen regularly enough to trust that.

I'll give this a more thorough review in the next few days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.. my tests (on a WeMos D1 mini with 80MHz) showed that without the delay(1) the pulsing was way too fast, and with it, one cycle was around 5 seconds (like specified in the config) and the animation looked nice and smooth.
Will look into a possibility to refactor this.

@corbanmailloux
Copy link
Owner

Can you clarify what exactly the pulse mode does?
If it's just toggling between the already set color and black, I think you could achieve the same thing by using the existing flash effect and uncommenting this line. I'm all for adding more effects, I just want to make sure that they make sense.

@depuits
Copy link

depuits commented Dec 13, 2017

the difference between this pulse and the flash effect seems to be that the flash only flashes a specified amount of times while this pulse would keep on flashing until stopped.

This could be implemented in the flash effect using 0 as flash length or by adding another option to start it indefinitely. I'm not sure if and how you could pass effect parameters in HA however.

@r41d
Copy link
Contributor Author

r41d commented Dec 17, 2017

Flash does 0-255-0-255-0 in a specified interval
Pulse does 0-1-2-3-4-5-...-255-254-253-252-...-0

@depuits
Copy link

depuits commented Dec 17, 2017

So do we also need a flash effect that pulses indefinitely?

@r41d
Copy link
Contributor Author

r41d commented Aug 8, 2018

TODO: adapt to the new unified sketch

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

Successfully merging this pull request may close these issues.

3 participants