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

Add support for warmth control on Tolino Shine 4 #410

Closed
robintschoetschel opened this issue Mar 8, 2023 · 48 comments
Closed

Add support for warmth control on Tolino Shine 4 #410

robintschoetschel opened this issue Mar 8, 2023 · 48 comments

Comments

@robintschoetschel
Copy link

I saw you managed to get illumination color/temperature support working on the Tolino Shine 3: #400

As far as I can tell it could be done by adding a device tag here: https://github.com/koreader/android-luajit-launcher/blob/17901da600ec85b02d3f7dc0bd0ce0fc55a6aaec/app/src/main/java/org/koreader/launcher/device/DeviceInfo.kt

Let me know if I can help. I have the device.

Thank you!
And many additional thanks for an awesome app!

@pazos
Copy link
Member

pazos commented Mar 9, 2023

Please follow instructions in koreader/koreader#8482

@robintschoetschel
Copy link
Author

Thanks @pazos, for pointing me to that issue.

I tried the steps there but unfortunately couldn't get the warmth control to work with any of the drivers.

I had to run the ActivityTest using ADB, since the menu item wasn't available, so it seems like KOReader doesn't recognize my device as Android?

The tolino drivers (NTK and "Root") manipulate brightness well, but no warmth.

Here is my device info, and I've attached the log file test.log

Device info:
Manufacturer: rakuten kobo inc.
Brand: rakutenkobo
Model: tolino shine 4
Device: tolino
Product: tolino
Hardware: sun8iw15p1
Platform: virgo

Additional device info:
Android Version 8.1.0
Security patch level July 5, 2018
Processor type: Allwinner B300 (Quadcore, Cortex-A7 (arm))

Happy to do whatever I can to try and make this work. Not too good with Java or C though. But easy edits I can try myself. Testing in any case.

@robintschoetschel
Copy link
Author

@pazos I would be happy to have another go at this, since I love the little device and koreader, but the lack of full dim (koreader/koreader#11013), and the missing warmth control are a bit annoying.

From the log above, and further testing on a rooted device, the crux seems to lie in

03-10 10:00:24.909 W/System.err( 5633): java.io.FileNotFoundException: /sys/class/backlight/tlc5947_bl/color (No such file or directory)
(the error is the same on a rooted shine 4)

Let me know what I can do to help here. Happy to put in some time coding and testing

@NiLuJe
Copy link
Member

NiLuJe commented Dec 11, 2023

I can never keep track of the Tolino boards, but it looks like a Clara 2E, so try with /sys/class/leds/aw99703-bl_FL1/color instead (or wherever that ends up being on Android).

Basically, check the Kobo code, find the closest matching board, and poke at the sysfs until you find the right one. NTX thankfully hasn't used that many different PWM controllers, so it shouldn't take long.

@robintschoetschel
Copy link
Author

I'll give that a try. How would I best test the different files? Do I need to rebuild Koreader every time I want to do that? Or is there some way for me to hotfix using a config?

@NiLuJe
Copy link
Member

NiLuJe commented Dec 13, 2023

Err, just poke at it in a shell to begin with, KOReader doesn't have to enter the equation until you've got the right paths ;).

At which point, you also don't need to rebuild anything, it's Lua, since you're root, you can just replace the affected files in place, wherever they are on Android (/data/something, IIRC).

@robintschoetschel
Copy link
Author

Hm, I'm finding the right file (it is indeed sys/class/leds/aw99703-bl_FL1/color) and can adjust warmth by changing the value in it — so I should be able to fix the RootController for this device. I can't seem to locate the LUA file where the reference is stored for testing, so I think I'll write a patch and try to rebuild — that would also allow others to use the functionality

I would like to avoid breaking things for users that do not have a rooted device, so I think I should add a switch for that somewhere in the lightsfactory — do you have a way for me to easily test for root?

Finally, I was also testing the ntx controller, and it fails because of
/dev/ntx_io: ioctl failed: permission denied
when trying to use ioctl — could it be that the warmth_id is different on my device and I could find that out somehow?

