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

Ignore old GoToRunning messages #2165

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

mark9064
Copy link
Member

Fixes the recent display inversion issue (#2160)

In short DisplayApp could process a stale GoToRunning message with a GoToSleep also queued up after it, and do one LVGL iteration when the SPI is off

Verified locally by adding guards around SpiMaster that reset the device immediately if SPI is used while asleep (easiest way without a devkit)

I'm pretty confident DisplayApp state handling is sound after this change. But it's also quite overcomplicated now and I'd like to simplify it, but right before a release probably isn't a good time so I'll propose that later

Copy link

Build size and comparison to main:

Section Size Difference
text 374592B 0B
data 948B 0B
bss 63504B 0B

@mark9064 mark9064 added this to the 1.15.0 milestone Nov 11, 2024
@JF002
Copy link
Collaborator

JF002 commented Nov 17, 2024

But it's also quite overcomplicated now and I'd like to simplify it, but right before a release probably isn't a good time so I'll propose that later

Yeah, I would like to simplify those states and improve the design so that those data race cannot occur anymore. Or maybe add unit test so we can ensure we won't break the sleep/wakeup feature again :)

Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

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

Works well from my testing!

@FintasticMan FintasticMan merged commit 8aefa3b into InfiniTimeOrg:main Nov 17, 2024
7 checks passed
@mark9064 mark9064 mentioned this pull request Nov 17, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants