Skip to content

reMarkable Paper Pro support - #13620

Merged
Frenzie merged 18 commits into
koreader:masterfrom
NoaHimesaka1873:master
Apr 21, 2025
Merged

Frenzie merged 18 commits into
koreader:masterfrom
NoaHimesaka1873:master

Conversation

@NoaHimesaka1873

@NoaHimesaka1873 NoaHimesaka1873 commented Apr 17, 2025 •

Copy link
Copy Markdown
Contributor

What works:

  • Everything except restarting KOReader

What's janky:

  • Shutdown: rmpp-qtfb-shim and qtfb dies before it can display shutdown screen, resulting in whatever was displayed before staying
  • Refresh: controlled by xochitl, (reMarkable's proprietary UI/UX and controls screen) will change in future after qtfb and shim gets ability to control it.

What doesn't work:

  • restart KOReader: it doesn't even try to lol

Dependencies required to run:

You need to set a few environmental variables when launching, like this:
LD_PRELOAD=(path to your qtfb-shim.so) QTFB_SHIM_INPUT=false QTFB_SHIM_MODEL=false QTFB_SHIM_MODE=RGB565 KO_DONT_GRAB_INPUT=1 ./reader.lua

Closes #12856


This change is Reviewable

@Frenzie Frenzie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm, just a couple of nits and minor questions

Updating l10n/ is fine but I suspect done by mistake.

Comment thread frontend/device/remarkable/device.lua Outdated
Comment thread frontend/device/remarkable/device.lua Outdated
Comment thread frontend/device/remarkable/device.lua Outdated
@Frenzie Frenzie added this to the 2025.05 milestone Apr 17, 2025
@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

Yep, l10n/ update is a mistake which I don't know how to remove lol

Comment thread frontend/device/remarkable/device.lua Outdated
@pazos

pazos commented Apr 17, 2025 •

Copy link
Copy Markdown
Member

Frontlight: documentation is janky so couldn't figure out how to implement

I'm not sure what documentation :p .

It should be pretty straightforward (assuming only brightness, no warmth). Have a look at https://github.com/koreader/koreader/blob/master/frontend/device/sysfs_light.lua

Then on your device def you tell if has light and has warmth and include frontlight_settings override (mostly sysfs nodes, but also min and max if they're different). For instance: https://github.com/koreader/koreader/blob/master/frontend/device/kobo/device.lua#L323-L329

@NoaHimesaka1873

NoaHimesaka1873 commented Apr 17, 2025 •

Copy link
Copy Markdown
Contributor Author

@pazos One problem:

  1. rM uses 4 as off, 0 as on.
  2. brightness range is 0-2047 instead of 0-100 which that code assumes. (that range check seem like debug only?)

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

I can't really figure out how it works even after reading all the devices' examples, e.g. why the hell other devices call kobolight? What functions are actually necessary to implement?

It's just simple frontlight system, but the unnecessarily complex PowerD implementation is making me confused. It just need to do these things (base path is /sys/class/backlight/rm_frontlight/):

  • To turn frontlight on, write 0 to bl_power
  • To turn frontlight off, write 4 to bl_power
  • To read intensity, read brightness
  • To write intensity, write to brightness (0-2047)

I can't really figure this out, so I need help from someone else.

@pazos

pazos commented Apr 17, 2025

Copy link
Copy Markdown
Member

No problem. Lights don't need to be part of this PR.

why the hell other devices call kobolight?

currently there's no device that:

  1. has brightness only, no warmth
  2. updates the brightness by writting to sysfs nodes

kobolight uses a ioctl system call instead of writting to sysfs. It turns out it is widely used outside kobo too.

What functions are actually necessary to implement?

We need to handle all these bits in device specific powerd implementation. If you want to read one that doesn't hurt your brain you can look at https://github.com/koreader/koreader/blob/master/frontend/device/cervantes/powerd.lua

If you look at this chunk:

function CervantesPowerD:init()
-- Default values in case self:_syncLightOnStart() does not find
-- any previously saved setting (and for unit tests where it will
-- not be called)
self.hw_intensity = 20
self.initial_is_fl_on = true
if self.device:hasFrontlight() then
if self.device:hasNaturalLight() then
local nl_config = G_reader_settings:readSetting("natural_light_config")
if nl_config then
for key, val in pairs(nl_config) do
self.device.frontlight_settings[key] = val
end
end
-- Does this device's NaturalLight use a custom scale?
self.fl_warmth_min = self.device.frontlight_settings.nl_min or self.fl_warmth_min
self.fl_warmth_max = self.device.frontlight_settings.nl_max or self.fl_warmth_max
-- If this device has a mixer, we can use the ioctl for brightness control, as it's much lower latency.
if self.device:hasNaturalLightMixer() then
local kobolight = require("ffi/kobolight")
local ok, light = pcall(kobolight.open)
if ok then
self.device.frontlight_settings.frontlight_ioctl = light
end
end
self.fl = SysfsLight:new(self.device.frontlight_settings)
self:_syncLightOnStart()
else
local kobolight = require("ffi/kobolight")
local ok, light = pcall(kobolight.open)
if ok then
self.fl = light
self:_syncLightOnStart()
end
end
end
end

You'll see that it uses kobolight for brightness on both branches (hasNaturalLight or not!) but that doesn't mean the remarkable needs to do the same.

It should be enough to fill the same powerd functions as cervantes has, that take extra logic when the device has frontlight.

But, no problem. The rest of the work is solid. This is just peanuts. We can take care later 👍

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

@pazos Thanks to your comment, I actually managed to push frontlight support!

@pazos

pazos commented Apr 17, 2025

Copy link
Copy Markdown
Member

@pazos Thanks to your comment, I actually managed to push frontlight support!

Great! I think you're now on the mood to fix the last "doesn't work" from OP :)

restart KOReader: likely because it can't LD_PRELOAD rmpp-qtfb-shim again

You can add canRestart = no to your device definition to prevent users from trying it.

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

Sonce it's 4am here now, I'll work on it after I get some sleep

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

@pazos Thanks to your comment, I actually managed to push frontlight support!

Great! I think you're now on the mood to fix the last "doesn't work" from OP :)

restart KOReader: likely because it can't LD_PRELOAD rmpp-qtfb-shim again

You can add canRestart = no to your device definition to prevent users from trying it.

canRestart = no also removes the option to exit KOReader so I'll leave it like that.

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

On second look, it doesn't seem like it even tries to relaunch itself.

@pazos

pazos commented Apr 18, 2025

Copy link
Copy Markdown
Member

canRestart = no also removes the option to exit KOReader so I'll leave it like that.

Are you sure?. Then the android version would lack the exit button, too

@Frenzie

Frenzie commented Apr 18, 2025

Copy link
Copy Markdown
Member

Yeah, only the restart menu entry has that condition. :-)

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

Yeah, only the restart menu entry has that condition. :-)

