Skip to content

Conversation

@Riksu9000
Copy link
Contributor

lv_color_hex can't be evaluated at compile time, but LV_COLOR_MAKE can.

I feel like there must be a better approach that I'm missing. Let me know if you have ideas.

@Riksu9000 Riksu9000 added the maintenance Background work label Jan 26, 2023
@Riksu9000 Riksu9000 requested a review from a team January 26, 2023 10:02
@github-actions
Copy link

github-actions bot commented Jan 26, 2023

Build size and comparison to develop:

Section Size Difference
text 406752B -400B
data 940B 0B
bss 53568B 0B

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

I like the use of variable names for colors instead of hard coding the color values in the setting code. Letting the compiler do the heavy lifting, and then the PineTime can relax a bit is a very welcome change

I couldn't think of anything better, LGTM

@lman0
Copy link

lman0 commented Feb 26, 2023

@Riksu9000 could you check if your PR , don't impact / increase the ram use of infinitime (reduce free mem) on pinetime?
Because
I remember in the past that a PR/proposition similar in purpose was done but in the end was rejected because live memory usage in pinetime increased a lot.
(And in contrast the PR/proposition was removing firmware memory usage too).

If your PR reduce usage (or don't impact) memory ram
And reduce firmware , this would indeed great.

@JF002
Copy link
Collaborator

JF002 commented Feb 26, 2023

@lman0 From what I can tell, this PR reduces the flash usage (by removing code) and also RAM usage : it removes the struct WatchFaceInfineat::InfineatColors which was stored in flash and allocated in ram at runtime. It also reduce the stack usage (in ram) by not calling a function.
So I don't think it'll have any negative side effect.

lv_color_hex can't be evaluated at compile time, but LV_COLOR_MAKE can.
@Riksu9000 Riksu9000 force-pushed the infineat-more-optimization-rik branch from 8dfdb9a to b01f5a8 Compare February 26, 2023 17:47
@Riksu9000 Riksu9000 merged commit ce2277c into develop Feb 26, 2023
@Riksu9000 Riksu9000 deleted the infineat-more-optimization-rik branch February 26, 2023 17:53
@JF002 JF002 added this to the 1.12.0 milestone Apr 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Background work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants