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

Kobo Forma support #4291

Closed
patrick-tang opened this issue Oct 28, 2018 · 91 comments
Closed

Kobo Forma support #4291

patrick-tang opened this issue Oct 28, 2018 · 91 comments

Comments

@patrick-tang
Copy link
Contributor

  • KOReader version:
    koreader-kobo-arm-linux-gnueabihf-v2015.11-stable
    (launch with KFMon-v1.2.6-1-g9ab7d3a)
  • Device:
    Kobo Forma
    (firmware version: N782890001425,4.1.15,4.11.11911,4.1.15,4.1.15,00000000-0000-0000-0000-000000000377)

Issue

In new released Kobo Forma, when launching Koreader, it crashed right away, reporting "unrecognized Kobo model frost".

Complete detailed log:
crash.log

Steps to reproduce

After installed, try to launch Koread, it will crash with above crash log.

crash.log (if applicable)

./luajit: frontend/device/kobo/device.lua:177: unrecognized Kobo model frost
stack traceback:
[C]: in function 'error'
frontend/device/kobo/device.lua:177: in main chunk
[C]: in function 'probeDevice'
frontend/device.lua:37: in main chunk
[C]: in function 'require'
frontend/ui/uimanager.lua:1: in main chunk
[C]: in function 'require'
./reader.lua:92: in main chunk
[C]: at 0x0000bea5

@Frenzie
Copy link
Member

Frenzie commented Oct 28, 2018

In frontend/device/kobo/device.lua, you could try adding a definition:

Here's an example:

-- Kobo Clara HD:
local KoboNova = Kobo:new{
model = "Kobo_nova",
hasFrontlight = yes,
touch_snow_protocol = true,
display_dpi = 300,
hasNaturalLight = yes,
frontlight_settings = {
frontlight_white = "/sys/class/backlight/lm3630a_ledb",
frontlight_red = "/sys/class/backlight/lm3630a_leda",
-- NOTE: There doesn't appear to be a dedicated "green" LED, instead,
-- there's a knob that mixes the white & red ones together (/sys/class/backlight/lm3630a_led).
-- c.f., https://www.mobileread.com/forums/showpost.php?p=3728236&postcount=2947
},
}

Sorry for the brevity, but I have to be off now. :-)

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 28, 2018

Thanks, Frenzie.

I've made two changes in /front/device/kobo/device.lua file:

  1. line 91: (after KoboDragon block)
    -- Kobo Forma:
    local KoboFrost = Kobo:new{
    model = "Kobo_frost",
    hasFrontlight = yes,
    touch_probe_ev_epoch_time = true,
    touch_phoenix_protocol = true,
    display_dpi = 300,
    hasNaturalLight = yes,
    }

  2. line 683 (after elseif codename=="snow" )
    elseif codename == "frost" then
    return KoboFrost

Then I tried to launch Koreader, my Kobo Forma restarted then see following:

  1. it asked me to login Kobo.
  2. Even though all books are in the device, the Home shows no any book.
  3. When I connected to computer, the screen is different (like connection screen from another firmware?)

@NiLuJe
Copy link
Member

NiLuJe commented Oct 28, 2018

When a Kobo asks you to re-login, that wipes the DB, IIRC. Which means you may need to plug/unplug or reboot it for it to process stuff again, provided it hasn't done a bonus factory reset, which would explain 3., as that reinstalls the factory FW, but not 2., because I think that wipes the user partition, too.

If it actually did a factory reset, that's a fairly visible process, though, so you'd have noticed the process happening on screen.

On the other hand, if that was a simple reboot and it just asked you to re-login just after that, that was simply a trashed database, which can happen (or the whole process can sometimes be misdetected as "useful" and almost forced on you, even when it isn't actually useful, and the db isn't actually damaged), after bad crashes.

Sidebar: starting with the extremely old "stable" release was also a fairly bad idea ;).

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

