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.

Sign up for GitHub

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

Jump to bottom

Tolino Shine 4 support #457

Closed
wants to merge 12 commits into from
Closed

Tolino Shine 4 support #457

wants to merge 12 commits into from

Conversation

mergen3107
Copy link
Contributor

@mergen3107 mergen3107 commented Dec 27, 2023 •
edited by Frenzie
Loading

C.f., #410

This change is Reviewable

@mergen3107
Copy link
Contributor Author

There's no lua code to tweak here. What needs to be done is

  1. create a device definition for the TOLINO_SHINE_4.
  2. make sure the device isn't catched in the TOLINO superdevice.
  3. tweak TolinoRootController to include the sysfs path (lines highlighted on my previous comment). Put the new case at the top of the if/else branch, so it fallbacks to the same branch as it does now)
  4. assign the proper Lights and EPD drivers to the new device TOLINO_SHINE_4.
  5. Build a test APK.

@pazos
I attempted steps 1-4. Please let me know if these are OK, so I can go an build the apk.

@mergen3107
Copy link
Contributor Author

2. make sure the device isn't catched in the TOLINO superdevice.
I wasn't really sure about this one, so for the EPD Controller there are now TOLINO and TOLINO_SHINE_4 with the latter being a first outstanding Tolino device.

@pazos
Copy link
Member

pazos commented Dec 27, 2023

I attempted steps 1-4. Please let me know if these are OK, so I can go an build the apk.

Thanks, they're ok for testing. I'm not sure if the EPD impl is going to be worse than no implementation at all, but lets ask for feedback first.

@hugleo
Copy link
Contributor

hugleo commented Dec 27, 2023

I think that will do it, you need build a new apk

@mergen3107
Copy link
Contributor Author

@hugleo Thanks for the review!
I posted the new apk here: #410 (comment)
I've been updating it on Yandex.Disk, but the link stays the same.

pazos
pazos reviewed Dec 28, 2023
View reviewed changes
app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt Outdated
@@ -93,7 +93,8 @@ object DeviceInfo {
SONY_CP1,
SONY_RP1,
TAGUS_GEA,
TOLINO
TOLINO,
TOLINO_SHINE4
Copy link
Member

Choose a reason for hiding this comment

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

delete me

pazos
pazos reviewed Dec 28, 2023
View reviewed changes
app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt Outdated
@@ -645,6 +654,7 @@ object DeviceInfo {
deviceMap[EinkDevice.SONY_RP1] = SONY_RP1
deviceMap[EinkDevice.TAGUS_GEA] = TAGUS_GEA
deviceMap[EinkDevice.TOLINO] = TOLINO
deviceMap[EinkDevice.TOLINO_SHINE4] = TOLINO_SHINE4
Copy link
Member

Choose a reason for hiding this comment

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

delete me

pazos
pazos reviewed Dec 28, 2023
View reviewed changes
app/src/main/java/org/koreader/launcher/device/EPDFactory.kt Outdated
@@ -58,7 +58,8 @@ object EPDFactory {
DeviceInfo.EinkDevice.CREMA_CARTA_G,
DeviceInfo.EinkDevice.HANVON_960,
DeviceInfo.EinkDevice.RIDI_PAPER_3,
DeviceInfo.EinkDevice.TOLINO -> {
DeviceInfo.EinkDevice.TOLINO,
DeviceInfo.EinkDevice.TOLINO_SHINE4-> {
Copy link
Member

Choose a reason for hiding this comment

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

delete me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean, to remove TOLINO_SHINE4 as a separate entry in these three comments you made?

Copy link
Member

Choose a reason for hiding this comment

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

I mean, revert all EINK stuff since it doesn't apply

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, OK! Thanks!
I think I understood your "make sure it doesn't get caught by Tolino super device" wrong and applied everywhere :D

Copy link
Member

Choose a reason for hiding this comment

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

yay, the superdevice for Lights seems to be TOLINO_EPOS

@pazos pazos mentioned this pull request Jan 3, 2024
Closed
@pazos pazos closed this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pazos pazos pazos left review comments

@hugleo hugleo hugleo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mergen3107 @pazos @hugleo