@pazos
Copy link
Member

pazos commented Dec 16, 2023

I would like to avoid breaking things for users that do not have a rooted device, so I think I should add a switch for that somewhere in the lightsfactory — do you have a way for me to easily test for root?

Not needed. The class is called TolinoRoot for a reason :)

@NiLuJe
Copy link
Member

NiLuJe commented Dec 16, 2023

when trying to use ioctl — could it be that the warmth_id is different on my device and I could find that out somehow?

That ID seems to be "stable" (as CM_FL_LM3630_TABLE), but I've never seen it implemented on Kobo kernels (even on devices with that very PWM controller), so I wouldn't worry overmuch ;).

EDIT: Especially since your device is not using that controller ;p.

(On Kobo, warmth is always handled via sysfs).

@robintschoetschel
Copy link
Author

Hm, maybe I am a bit dense here, but I am trying.

@NiLuJe, I am trying to replace the lua file that contains the RootController, but I can't seem to find that one in the apk (so I can hotfix it to test any changes). I think I am missing something/am misunderstanding you

@pazos so I should just set TolinoRootcontroller as the controller for Tolino Shine 4 (independent if the user is root)? As far as I can tell, changes need to be made to DeviceInfo and LightsFactory — any other obvious place I am missing? (along with the changes to TolinoRootController, of course)

@mergen3107
Copy link
Contributor

I am trying to replace the lua file that contains the RootController, but I can't seem to find that one in the apk (so I can hotfix it to test any changes)

KOReader on Android debugging a little rough. You need to build new apk with these changes to test.

Let me know if I can help you set up the build :D

@NiLuJe
Copy link
Member

NiLuJe commented Dec 26, 2023

You can just overwrite Lua files directly (e.g., over adb), since you're root. No need to build a full APK just to tweak a few Lua scripts ;).

I don't have access to adb right now, but once you've got the right app folder in /data or whatever, the directory tree is nothing special.

@mergen3107
Copy link
Contributor

It is
/data/data/org.koreader.launcher/files/

Lua files can be edited there.

When making edits (like I do in WinSCP) keep in mind that some files might change their owner and group after edits. KOReader doesn't like this so you need to restore them. All files have the same owner/group for KOReader files.

LightsFactory.kt and the rest related to android-luajit are not there though.

@NiLuJe
Copy link
Member

NiLuJe commented Dec 26, 2023

Oh, right, for some reason I was stuck in powerd-land in my head. Yeah, if you're tweaking actual Kotlin sources, you'll need a full build ;).

@pazos
Copy link
Member

pazos commented Dec 26, 2023

Too much noise in here :)

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.

Since both @mergen3107 and @hugleo are quite active lately and both have a working KO/android environment on their machines feel free to submit a PR (and an APK for OP to test)

Otherwise we will wait for @robintschoetschel work.

@robintschoetschel
Copy link
Author

Thanks for the summary. I'll wait for a bit to see if either of the two reply. If they aren't keen on this task, I'll have a crack at it

@mergen3107
Copy link
Contributor

@robintschoetschel
Here is the link to the debug apk with Tolino Shine 4 support. Please let us know how it goes: try manual eink refresh, lights, and everything else to make sure it works.

https://disk.yandex.com/d/jmfIOM4mRqMQTw

@mergen3107
Copy link
Contributor

https://disk.yandex.com/d/SFeGytllDxZwdQ
Here is another link with fixed device definitions, it might work better.

@robintschoetschel
Copy link
Author

Thanks for the quick attempt. The device is marked as supported, but unfortunately, neither lights nor refresh works (on the second file you posted). See the attached log. I think it sill uses the wrong color file, which should be sys/class/leds/aw99703-bl_FL1/color

Using the first file, the device is still shown as unsupported, and the dimming works; warmth does not

test.log

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

Does your device is rooted? Is as permission problem.