In the meantime, if you got another crash.log from the hard crash, that might be useful, too ;).

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 29, 2018

@NiLuJe it seems a simple reboot, but something weired because other than a home without any book I also see the settings missing "accounts" submenu. I forced to restart again, everything is back, including colletion settings.

It was my bad. I forgot to metion when I made the above metioned changes, I actually used the latest beta version and copied the code for daylight ( which I blieve is Kobo Aura One) for my Kobo Forma. So seems those same code for Aura One not working for Kobo Forma.

What else information I can collect? Would like to fix this with help from you guys. The PDF reading with Kobo Forma is rally bad, plus there is one PDF I need to read keeps crashing the device.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

Might have been a botched Nickel restart after a KOReader crash then...

The crash.log from that might help, to begin with ;).

But it's probably going to take someone with shell access to get this right, because I'm expecting the gyro/rotation handling to be wonderfully annoying to deal with.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

The One is an older device, you'll probably have less terrible results if you start from a Clara, that's what's gotten the most testing out of that generation of devices ;).

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 29, 2018

@NiLuJe
Thanks for the tips. I have copied the setting for Clara, now I'm able to launch Koreader.
However I cannot tap menu items properly. It seems the viewport setting was not correct.
I tried to set up with the resolution, but Koreader then couldn't start.

How can I find out correct viewport setting for this device?
Attached is the log after I add viewport setting (Note: I tried different combination, so there are different log sections).
crash.log

I don't quit understand: no matter how I switched the numbers for the width and height, the error still showing same height and width. Same even after I commented out or removed the viewport lines.

@Frenzie
Copy link
Member

Frenzie commented Oct 29, 2018

@patrick-tang The viewport setting is just for KOReader to draw further from the edge of the actual panel, like on the H2O where ~10-11 px are covered up by the bezel.

The full framebuffer will always be the same size, in your case 1920x1440. The viewport is a smaller area within that framebuffer.

The simplest means of quickly testing is to start from scratch on touch workarounds. Getting rid of that touch_snow_protocol = true, for example.

If the touch is mirrored in some way, set mirrored to false:

-- most Kobos have X/Y switched for the touch screen
touch_switch_xy = true,
-- most Kobos have also mirrored X coordinates
touch_mirrored_x = true,

What do you mean by "cannot tap menu items properly"? Is there any response?

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

According to the latest news from the KSM thread, you might want to try phoenix + probe_ev ;)

@patrick-tang
Copy link
Contributor Author

@Frenzie
Sorry I didn't describe clearly. For "cannot tap menu items properly", I meant when I tried to tap the menu items I couldn't activate a menu item by tap on the item, but I had to tap another item (or need to shift the position) - however not like mirrored.
Question:
what is "touch_snow_protocol = true" for? or function of any such touch_xxxx_protocol ?

@NiLuJe
Do you have the link for KSM thread about "phoenix + probe_ev"? Or a keyword I can search?
I think I tried "touch_probe_ev_epoch_time = true, touch_phoenix_protocol = true" and suspect those settings reboot the device to special state (like another version of firmware).

@Frenzie
Copy link
Member

Frenzie commented Oct 29, 2018

So touch is working, just off by a few pixels? That's very weird, but also very easy to work around. Potentially even with viewport instead of event adjustment. Sorry, on my phone atm.

But please try Niluje's suggestion first.

Hold works too?

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018 via email

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

And, nope, my bad, skimmed stuff too rapidly, snow is indeed probably the right protocol, to begin with.

Might need some extra shenanigans, like touch_probe_ev_epoch_time = true, or unlikelier, but who knows, touch_mirrored_x = false.

Stuff might be affected by the rotation you were in before starting KOReader, so I'd keep it simple: Portrait, buttons on the right (or the left?).

And, yeah, as @Frenzie said, knowing if hold/swipes work right would be helpful.

@patrick-tang
Copy link
Contributor Author

