search

Aircoookie / WLED

Control WS2812B and many more types of digital RGB LEDs with an ESP8266 or ESP32 over WiFi!
https://kno.wled.ge
MIT License
14.83k stars 3.2k forks source link

Incorrect localTime when enabling countdown #4197

Open HackerShurf opened 1 week ago

HackerShurf commented 1 week ago

What happened?

When I set the feature to turn on the countdown timer it sets the date to some time in the 1970s

To Reproduce Bug

Turn on the NTP set to scrolling text let it establish the correct time and date Turn on countdown timer option with any date in 2025 I was using august 25, 2025

Expected Behavior

To show a count down timer to the date

Install Method

Binary from WLED.me

What version of WLED?

0.15.0-b6

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

No response

Anything else?

No response

Code of Conduct

blazoncek commented 1 week ago

What makes you think Scrolling text will display countdown?

HackerShurf commented 1 week ago

What makes you think Scrolling text will display countdown?

It used to in previous versions I've had other people test. Looks like it has not worked for a few versions. Unless you have instructions on how to make the countdown timer work.

blazoncek commented 1 week ago

I am the author of Scrolling text effect and I know I never implemented "coundown" of any kind. 😄

HackerShurf commented 1 week ago

SmartSelect_20241017_002844 Checking the box for countdown used to mak it count down to that date and time unless I'm mistaken.

HackerShurf commented 1 week ago

I am the author of Scrolling text effect and I know I never implemented "coundown" of any kind. 😄

But if you developed it then it must be a different item or setup. But I guess it needs to be a feature request then.

dosipod commented 1 week ago

@HackerShurf It is actually working if you play with the date ( the calculation is wrong though ) but scrolling text will do countdown , feature it is but it is a bug

https://github.com/user-attachments/assets/45dd923e-85ba-4b96-b9e1-fef07006018c

softhack007 commented 1 week ago

Checking the box for countdown used to make it count down to that date and time unless I'm mistaken.

"countdown mode" is actually a feature of the "analog clock overlay". This is an overlay effect for 1D rings. So it has no relation with 2D scrolling text.

DedeHai commented 1 week ago

Checking the box for countdown used to make it count down to that date and time unless I'm mistaken.

"countdown mode" is actually a feature of the "analog clock overlay". This is an overlay effect for 1D rings. So it has no relation with 2D scrolling text.

then this is a bug? if its just for analog clock overlay it should not change the behaviour of scrolling text IMHO. A good fix would be to allow it in scrolling text instead of braking it.

softhack007 commented 1 week ago

@DedeHai I wouldn't say it's a bug, but a feature that could be added to scrolling text. Right now I have no idea which "tag" could be used - maybe append a "Z" so that "#MMDD" shows month and date, but "#MMDDZ" shows countdown months and days.

I wouldn't change the display automatically based on a checkbox in time settings - this relation would be hard to understand for users who are running their digital clocks with WLED.

DedeHai commented 1 week ago

I wouldn't change the display automatically based on a checkbox in time settings - this relation would be hard to understand for users who are running their digital clocks with WLED.

isn't that what it is currently doing i.e. what the issue was opened for?

blazoncek commented 1 week ago

Scrolling text is displaying localTime as it is calcualted by ntp.cpp. If there is an error it is in the nttp.cpp not Scrolling text.

dosipod commented 1 week ago

With my none existing experience I have gone over ntp.cpp and that is not a code but all math . Since that is the case only math , with the correction by adding around 55 years (give or take for now ) to the target date I could get the date I want to countdown on the scrolling text from ui

Please do not close this , marked it any way you like and if code correction is not possible then I will plot multiple dates and will provide an accurate value to use in ui to get scrolling text to do the count down at least for date . If you see that as a silly way of doing it then I agree and please do not shot the messenger as myself and DedeHai do not use it and this is the first time I did just to verify for the OP , Cheers

5chubrakete commented 1 week ago

After a few tests the countdown TIME seems to work fine. The DATE is offset by 1.1.1970. since: if (countdownMode) localTime = n - countdownTime + utcOffsetSecs; So maybe we can subtract 1, 1 and 1970 before displaying DD, MO, YY, YYYY and DATE in this mode. Am-time and string representations of month or days make no sense though. So display "-" or ?

I also found that during effect transitions the localtime is displayed instead of the countdown time. (flickering)

HackerShurf commented 1 week ago

Checking the box for countdown used to make it count down to that date and time unless I'm mistaken.

"countdown mode" is actually a feature of the "analog clock overlay". This is an overlay effect for 1D rings. So it has no relation with 2D scrolling text.

then this is a bug? if its just for analog clock overlay it should not change the behaviour of scrolling text IMHO. A good fix would be to allow it in scrolling text instead of braking it.

This is my thinking as well since it globally effects the time/other effects.

DedeHai commented 1 week ago

I see two solutions: a) fix ntp and make the scrolling-text FX display the countdown-time if this option is activated b) fix ntp and handle countdown completely separate, add special strings to make it display in clock mode

personally I prefer option a). This "bug" of countdown mode changing the global time has been present for a long time and apparently there were no requests to fix it up to now. It will not break any existing setups and makes it easy to display a countdown. If special Strings (or just a "C" in front for countdown like "C#HH" for hours or "C#SS" for seconds) is used, then having the checkmark for countdown mode makes little sense to me, it could just always be enabled but that would break the analog clock. The only downside of option a) would be that displaying a countdown AND the time on different segments is not possible. Since not many seem to use this option and it was not possible up to this point I see no problem with that.

5chubrakete commented 5 days ago

It might also make sense to add (or chage the meaning of) some tags. e.g. having #DDDD display the total days so target instead of days to end of month requiring the display of month and years in addition.

I'dd prefer a) as well.

dosipod commented 2 days ago

After a few tests the countdown TIME seems to work fine. The DATE is offset by 1.1.1970. since: if (countdownMode) localTime = n - countdownTime + utcOffsetSecs; So maybe we can subtract 1, 1 and 1970 before displaying DD, MO, YY, YYYY and DATE in this mode. Am-time and string representations of month or days make no sense though. So display "-" or ?

I also found that during effect transitions the localtime is displayed instead of the countdown time. (flickering) @5chubrakete
I did not see this and yeah i have tested now as below and it is the offset by 1.1.1970 , image Note: I added one day using that site to the target date of Oct 25,2024 but the math checks out