Check these two new versions if you will get better logs:

https://we.tl/t-2wzVfKgr2

@robintschoetschel
Copy link
Author

robintschoetschel commented Dec 28, 2023

I have root, KoReader asks for it, and I have granted it. Your link is broken, it seems.

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

I have root, KoReader asks for it, and I have granted it. Your link is broken, it seems.

Oh, maybe they deleted already :(. I'll upload again later.

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

Looks like a letter was missing. I try all the letter possibilities at the end and guess the correct link :)
https://we.tl/t-2wzVfKgr2U

@robintschoetschel
Copy link
Author

Sorry, no dice. There still are some permission problems.

I checked to ensure I have root and can open all system files in KoReader's built-in text editor...
test_V2.log
test_V1.log

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

Can you also check on shell if there is anything related to selinux?

getenforce

for test if needed:
setenforce 0 # Set SELinux to Permissive

@robintschoetschel
Copy link
Author

I'm using the built-in terminal emulator. I get "permission denied" errors following these commands. However, I can punch in su and Magisk reports that KoReader has been given superuser status. After I setenforce 0, I can exit from su and getenforce now reports "Permissive" (even without su).

After closing the shell, I can adjust warmth in the compatibility test. Dim doesn't work, nor refresh. And warmth doesn't work in normal use (outside of the compatibility testing environment).

test.log

@mergen3107
Copy link
Contributor

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

For the brightness current is using the file: /sys/class/backlight/mxc_msp430_fl.0/actual_brightness
Should be different as well, you can find the correct.

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

After closing the shell, I can adjust warmth in the compatibility test. Dim doesn't work, nor refresh. And warmth doesn't work in normal use (outside of the compatibility testing environment).

Also good to see the logs from my V1, V2 again after the selinux change.

@robintschoetschel
Copy link
Author

Here are the two logs (V2 is the same as above).
In V1 warmth now works also in reader mode (!), but unfortunately no dimming yet.

test_permissive_V1.log
test_permissive_V2.log

@robintschoetschel
Copy link
Author

And the brightness files are /sys/class/backlight/mxc_msp430.0/brightness for changing and /sys/class/backlight/mxc_msp430.0/actual_brightness (read-only, also as su)

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

cat /sys/class/backlight/mxc_msp430.0/brightness
cat /sys/class/backlight/mxc_msp430.0/actual_brightness
same value? Can we use just the file /sys/class/backlight/mxc_msp430.0/brightness to read and set the dimming?

@robintschoetschel
Copy link
Author

robintschoetschel commented Dec 28, 2023

Yeah, they always have the same value. If I try to set "brightness" above 100, both get corrected to 70. Yes, the brightness file can be used for dimming

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

Try this first:
If I'm right you should be able to see correct dimming value but not set yet: https://we.tl/t-1rQnYWfVnZ

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

and for the /sys/class/leds/aw99703-bl_FL1/color the max value is 10, correct?

@robintschoetschel
Copy link
Author

Yes, correct.

Here is a log from V3: test_permissive_V3.log

I'm not sure what you mean by "see the correct dimming value" — but the slider doesn't reflect the value I set by changing the file (via adb)

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

Try this: https://we.tl/t-EXAV1JQHH6

@robintschoetschel
Copy link
Author

New log:
test.log

The brightness seemed to be moving in the opposite direction? Setting it to 100 is 0 in the file, setting it to 1 is 69

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

That's odd. Was a quick coding, maybe I've missed something :)

Then we finally have a drive skeleton. What I have so far:

package org.koreader.launcher.device.lights

import android.app.Activity
import android.provider.Settings
import android.util.Log
import org.koreader.launcher.device.LightsInterface
import org.koreader.launcher.extensions.read
import org.koreader.launcher.extensions.write
import java.io.File

/* Special controller for Tolino Epos/Epos2.
 * see https://github.com/koreader/koreader/pull/6332
 *
 * Thanks to @zwim
 */