@NiLuJe
what is "touch_probe_ev_epoch_time = true" for?
I don't see Clara using it.

Just want to confirm that hold/swipes work (but buttons don't work). Other problems I notice with current status:

  1. the backlight will keep on when in sleep.
    If I exit from Koreader and go back to Kobo Home, the backlight will be off in sleep.
    Is this related to backlight class not found issue in error log?

  2. buttons don't work, so can only use swipe/tap to turn pages

For above two problems, what should I look at?

@Frenzie
Copy link
Member

Frenzie commented Oct 29, 2018

Buttons are really simple, but you need to switch to debug mode, ideally combined with using Telnet or SSH. Isn't there a guide about that on Mobileread somewhere?

Then you simply get output like this:

10/29/18-20:03:36 DEBUG key event => type: 1, code: 13(J), value: 1, time: 1540839816.344482
10/29/18-20:03:36 DEBUG AutoSuspend: onInputEvent
10/29/18-20:03:36 DEBUG key event => type: 1, code: 13(J), value: 0, time: 1540839816.426518

tl;dr Buttons are the easiest problem to solve. :-)

Unfortunately the closest thing to a basic debugging for dummies guide we've currently got is the one for Android for which I supplied most of the materials, but that one presupposes you already know the basics.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 29, 2018

Not really, as there's a bunch of different ways you can actually get in.

In this specific instance, using the built-in one (i.e., enabling Nickel's debug features, which enables telnet) is probably a no-go, since Nickel won't be up ;).

That leaves a telnet/SSH package.
I've got one, with the potential caveat that it bundles a lot of CLI utilities, too. I see that as a plus, personally, and it's also fairly unintrusive as far as things go ;).

@frostschutz has one, too, which you'll just need to package yourself.

And there's the good old and venerable package from KevinShort.

Among a few others, but that'd be my top three, OTOH ;).

@Frenzie
Copy link
Member

Frenzie commented Oct 29, 2018

I think the last one there might be the one I first used to start hacking my H2O.

@patrick-tang
Copy link
Contributor Author

@NiLuJe @Frenzie
Thanks for the information. Let me figured out the telnet then get back.
I think it would be safer I get the inittab from my Forma and modify it.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 30, 2018

NO. NOPE. NOPE NOPE NOPE.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 30, 2018

That's why I put mine & frostchutz's first: we both go out of our way to actively not mess with any default system files.

That's both neater, the right thing to do, and allows us to survive FW updates ;).

(i.e.: if we liked the inittab method, which the third package does use, we'd have been using it ourselves, too ;p).
But, in a sense, yeah, if you want to go that way, definitely start from your own, current, up to date inittab, since that package is Very Old(TM).

@frostschutz
Copy link

Actually, mine totally does modify the inittab :-) and still uses the hwclock hack too. For other mods I just use udev rule to get things started, but I think I made the telnet one before I learned how to do that... oi should update it, I guess

@NiLuJe
Copy link
Member

NiLuJe commented Oct 30, 2018

@frostschutz: heretic! :D

@pazos
Copy link
Member

pazos commented Oct 30, 2018

@NiLuJe inittab does survive firmware upgrades, as far as I remember. /etc/init.d/on-animator.sh does not survive.

I remember when I was using a script launched from inittab to restore on-animator.sh from on-animator-fmon.sh each time I updated the FW and worked.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 30, 2018

I vaguely recall seeing it pop up in some FW updates, but that was a loooong while ago. Or something :D.

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 31, 2018

@Frenzie

You're right about mirrored tapping another day. The tapping was kind of mirrored: it was actually due to the inconsistency of display and tapping. The display followed screen rotation (rotation status before launching Koreader) but tapping was still expecting coordinates in non-rotated state (original orietation as device just started).

What's your suggestion of the fix?
Currently I have to rotate back (to original orientation like the device just started) before launching Koreader - mostly I have to restart the device to guarantee Koreader sees correct orientation.

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 31, 2018