Weird, when I test it, exit KOReader option is also gone too :(

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

first pic is with canRestart set to no and second pic is without that thing.
image
image

@Frenzie

Frenzie commented Apr 18, 2025

Copy link
Copy Markdown
Member

Something about the behavior must've broken since it was originally implemented. Drat.

@pazos

pazos commented Apr 18, 2025 •

Copy link
Copy Markdown
Member

Just give it a quick look.

It turns out that, until now, every device that cannot restart (android and macos) has defined hasExitOptions = no. That makes sense as in those devices it isn't possible to reboot/poweroff/suspend from the device.

canRestart was only tested paired with hasExitOptions. The intent is to replace the exit_menu with the exit callback directly.

But canRestart does indeed remove the exit entry on a device that hasExitOptions and the on_hold callback never fires.

@Frenzie

Frenzie commented Apr 18, 2025 •

Copy link
Copy Markdown
Member

Indeed, it still works on Android.

It looks like this doesn't work as intended anymore however, even when removing the suspend/reboot/poweroff. For the sake of this PR that doesn't matter, but it should become:

if not Device:canRestart() and not Device:canSuspend() and not Device:canReboot() and not Device:canPowerOff() then
    exit_settings.exit_menu = exit_settings.exit
    exit_settings.exit = nil
    exit_settings.restart_koreader = nil
end

if not Device:canRestart() then
exit_settings.exit_menu = exit_settings.exit
exit_settings.exit = nil
exit_settings.restart_koreader = nil
end
if Device:canSuspend() then
exit_settings.sleep = {
text = _("Sleep"),
callback = function()
UIManager:suspend()
end,
}
end
if Device:canReboot() then
exit_settings.reboot = {
text = _("Reboot the device"),
keep_menu_open = true,
callback = function()
UIManager:askForReboot()
end
}
end
if Device:canPowerOff() then
exit_settings.poweroff = {
text = _("Power off"),
keep_menu_open = true,
callback = function()
UIManager:askForPowerOff()
end
}
end

@Commodore64user

Copy link
Copy Markdown
Member

emulator with canRestart = no

Reader_bram-stoker_dracula epub_p13_2025-04-18_233430

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

ironed out some more quirks to make it more smooth

@benoit-pierre

Copy link
Copy Markdown
Member

Assuming everything will be squashed on merge, to fix the l10n module:

▸ git -C l10n checkout f00869ff7fb187bea212382636fe0877948ea7ae
▸ git commit -m 'revert unintended l10n changes' l10n

Also, now that the base PR was merged, please bump it as part of this PR:

▸ git -C base checkout fa825ad8edcdcef18b93c7575a9870437a6c4e7a
▸ git commit -m 'bump base' base

Comment thread frontend/device/remarkable/device.lua Outdated
Co-authored-by: Benoit Pierre <benoit.pierre@gmail.com>
Comment thread frontend/device/remarkable/device.lua Outdated
@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

@benoit-pierre Done!

@pazos

pazos commented Apr 20, 2025

Copy link
Copy Markdown
Member

@NoaHimesaka1873: we would need some instructions in https://github.com/koreader/koreader/wiki/Installation-on-ReMarkable

Could you add a new entry for the ReMarkable Paper Pro just below the manual installation?

The rest of the instructions also deserve some work, but first lets see how different the installation and launch of the program is on the rMPP

Comment thread frontend/device/remarkable/device.lua Outdated
Comment on lines +141 to +144
ev.value = (ev.value) * by.mt_scale_x
end
if ev.code == C.ABS_MT_POSITION_Y then
ev.value = (ev.value) * by.mt_scale_y

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for parentheses here either. ;-)

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

@pazos I was planning to do that after it was merged, but yeah I'll add it asap.

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

Done! It's quite complicated though.

@Frenzie

Frenzie commented Apr 21, 2025

Copy link
Copy Markdown
Member

Were you ready here or still testing/tweaking final details?

@NoaHimesaka1873

NoaHimesaka1873 commented Apr 21, 2025 •

Copy link
Copy Markdown
Contributor Author

@Frenzie Now ready!
Though some change (all related to screen refresh) will come in the future after qtfb and shim can control refresh. Don't know when it will be implemented so leave as is and let xochitl control refresh for now.

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

Also, current merge blocker is container upgrade to 22.04 to facilitate building remarkable-aarch64 target.

@Frenzie

Frenzie commented Apr 21, 2025

Copy link
Copy Markdown
Member

That's not a merge blocker, that's a nightly blocker. ;-)

@NoaHimesaka1873

Copy link
Copy Markdown
Contributor Author

I made PR at koreader/virdevenv#117

@Frenzie
Frenzie merged commit 1599565 into koreader:master Apr 21, 2025
0xstillb pushed a commit to 0xstillb/koreader-thai that referenced this pull request May 9, 2026
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.

FR: Support for Remarkable Paper Pro

6 participants