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

WIP: [Android] Boox NovaPro sys natural backlight support #5331

Closed

Conversation

ColinKinloch
Copy link

@ColinKinloch ColinKinloch commented Sep 6, 2019

I recently bought a Boox Nova Pro (Android eInk device).
It doesn't respect the android system brightness settings.
It has two frontlight types white and orange at /sys/class/backlight/white/brightness and /sys/class/backlight/warm/brightness respectively.
Much of the code in this merge request is copied from the kobo frontend.
I feel much of the auto warmth code should be move to the generic frontend.
Any pointers on how this code could be improved would be appreciated.

white/device/uevent and warm/device/uevent contain:

DRIVER=lm3630a_bl
OF_NAME=lm3630a
OF_FULLNAME=/i2c@ff140000/lm3630a@38
OF_COMPATIBLE_0=onyx,lm3630a
OF_COMPATIBLE_N=1
MODALIAS=i2c:lm3630a

https://github.com/torvalds/linux/blob/master/Documentation/ABI/stable/sysfs-class-backlight
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/backlight/lm3630a_bl.c
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/video/backlight/backlight.c


This change is Reviewable

@Frenzie
Copy link
Member

Frenzie commented Sep 6, 2019

The generic SysfsLight module was abstracted out of Kobo because Cervantes also supports (cf. #4283). Pinging @pazos who should probably know the most about it.

@Frenzie Frenzie added the Android label Sep 6, 2019
@Frenzie Frenzie added this to the 2019.10 milestone Sep 6, 2019
@pazos
Copy link
Member

pazos commented Sep 6, 2019

I recently bought a Boox Nova Pro (Android eInk device).
It doesn't respect the android system brightness settings.
It has two frontlight types white and orange at /sys/class/backlight/white/brightness and /sys/class/backlight/warm/brightness respectively.

Wow, that's weird. Thanks for your time!

I feel much of the auto warmth code should be move to the generic frontend.

Yeah, some of the powerd code is now triplicated (it was duplicated before between Kobo and Cervantes). I will leave the abstraction out of the equation here unless you want to take a look on other devices powerd implementations. But yeah, something to figure out in the future.

@pazos
Copy link
Member

pazos commented Sep 6, 2019

BTW @ColinKinloch: are you aware of #4551 (comment)?

The SDK that onyx provides takes care of lights too among eink updates and other things. Feel free to give it a try if you wish. Anyways until somebody integrates the onyx sdk with https://github.com/koreader/android-luajit-launcher this PR can help!

@ColinKinloch ColinKinloch changed the title [Android] Boox NovaPro sys natural backlight support WIP: [Android] Boox NovaPro sys natural backlight support Sep 6, 2019
@ColinKinloch
Copy link
Author

The setBrightness of the Boox SDK only seems to take a single unsigned char.
I just compiled the demo, the buttons that trigger FrontLightController.setBrightness have no effect.
:(

@ColinKinloch
Copy link
Author

adb shell settings list system
Lists values related to brightness

brightness_state_key=0
cold_brightness_state_key=1
screen_brightness=2
screen_brightness_mode=0
screen_cold_brightness=120
screen_warm_brightness=88
sync_brightness_adjust_key=0
wake_up_brightness_key=0
warm_brightness_state_key=1

However they only seem to map to the boox frontlight config dialog.
Setting them with settings put system screen_cold_brightness 4 doesn't change anything.

@pazos
Copy link
Member

pazos commented Sep 6, 2019

adb shell settings list system lists values related to brightness

Yeah, they're free to get but (on normal systems) put a new value requires that the app has granted WRITE_SETTINGS permissions. We just switched to a less intrusive method on koreader/android-luajit-launcher@3750551:

It works this way:

  1. by default we're using the same brightness that the rest of the system. (The one that can be retrieved with adb shell settings get system screen_brightness and we retrieve using
    readSettingScreenBrightness. It returns an integer between 0 and 255)

  2. if the user overwrites system settings we just write the new one inside the activity window parameters unless it is out the valid range, in that case the system will use a value less than 0, which means use system settings again.

  3. to populate the frontlight widget we use getScreenBrightness(), which sould cover quirks present in 1 and 2.

On sleep the Nova switches off the backlight and doesn't restore it on wake.

That's weird. Does this happens on every app?

As I said, feel free to workaround this as you like. But I find your current implementation (writtind directly to sys nodes instead of using WRITE_SETTINGS permission) simpler and less hackier.

@ColinKinloch
Copy link
Author

Yeah, they're free to get but (on normal systems) put a new value requires that the app has granted WRITE_SETTINGS permissions. We just switched to a less intrusive method on koreader/android-luajit-launcher@3750551:

I'm writing and verifying the values using adb shell.
I think the values are just used to maintain state for the Boox frontlight dialog.

@ColinKinloch
Copy link
Author

That's weird. Does this happens on every app?

Yes.
It's not mapped to the standard android brightness settings so it would need to be implemented by the developers at Onyx.

@pazos
Copy link
Member

pazos commented Sep 6, 2019

I just give a quick look at the sdk. It seems that the methods used to retrieve and set light values are (for rk32xx) in https://github.com/buggins/coolreader/blob/master/android/src/com/onyx/android/sdk/device/RK32XXDevice.java#L797-L817

Edit: But, as you can see just above the hightlight, it seems that first needs to open the frontlight and to close once its done, so it seems exactly the same that you already implemented as just access to the /sys file descriptors via jni / framework methods.

@pazos
Copy link
Member

pazos commented Sep 6, 2019

About e-ink devices:

It seems that an already supported epd update routine does work on rk32xx devices.

You can test that with EinkTest. If the method called "ntx" works on your device you can add it to our list of supported devices as EINK_FREESCALE and EINK_SUPPORT. This should enable a submenu to change full refresh rate and the hability of requesting a full refresh with a diagonal swipe by default.

@pazos pazos mentioned this pull request Sep 13, 2019
@ColinKinloch
Copy link
Author

The UI on the Nova Pro has two independant sliders to control white and warm frontlights seen here.

Initialising / updating the brightness / warmth values from the hardware values for koreader is not a simple problem.

Would a simpler equation, per device equation or an option for a 0 - 255 slider per backlight colour be possible?

if brightness > 0 then
-- On Nickel, the values for white/red/green are roughly linearly dependent
-- on the 4th root of brightness and warmth.
white = math.min(self.white_gain * math.pow(brightness, self.exponent) *
math.pow(100 - warmth, self.exponent) + self.white_offset, 255)
end
if warmth > 0 then
red = math.min(self.red_gain * math.pow(brightness, self.exponent) *
math.pow(warmth, self.exponent) + self.red_offset, 255)
green = math.min(self.green_gain * math.pow(brightness, self.exponent) *
math.pow(warmth, self.exponent) + self.green_offset, 255)
end

@pazos
Copy link
Member

pazos commented Sep 13, 2019

Initialising / updating the brightness / warmth values from the hardware values for koreader is not a simple problem.

@ColinKinloch: you're using (part) of the interface for devices that have separate green and red leds. You might want to replace frontlight_red with frontlight_mixer to apply the code path:

if self.frontlight_mixer then
-- Honor the device's scale, which may not be [0...100] (f.g., it's [0...10] on the Forma) ;).
warmth = math.floor(warmth / self.nl_max)
if set_brightness then
self:_write_value(self.frontlight_white, brightness)
end
-- And it may be inverted... (cold is nl_max, warm is nl_min)
if set_warmth then
if self.nl_inverted then
self:_write_value(self.frontlight_mixer, self.nl_max - warmth)
else
self:_write_value(self.frontlight_mixer, warmth)
end