@Frenzie @NiLuJe
I can telnet into my Forma now.
Would you please help with some simple guide for enabling debug? I would generate some key event log for the button issue.

@patrick-tang
Copy link
Contributor Author

patrick-tang commented Oct 31, 2018

For the light issue, I got following information. It seems there are two files for the light control:

  • mxc_msp430.0: for the brightness only
  • tlc5947_bl: for color and brightness

ls -lah /sys/class/backlight/

lrwxrwxrwx    1 root     root           0 Oct 30 22:31 mxc_msp430.0 -> ../../devices/platform/ntx_bl/backlight/mxc_msp430.0
lrwxrwxrwx    1 root     root           0 Oct 30 22:31 tlc5947_bl -> ../../devices/platform/soc/2000000.aips-bus/2000000.spba-bus/2008000.ecspi/spi_master/spi0/spi0.0/backlight/tlc5947_bl

ls -alh /sys/class/backlight/mxc_msp430.0/

drwxr-xr-x    3 root     root           0 Oct 30 22:31 .
drwxr-xr-x    3 root     root           0 Oct 30 22:31 ..
-r--r--r--    1 root     root        4.0K Oct 30 22:43 actual_brightness
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 bl_power
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 brightness
lrwxrwxrwx    1 root     root           0 Oct 30 22:43 device -> ../../../ntx_bl
-r--r--r--    1 root     root        4.0K Oct 30 22:43 max_brightness
drwxr-xr-x    2 root     root           0 Oct 30 22:43 power
lrwxrwxrwx    1 root     root           0 Oct 30 22:43 subsystem -> ../../../../../class/backlight
-r--r--r--    1 root     root        4.0K Oct 30 22:43 type
-rw-r--r--    1 root     root        4.0K Oct 30 22:31 uevent

ls -alh /sys/class/backlight/tlc5947_bl/

drwxr-xr-x    3 root     root           0 Oct 30 22:31 .
drwxr-xr-x    3 root     root           0 Oct 30 22:31 ..
-r--r--r--    1 root     root        4.0K Oct 30 22:43 actual_brightness
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 bl_power
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 brightness
-rw-r--r--    1 root     root        4.0K Oct 30 22:31 color
lrwxrwxrwx    1 root     root           0 Oct 30 22:43 device -> ../../../spi0.0
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 fl_r_en
-r--r--r--    1 root     root        4.0K Oct 30 22:43 max_brightness
-rw-r--r--    1 root     root        4.0K Oct 30 22:31 max_color
-rw-r--r--    1 root     root        4.0K Oct 30 22:43 out
drwxr-xr-x    2 root     root           0 Oct 30 22:43 power
lrwxrwxrwx    1 root     root           0 Oct 30 22:43 subsystem -> ../../../../../../../../../../../class/backlight
-r--r--r--    1 root     root        4.0K Oct 30 22:43 type
-rw-r--r--    1 root     root        4.0K Oct 30 22:31 uevent

@NiLuJe
Copy link
Member

NiLuJe commented Nov 11, 2018

Eh, that might be enough for us once Nickel's gone, thanks!

I'll post a test koreader.sh script (much) later tonight.

NiLuJe added a commit to NiLuJe/koreader that referenced this issue Nov 12, 2018
@NiLuJe
Copy link
Member

NiLuJe commented Nov 12, 2018

There, check if this version of the koreader.sh startup script allows you to start properly, no matter the orientation.

@pickx
Copy link

pickx commented Nov 12, 2018

I'm not sure if warm light control is supposed to work with this initial support, but it doesn't on my device. Both sliders seem to be controlling regular light.

@pazos
Copy link
Member

pazos commented Nov 12, 2018

@pickx

I'm not sure if warm light control is supposed to work with this initial support, but it doesn't on my device. Both sliders seem to be controlling regular light.

