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

Add support for Pocketbook Color Lux #4212

Merged
merged 6 commits into from Sep 30, 2018

Conversation

Projects
None yet
4 participants
@thomasrebele
Contributor

thomasrebele commented Sep 9, 2018

This solves issue #4193.

hasColorScreen = yes,
has3BytesWideFrameBuffer = yes,
isAlwaysPortrait = no,
display_dpi = 100,

This comment has been minimized.

@NiLuJe

NiLuJe Sep 9, 2018

Member

You can probably leave it out, fb:getDPI will fall back to 160, which should be roughly okay (it's what we do on other 600x800 devices) ;).

@@ -46,6 +46,9 @@ local Device = {
isAlwaysPortrait = no,
-- needs full screen refresh when resumed from screensaver?
needsScreenRefreshAfterResume = yes,
-- framebuffer reports 8bit per pixel, but is actually 24bit per pixel

This comment has been minimized.

@NiLuJe

NiLuJe Sep 9, 2018

Member

I'd add a link to your original issue in the comment, in case that confuses someone down the road ;).

This comment has been minimized.

@NiLuJe

NiLuJe Sep 9, 2018

Member

Possibly even saying right then and there that it's a Triton/color eInk screen

@NiLuJe

This comment has been minimized.

Member

NiLuJe commented Sep 9, 2018

Besides those two minor comments, LGTM ;).

@@ -46,6 +46,9 @@ local Device = {
isAlwaysPortrait = no,
-- needs full screen refresh when resumed from screensaver?
needsScreenRefreshAfterResume = yes,
-- framebuffer reports 8bit per pixel, but is actually 24bit per pixel
-- (refresh is still based on bytes)

This comment has been minimized.

@poire-z

poire-z Sep 9, 2018

Contributor

Looks like this comment (the way it is phrased) is aimed at device/pocketbook/device.lua.
Here should be a more generic comment, or that one just prefixed with "set to yes on devices where..."

@Frenzie

This comment has been minimized.

Member

Frenzie commented Sep 29, 2018

Pinging @thomasrebele about the small comments.

@thomasrebele

This comment has been minimized.

Contributor

thomasrebele commented Sep 30, 2018

thanks for the feedback, and sorry for the late reply! The new commits should fix it

@Frenzie

This comment has been minimized.

Member

Frenzie commented Sep 30, 2018

@thomasrebele Thanks! Could you please add a base bump to this PR to wrap it all up?

thomasrebele added a commit to thomasrebele/koreader that referenced this pull request Sep 30, 2018

@thomasrebele

This comment has been minimized.

Contributor

thomasrebele commented Sep 30, 2018

I hope I bumped it correctly. Thanks again for all the help. Koreader is a great project.

@Frenzie

This comment has been minimized.

Member

Frenzie commented Sep 30, 2018

Darn, it seems to have caused some kind of conflict. You could try to fix it with rebasing, e.g., using the guide from http://koreader.rocks/doc/topics/Collaborating.html or I can also take care of it, albeit not right at this very moment. (Hence why I asked you to bump base. :-) )

@Frenzie Frenzie force-pushed the thomasrebele:master branch from bfa3877 to c3c2770 Sep 30, 2018

@Frenzie Frenzie merged commit 1e275fd into koreader:master Sep 30, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@thomasrebele

This comment has been minimized.

Contributor

thomasrebele commented Sep 30, 2018

thanks for fixing it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment