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

Nook Glowlight 4/4e/4 plus EInk refresh support (full-only) #460

Merged
merged 8 commits into from Jan 11, 2024

Conversation

Codereamp
Copy link
Contributor

@Codereamp Codereamp commented Jan 2, 2024

This commit overrides the previously closed with the following changes

  • NativeSurfaceView is brought to top in order for Nook Glowlight 4 refresh to work. This change logically affects other controllers that report needsView, but the change should be compatible
  • NGL4EPDController.kt module was introduced (mostly in the previous PR) with all registering routines. In this PR made several changes based on the previous tests and discussions.

The changes were tested on Nook Glowlight 4e (debug compilation to koreader-android-arm-debug-v2023.10-82-g5d2a44106_2023-12-29.apk)


This change is Reviewable

@Codereamp
Copy link
Contributor Author

@pazos, I hope I did the new PR more or less correctly. Please, let me know if anything was wrong.

@pazos
Copy link
Member

pazos commented Jan 2, 2024

@pazos, I hope I did the new PR more or less correctly. Please, let me know if anything was wrong.

It looks quite good.

Just replace NGL4 for something a bit more specific (NOOK_GL4 is fine) and please leave the list sorted.

Also, this is going to be merged after the next release because a few things changed this month and is going to be way easier to triage if something went wrong with those.

@Codereamp
Copy link
Contributor Author

Just replace NGL4 for something a bit more specific (NOOK_GL4 is fine) and please leave the list sorted.

Ok, I assume that you're talking also about the class name and the file name, in this case I suggest NookGL4, without underscore, I think it would fit more nicely as the prefix.

@pazos
Copy link
Member

pazos commented Jan 3, 2024

Just replace NGL4 for something a bit more specific (NOOK_GL4 is fine) and please leave the list sorted.

Ok, I assume that you're talking also about the class name and the file name, in this case I suggest NookGL4, without underscore, I think it would fit more nicely as the prefix.

The class is fine (and its file name is ok as long as it matches the name of the class). I was talking about the Device name on DeviceInfo.

@Codereamp
Copy link
Contributor Author

@pazos, probably both wishes now addressed (NOOK_GL4 and sorted order). I see that app.codacy.com is super pedantic with its analysis :) let me know whether I should also adjust for its issues.

@Codereamp
Copy link
Contributor Author

A minor question regarding compiling. During the whole process I wanted to avoid extra actions with file packaging during compiling (especially it was time saver when there were syntax errors) and in the full tree of the koreader project made a shell script in .. luajit-launcher sub-folder with the following text (multiply dots collapses the actual path to ndk)

export ANDROID_NDK_HOME=/home/........ /android-ndk-r23c
export ANDROID_NDK_ROOT=/home/......../android-ndk-r23c
make debug

With it I was able to compile much faster (sure when the primary compilation once made complete). There were drawbacks like different apk location and different name, but they were minor.
Is this approach ok or somehow there already exist scripts for such faster compilation?

@pazos
Copy link
Member

pazos commented Jan 9, 2024

@pazos, probably both wishes now addressed (NOOK_GL4 and sorted order). I see that app.codacy.com is super pedantic with its analysis :) let me know whether I should also adjust for its issues.

Yes, please

Copy link
Member

@pazos pazos left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

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.

None yet

2 participants