This makes things a lot of easier than calculting warmth with a mix of R/G leds

hasLightLevelFallback = no,
frontlight_settings = {
frontlight_white = "/sys/class/backlight/white",
frontlight_red = "/sys/class/backlight/warm",
Copy link
Member

Choose a reason for hiding this comment

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

use frontlight_mixer = "/sys/class/backlight/warm" instead

Copy link
Author

Choose a reason for hiding this comment

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

On mixer devices does writing to the /sys/class/backlight/x/color change the backlight colour or change the brightness of a separate set of LEDs?

The two brightness files on the Nova Pro control independent sets of LEDs.

Copy link
Member

Choose a reason for hiding this comment

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

We don't do that but probably the system does something like that (ie: adjust brightness intensity based on warmth level). See #4291 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

I feel like I cannot help because I don't know nothing about your device. Do you found some issues with your current implementation?

@pazos
Copy link
Member

pazos commented Sep 21, 2019

@ColinKinloch: please ping me when you want a review. I will also ping @NiLuJe since he implented the frontlight mixer and has awesome reviewer skills 😄

@pazos pazos modified the milestones: 2019.10, 2019.11 Oct 6, 2019
@Frenzie Frenzie modified the milestones: 2019.11, 2019.12 Nov 1, 2019
@pazos pazos removed this from the 2019.12 milestone Nov 19, 2019
@pazos
Copy link
Member

pazos commented Aug 1, 2020

Superseded by #6426

@ColinKinloch: feel free to submit an updated PR. You can implement your own controller using one of the existent as a template. See GenericController and TolinoWarmthController.

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

Successfully merging this pull request may close these issues.

None yet

3 participants