@NiLuJe added support based on this thread feedback and @patrick-tang said that it worked for him.

If you have shell access you can try #4291 (comment). Also please post the output of
udevadm info --attribute-walk --path=/sys/class/backlight/tlc5947_bl/

Current sysfs interface write on bl_power & brightness. Newer devices may need to write to color instead.

@pickx
Copy link

pickx commented Nov 13, 2018

@pazos
I did some experimentation with tlc5947_bl. I'm a newbie, bear with me if all of this is obvious.

/sys/class/backlight/tlc5947_bl/ indeed controls the natural light color on my Forma. Writing to bl_power or brightness does not seem like the right approach because Nickel does not seem to write to these values. Going back and forth from Nickel to koreader and checking the attributes of tlc5947_bl, it seems Nickel instead just uses the color attribute, as you have said.

Nickel doesn't allow fine-grained control over natural light (at least on Forma), instead the slider has 11 steps going from 10 to 0, which gives 10 color presets (10 is whitest, 0 reddest), and sets the "out" attribute via some combination with the backlight brightness (I'm guessing color presets are just scalars).

Fine-grained control can probably be achieved by modifiying the "out" attribute directly, though I'm not sure such functionality is wanted. Keeping sync with Nickel seems like a more reasonable approach. I personally don't find such fine-grained control useful.

@baskerville
Copy link
Contributor

As I said earlier, the output of lsof | grep '/sys/' could be helpful.

@pazos
Copy link
Member

pazos commented Nov 18, 2018

@pickx: for the record, on a BQ Cervantes 4:

ls -lah  /sys/class/backlight/
total 0
drwxr-xr-x  2 root root 0 Nov 18 15:35 .
drwxr-xr-x 32 root root 0 Nov 18 15:34 ..
lrwxrwxrwx  1 root root 0 Nov 18 15:37 lm3630a_led -> ../../devices/platform/imx-i2c.0/i2c-0/0-0036/backlight/lm3630a_led
lrwxrwxrwx  1 root root 0 Nov 18 15:37 lm3630a_leda -> ../../devices/platform/imx-i2c.0/i2c-0/0-0036/backlight/lm3630a_leda
lrwxrwxrwx  1 root root 0 Nov 18 15:37 lm3630a_ledb -> ../../devices/platform/imx-i2c.0/i2c-0/0-0036/backlight/lm3630a_ledb
lrwxrwxrwx  1 root root 0 Nov 18 15:37 mxc_msp430_fl.0 -> ../../devices/platform/mxc_msp430_fl.0/backlight/mxc_msp430_fl.0

This device has both the "classic" interface (being lm3630a_ledb for white leds and lm3630a_leda for red leds) and the newer color mixer interface (lm3630a_led is the color mixer and mxc_msp430_fl.0 is for controlling the brightness). It seems that the Kobo Forma doesn't have the "classic" interface. Other recent models, like the Clara, happen to have both interfaces (and the classic is used https://github.com/koreader/koreader/blob/master/frontend/device/kobo/device.lua#L192-L198)

Color mixer

