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

Fix some screen refresh effects #10306

Merged
merged 5 commits into from
Apr 30, 2023
Merged

Fix some screen refresh effects #10306

merged 5 commits into from
Apr 30, 2023

Conversation

zwim
Copy link
Contributor

@zwim zwim commented Apr 7, 2023

This fixes the strange screen refresh (screen is split into four parts, which get refreshed from bottom left clockwise) when toggling Fiwi on (on a Sage).


This change is Reviewable

@zwim zwim added this to the 2023.04 milestone Apr 7, 2023
@zwim zwim marked this pull request as ready for review April 7, 2023 22:10
@NiLuJe
Copy link
Member

NiLuJe commented Apr 8, 2023

Hum, what does this have to do with toggling Wi-Fi, since this is the standby scheduler?

(If it "fixes" anything, this is just because of the added delay this incurs, which is why I'm not a fan of this approach; on the other hand, if it actually does somethign because of a mismatch between getWifiState and isWifiOn, I'd much, much, much rather fix NetworkManager instead ;)).

@NiLuJe
Copy link
Member

NiLuJe commented Apr 10, 2023

I wouldn't mind a comment on that one, too; but I'm fine with this approach (even if I'm still not quite sure what's happenign here, and if it isn't very very very timing-specific to your setup...).

@zwim
Copy link
Contributor Author

zwim commented Apr 10, 2023

Let us wait with this one too.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 10, 2023

I assume the idea is to prevent entering suspend right when we stop blocking from the WiFi stuff (which would probably fail, even if there were no weird display stuff entering the mix), which certainly makes sense on paper ;).

@NiLuJe
Copy link
Member

NiLuJe commented Apr 10, 2023

This opens up another can of worms though: should we take the plunge and hope that our onNetwork(Dis)Connected events are reliable, and simply stop scheduling standby on connected, and resume on disconnected? (Instead of checking that inside the scheduler).

(Not necessarily recommending that, mind you, just a brain dump ;)).

@zwim
Copy link
Contributor Author

zwim commented Apr 12, 2023

This commit fixes my problems with the strange screen refresh on turning Wifi on.

The root cause is, that toggleWifiOn in NetworkManager might take long (up to 12 seconds here).

Additionally, this solution quasi disables the standby when Wifi is on (by setting a laaarge time for the next sleep; then wie don't have to bother with allow/prevent-standby symmetry). Which simplifies the program flow with Wifi on.

@NiLuJe
Copy link
Member

NiLuJe commented Apr 12, 2023

Makes sense ;o).

Now we just have to hope that my Network* events are reliable ;o).

@zwim
Copy link
Contributor Author

zwim commented Apr 17, 2023

Ready to merge?

Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @zwim)


plugins/autosuspend.koplugin/main.lua line 682 at r4 (raw file):

end

--[[ -- not neccessary right now

nit: necessary

@Frenzie Frenzie modified the milestones: 2023.04, 2023.05 Apr 19, 2023
Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @zwim)

@zwim
Copy link
Contributor Author

zwim commented Apr 20, 2023

lgtm

@zwim zwim merged commit 662bd65 into koreader:master Apr 30, 2023
@zwim zwim deleted the fixStrangeScreenReferschesPart1 branch April 30, 2023 20:15
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