class TolinoRootController : LightsInterface {
    companion object {
        private const val TAG = "Lights"
        private const val BRIGHTNESS_MAX = 70
        private const val WARMTH_MAX = 10
        private const val MIN = 0
        private const val BRIGHTNESS_FILE = "/sys/class/backlight/mxc_msp430.0/brightness" // always readable, same for Epos2 and Vision4
        private const val COLOR_FILE = "/sys/class/leds/aw99703-bl_FL1/color"
    }

    override fun getPlatform(): String {
        return "tolino"
    }

    override fun hasFallback(): Boolean {
        return false
    }

    override fun hasWarmth(): Boolean {
        return true
    }

    override fun needsPermission(): Boolean {
        return false
    }

    // try to toggle on frontlight switch on Tolinos, returns the former switch state
    override fun enableFrontlightSwitch(activity: Activity): Int {
        /*// ATTENTION: getBrightness, setBrightness use the Android range 0..255
        // in the brightness files the used range is 0..100
        val startBrightness = getBrightness(activity)

        val actualBrightnessFile = File(ACTUAL_BRIGHTNESS_FILE)
        val startBrightnessFromFile = try {
            actualBrightnessFile.readText().trim().toInt()
        } catch (e: Exception) {
            Log.w(TAG, "$e")
            -1
        }

        // change the brightness through android. Be aware one step in Android is less than one step in the file
        if (startBrightness > BRIGHTNESS_MAX/2)
            setBrightness(activity, startBrightness - (BRIGHTNESS_MAX/100+1))
        else
            setBrightness(activity, startBrightness + (BRIGHTNESS_MAX/100+1))

        // we have to wait until the android changes seep through to the file,
        // 50ms is to less, 60ms seems to work, so use 80 to have some safety
        Thread.sleep(80)

        val actualBrightnessFromFile = try {
            actualBrightnessFile.readText().trim().toInt()
        } catch (e: Exception) {
            Log.w(TAG, "$e")
            -1
        }

        setBrightness(activity, startBrightness)

        if (startBrightnessFromFile == actualBrightnessFromFile) {
            return try { // try to send keyevent to system to turn on frontlight, needs extended permissions
                Runtime.getRuntime().exec("su -c input keyevent KEYCODE_BUTTON_A && echo OK")
                1
            } catch (e: Exception) {
                e.printStackTrace()
                0
            }
        }*/

        return 1
    }

    override fun getBrightness(activity: Activity): Int {
        return BRIGHTNESS_MAX - File(BRIGHTNESS_FILE).read()
    }

    override fun getWarmth(activity: Activity): Int {
        return WARMTH_MAX - File(COLOR_FILE).read()
    }

    override fun setBrightness(activity: Activity, brightness: Int) {
        if (brightness < MIN || brightness > BRIGHTNESS_MAX) {
            Log.w(TAG, "brightness value of of range: $brightness")
            return
        }
        val brightnessFile = File(BRIGHTNESS_FILE)
        Log.v(TAG, "Setting brightness to $brightness")

        try {
            if (!brightnessFile.canWrite()) {
                Runtime.getRuntime().exec("su -c chmod 666 $COLOR_FILE")
            }
            brightnessFile.write(BRIGHTNESS_MAX - brightness)
        } catch (e: Exception) {
            Log.w(TAG, "$e")
        }
    }

    override fun setWarmth(activity: Activity, warmth: Int) {
        if (warmth < MIN || warmth > WARMTH_MAX) {
            Log.w(TAG, "warmth value of of range: $warmth")
            return
        }
        val colorFile = File(COLOR_FILE)
        Log.v(TAG, "Setting warmth to $warmth")
        try {
            if (!colorFile.canWrite()) {
                Runtime.getRuntime().exec("su -c chmod 666 $COLOR_FILE")
            }
            colorFile.write(WARMTH_MAX - warmth)
        } catch (e: Exception) {
            Log.w(TAG, "$e")
        }
    }

    override fun getMinWarmth(): Int {
        return MIN
    }