udevadm info --attribute-walk --path=/sys/class/backlight/lm3630a_led
  looking at device '/devices/platform/imx-i2c.0/i2c-0/0-0036/backlight/lm3630a_led':
    KERNEL=="lm3630a_led"
    SUBSYSTEM=="backlight"
    DRIVER==""
    ATTR{bl_power}=="31"
    ATTR{brightness}=="50"
    ATTR{max_brightness}=="100"
    ATTR{type}=="raw"
    ATTR{color}=="1"
    ATTR{max_color}=="10"
    ATTR{ramp}=="0"
    ATTR{ramp_on}=="0"
    ATTR{ramp_off}=="0"
    ATTR{ramp_up}=="0"
    ATTR{ramp_down}=="0"

  looking at parent device '/devices/platform/imx-i2c.0/i2c-0/0-0036':
    KERNELS=="0-0036"
    SUBSYSTEMS=="i2c"
    DRIVERS=="lm3630a_bl"
    ATTRS{name}=="lm3630a_bl"
    ATTRS{modalias}=="i2c:lm3630a_bl"
    ATTRS{microamps_requested_vdd_fl_lm36}=="0"

  looking at parent device '/devices/platform/imx-i2c.0/i2c-0':
    KERNELS=="i2c-0"
    SUBSYSTEMS=="i2c"
    DRIVERS==""
    ATTRS{name}=="imx-i2c"

  looking at parent device '/devices/platform/imx-i2c.0':
    KERNELS=="imx-i2c.0"
    SUBSYSTEMS=="platform"
    DRIVERS=="imx-i2c"
    ATTRS{modalias}=="platform:imx-i2c"

  looking at parent device '/devices/platform':
    KERNELS=="platform"
    SUBSYSTEMS==""
    DRIVERS==""

bl_power and brightness attributes of the color mixer are taken from mxc_msp430_fl.0 and you can not write directly to lm3630a_led/bl_power or lm3630a_led/brightness.

In short

With the frontlight/naturalLight turned off the following commands will turn on both with maximum values

# turn on frontlight
echo 100 > /sys/class/backlight/mxc_msp430_fl.0/brightness
# set color mix (0 - 10)
echo 0 > /sys/class/backlight/lm3630a_led/color

NiLuJe added a commit to NiLuJe/koreader that referenced this issue Nov 20, 2018
@NiLuJe NiLuJe changed the title Kobo Forma (new model): unrecognized Kobo model frost Kobo Forma support Nov 21, 2018
@patrick-tang
Copy link
Contributor Author

patrick-tang commented Dec 5, 2018

@NiLuJe
I've created a PR as the key mapping is wrong for Forma in new release. (#4376)

And also the screen rotation handling is still not working properly in Forma:
The book and gesture following the rotation but the menu not, so still having problem.

@pazos
Copy link
Member

pazos commented Dec 5, 2018

@patrick-tang: Could you please give us feedback about natural(orange) light? Does it works?

About key mapping: in nickel key mapping depends on orientation, right?

@patrick-tang
Copy link
Contributor Author

@pazos
The natural light doesn't work in koreader.

The key mapping follows orientation, meaning the two keys will switch accordingly with orientation.

@cairnsh
Copy link
Contributor

cairnsh commented Dec 17, 2018

This replacement for the function setNaturalBrightness in frontend/device/sysfs_light.lua seems to work okay for me on the Forma that I got:

function SysfsLight:setNaturalBrightness(brightness, warmth)
    if not brightness then
        brightness = self.current_brightness
    end
    if not warmth then
        warmth = self.current_warmth
    end

    warmth = math.floor(warmth/10)

    self:_write_value("/sys/class/backlight/mxc_msp430.0/brightness", brightness)
    self:_write_value("/sys/class/backlight/tlc5947_bl/color", 10 - warmth)

    self.current_brightness = brightness
    self.current_warmth = warmth
end

It seems to be unnecessary to set bl_power.

@pazos
Copy link
Member

pazos commented Dec 17, 2018

@cairnsh: good work. Did you test if sync with nickel still works?

I guess we can add a new var named frontlight_mixer in https://github.com/koreader/koreader/blob/master/frontend/device/sysfs_light.lua#L11 and check if current device has frontlight_mixer with

if self.frontlight_mixer then
    warmth = math.floor(warmth/10)
    self:_write_value(self.frontlight_white, brightness)
    self:_write_value(self.frontlight_mixer, 10 - warmth)
else
    ...
end

@pazos
Copy link
Member

pazos commented Dec 18, 2018

This diff against master should work for the forma. The same aproach should work for other devices too

