Skip to content

Conversation

@5chubrakete
Copy link
Contributor

Added some date and time formatting options to scrolling text effect.
Updated to nonbreaking change and auto uppercasing according to review.

This is a rebased pull request for #4192

Updated to nonbreaking change and auto uppercasing according to review.
@DedeHai
Copy link
Collaborator

DedeHai commented Oct 17, 2024

@5chubrakete since you are working on this part of the code: can you check if #4197 is easy to fix?

@5chubrakete
Copy link
Contributor Author

5chubrakete commented Oct 17, 2024

@5chubrakete since you are working on this part of the code: can you check if #4197 is easy to fix?

okay

else if (!strncmp_P(text,PSTR("#HHMM"),5)) sprintf_P(text, zero?PSTR("%02d:%02d") :PSTR("%d:%02d"), AmPmHour, minute(localTime));
else if (!strncmp_P(text,PSTR("#HH"),3)) sprintf_P(text, zero?PSTR("%02d") :PSTR("%d"), AmPmHour);
else if (!strncmp_P(text,PSTR("#MM"),3)) sprintf_P(text, zero?PSTR("%02d") :PSTR("%d"), minute(localTime));
else if (!strncmp_P(text,PSTR("#HH"),3)) sprintf (text, zero? ("%02d") : ("%d"), AmPmHour);
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not PSTR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

repeated many times but using only 5 bytes. saves flash at small expense of RAM

Copy link
Collaborator

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

Approved as is.

@blazoncek
Copy link
Collaborator

@5chubrakete please add entries for added templates to WLED-Docs. It can be merged after that PR is done.

@5chubrakete
Copy link
Contributor Author

done. WLED-Doc #231

@netmindz netmindz changed the base branch from 0_15 to main December 16, 2024 13:30
@DedeHai DedeHai merged commit 4951be6 into wled:main Jan 20, 2025
20 checks passed
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.

4 participants