    override fun getMaxWarmth(): Int {
        return WARMTH_MAX
    }

    override fun getMinBrightness(): Int {
        return MIN
    }

    override fun getMaxBrightness(): Int {
        return BRIGHTNESS_MAX
    }

    override fun hasStandaloneWarmth(): Boolean {
        return false
    }
}

@mergen3107 @pazos I think is better create a new driver or this can be refactored somehow?
@robintschoetschel About the sle setenforce permissive thing, I think the config will be lost if you reboot the device, maybe there is possible something like a script you can set again each boot.

@robintschoetschel
Copy link
Author

Thank you for your work! It's already a big improvement. Anybody an idea about how to get around the setenforce thing? I can write a little shell script to be run on boot, but it'd be more elegant without.

The manual refresh doesn't work still; just to document that.

@NiLuJe
Copy link
Member

NiLuJe commented Dec 28, 2023

cat /sys/class/backlight/mxc_msp430.0/brightness
cat /sys/class/backlight/mxc_msp430.0/actual_brightness

One is technically wo (brightness), and one is ro (actual_brightness).

i.e., there's a getter and a setter ;). (You might be able to read from brightness though, but that's not a guarantee).

As a historical note, actual_brightness is not available on early NTX boards (and brightness is really wo there), meaning you can't request the current brightness level from the system, which is part of why the KOReader frontlight API can be so twisted to deal with ;).

@pazos
Copy link
Member

pazos commented Dec 28, 2023

In #410 (comment) OP stated that both drivers are OK for changing brightness, so no changes are required. Please keep it simple :)

Also, no driver that requires disabling selinux will be commited. At that point please consider other routes, including noop. Because there're no reason for a particular app to request their users to harm themselves without any purpose. Better to avoid the feature completely, since it can be changed elsewhere.

The manual refresh doesn't work still; just to document that.

That was also expected and won't be fixed here. Please keep this ticket focused on warmth.

@hugleo
Copy link
Contributor

hugleo commented Dec 28, 2023

In #410 (comment) OP stated that both drivers are OK for changing brightness, so no changes are required. Please keep it simple :)

That's weird because the V1 from the message here #410 (comment) is the same @mergen3107 with some logs.
The last version the main changes are the brightness (dimm) part that works now with sle permissive.

@robintschoetschel Now with de sle permissive, can you test again the @mergen3107 from yesterday? #410 (comment)
Tell what works and what not work. Delete the sle rule for the default just in case and see if the brightness (dimm) works again if that was really working at beginning.
Trying to figure out what happened here :)

EDIT: As selinux stuff is not allowed, another path must be found :)

@robintschoetschel
Copy link
Author

robintschoetschel commented Dec 28, 2023

Ok, back to the drawing board. No problem. I won't report on the refresh anymore, though.

@hugleo the version in the comment you linked by @mergen3107 has neither warmth nor lights working... Here is the log: test_TOLINO_SHINE4_457_Version2_koreader-android-arm-debug-v2023.10-75-gc12b4f2e1_2023-12-27.log — I guess something broke already there? In stable/Ovis I can still dim without problems.

And here is a log of your V4 with selinux back to default:
test_V4.log — unfortunately neither is working. When it did, it used the range of the frontlight dim much better than the current default, which doesn't turn off completely, as per (koreader/koreader#11013) (this is strictly out of scope here, but maybe we can fix it along the way)

@pazos
Copy link
Member

pazos commented Jan 3, 2024

@robintschoetschel: just do a build from master with #457 merged in.

And start the test activity again from the new application.

Then tell us if the test activity says "device supported" and which eink/light drivers are being used.

@robintschoetschel
Copy link
Author

I'm travelling and a bit busy with work, but will follow up on this later next week, thanks @pazos

@pazos
Copy link
Member

pazos commented Jun 28, 2024

No activity, closing

@pazos pazos closed this as completed Jun 28, 2024
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

No branches or pull requests

5 participants