diff --git a/frontend/device/kobo/device.lua b/frontend/device/kobo/device.lua
index fd19a7cd..0512dc19 100644
--- a/frontend/device/kobo/device.lua
+++ b/frontend/device/kobo/device.lua
@@ -215,7 +215,7 @@ local KoboFrost = Kobo:new{
     hasNaturalLight = yes,
     frontlight_settings = {
         frontlight_white = "/sys/class/backlight/mxc_msp430.0",
-        frontlight_red = "/sys/class/backlight/tlc5947_bl",
+        frontlight_mixer = "/sys/class/backlight/tlc5947_bl",
     }
 }
 
diff --git a/frontend/device/sysfs_light.lua b/frontend/device/sysfs_light.lua
index 0252741e..cdb40559 100644
--- a/frontend/device/sysfs_light.lua
+++ b/frontend/device/sysfs_light.lua
@@ -9,6 +9,7 @@ local SysfsLight = {
     frontlight_white = nil,
     frontlight_red = nil,
     frontlight_green = nil,
+    frontlight_mixer = nil,
     current_brightness = 0,
     current_warmth = 0,
     white_gain = 25,
@@ -56,29 +57,36 @@ function SysfsLight:setNaturalBrightness(brightness, warmth)
         warmth = self.current_warmth
     end
 
-    local red = 0
-    local green = 0
-    local white = 0
-    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) *
+    -- Newer devices use a mixer instead of writting values per color.
+    if self.frontlight_mixer then
+        warmth = math.floor(warmth/10)
+        self:_write_value(self.frontlight_white, brightness)
+        self:_write_value(self.frontlight_mixer, 10 - warmth)
+    else
+        local red = 0
+        local green = 0
+        local white = 0
+        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) *
+        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) *
+            green = math.min(self.green_gain * math.pow(brightness, self.exponent) *
                              math.pow(warmth, self.exponent) + self.green_offset, 255)
-    end
+        end
 
-    white = math.max(white, 0)
-    red = math.max(red, 0)
-    green = math.max(green, 0)
+        white = math.max(white, 0)
+        red = math.max(red, 0)
+        green = math.max(green, 0)
 
-    self:_set_light_value(self.frontlight_white, math.floor(white))
-    self:_set_light_value(self.frontlight_green, math.floor(green))
-    self:_set_light_value(self.frontlight_red, math.floor(red))
+        self:_set_light_value(self.frontlight_white, math.floor(white))
+        self:_set_light_value(self.frontlight_green, math.floor(green))
+        self:_set_light_value(self.frontlight_red, math.floor(red))
+    end
 
     self.current_brightness = brightness
     self.current_warmth = warmth

@Frenzie
Copy link
Member

Frenzie commented Dec 18, 2018

You get syntax highlighting with

```diff

or lua/python/bash/etc. depending on what's in there ;-)

The diff looks good.

@NiLuJe
Copy link
Member

NiLuJe commented Dec 18, 2018

I should be getting my hands on a Forma tomorrow, so I'll be able to join in on the fun ;).

@NiLuJe
Copy link
Member

NiLuJe commented Dec 20, 2018

On Nickel's side, ColorSetting is still [1500 ... 6400], so I'll have to check how the normalization fares, because it's now only 10 steps on Nickel's side, while we're doing 100 in KoboPowerD:_syncKoboLightOnStart ;).

(Also, brightness seems to be written to actual_brightness [0 ...100] @ mxc_msp430.0, but I haven't checked what our code does on that front).

@NiLuJe
Copy link
Member

NiLuJe commented Dec 20, 2018

And before anyone asks: yes, that weird strip of brighter/colder light near the LEDs is real, and either a pretty serious design defect, or Kobo's QA team is entirely composed of blind monkeys.

At least I knew what I was getting, so, my expectations were suitably lowered ;).
Because other than that, it's a pretty solid device. Glad to see that the Mk.7 devices are actually feeling snappy!

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

No branches or pull requests

10 participants