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

[RFC] Remarkable port #1023

Merged
merged 14 commits into from Feb 2, 2020
Merged

[RFC] Remarkable port #1023

merged 14 commits into from Feb 2, 2020

Conversation

tcrs
Copy link
Contributor

@tcrs tcrs commented Dec 19, 2019

I have a remarkable (https://remarkable.com/). The builtin PDF reader was too slow so I did a quick port of koreader. This is mostly based on the Sony PRS port.

I've never worked on koreader before so would appreciate any comments etc.

I display stuff could be improved. I think the remarkable has a modern eink display with reagl and stuff so could probably use better paths for partial updates etc (xochitl, the interface the remarkable ships with, does a good job of low-latency not too flashy updates). I don't really know much about the eink driver stuff so I've just left it alone now things seem to be working...

There's also a pull request open over in koreader/koreader for the other bits and bobs.


This change is Reviewable

@tcrs tcrs changed the title Remarkable [RFC] Remarkable port Dec 19, 2019
#define _KO_INPUT_REMARKABLE_H

static void generateFakeEvent(int pipefd[2]) {
// TODO anything required here? Sony PRS port monitors battery charge etc
Copy link
Member

Choose a reason for hiding this comment

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

You can generate fake events that will be forwarded to the frontend as normal input events. There are a few defined in https://github.com/koreader/koreader-base/blob/master/input/input.c#L36-L41 but you can define more.

Normally is used to notify the program hardware changes, and we use libue to get that info from udev.

Once you have your fake events ready you can react to them in UIManager, but first do note that names are slightly different in the frontend. See how they're named in https://github.com/koreader/koreader/blob/f1f75c5cb0fa063c35f49a84b2e025f1d292ea01/frontend/device/input.lua#L176

Makefile.defs Outdated
@@ -151,6 +151,10 @@ else ifeq ($(TARGET), sony-prstux)
CHOST?=arm-linux-gnueabihf
export SONY_PRSTUX=1
export USE_LJ_WPACLIENT=1
else ifeq ($(TARGET), remarkable)
CHOST?=arm-kobo-linux-gnueabihf
Copy link
Member

Choose a reason for hiding this comment

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

Are you using the kobo toolchain from https://github.com/koreader/koxtoolchain?

Copy link
Member

Choose a reason for hiding this comment

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

The Kobo tc should work fine but may be targeting somewhat older features.

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 yea I had forgotten about that, I did use the kobo toolchain from koxtoolchain. I started using the toolchain provided by remarkable but it doesn't have a static libstdc++ library (which the koreader build seems to require).

I will take a look at creating a custom toolchain, I think I compared the kobo/remarkable SoCs and decided that there wasn't much difference in the hardware really, but the remarkable is running a much newer version of the Linux kernel so possibly it's worth building a custom toolchain for that reason.

Copy link
Member

Choose a reason for hiding this comment

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

remarkable's cpuinfo is exactly the same as the Kobo Forma. Not sure about kernel/libc versions in these devices and if is possible to make a new target for both and/or if makes sense. Pinging @NiLuJe

Copy link
Member

@NiLuJe NiLuJe Dec 28, 2019

Choose a reason for hiding this comment

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

Oh, then you're in luck ;).

It's not plugged in the koxtoolchain frontend, but the 1.24 branch of my ct-ng tree has a kobomk7 sample, which should be much closer to that ;).

Copy link
Member

Choose a reason for hiding this comment

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

You'll just have to tweak the minimum kernel version (or not, as the Mk7 one is lower and still 4.1.x), and double-check the glibc version, as I'm not sure what poky 2.1.3 was using.

Probably something along those lines, since that's similar the the current Kindle (lab126) TC, IIRC.

Copy link
Member

@NiLuJe NiLuJe Dec 29, 2019

Choose a reason for hiding this comment

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

Err, now that I think about it, probably best to take it as inspiration only, because it's using mainline GCC instead of the final Linaro GCC 7.5 release, and GCC 8/9 (possibly 10) is still suffering from severe neon performance regressions.

Copy link
Member

@pazos pazos left a comment

Choose a reason for hiding this comment

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

At first glance everything looks good. I dunno about that Archlinux change with luarocks will break older debian/ubuntu luarocks versions. Pinging @NiLuJe

Since the Remarkable runs plain linux you might want to port FBInk too. FBInk is used to give feedback on certain actions, like OTA updates. It shouldn't take you too long.

The same with the toolchain. You can pick one of the availables or create one for your device.

BTW, welcome aboard 😄

@Frenzie
Copy link
Member

Frenzie commented Dec 19, 2019

For reference, the other PR is koreader/koreader#5697.

Makefile Outdated Show resolved Hide resolved
Makefile.defs Outdated
@@ -151,6 +151,10 @@ else ifeq ($(TARGET), sony-prstux)
CHOST?=arm-linux-gnueabihf
export SONY_PRSTUX=1
export USE_LJ_WPACLIENT=1
else ifeq ($(TARGET), remarkable)
CHOST?=arm-kobo-linux-gnueabihf
Copy link
Member

Choose a reason for hiding this comment

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

The Kobo tc should work fine but may be targeting somewhat older features.

Makefile.third Outdated
@@ -682,7 +682,7 @@ $(LUA_SPORE_ROCK): $(THIRDPARTY_DIR)/lua-Spore/*.*
$(CMAKE) $(CMAKE_FLAGS) -DOUTPUT_DIR="$(CURDIR)/$(OUTPUT_DIR)" \
-DLUA_SPORE_VER=$(LUA_SPORE_VER) -DLD="$(CC)" \
-DCC="$(CC)" -DCFLAGS="$(CFLAGS) -I$(LUAJIT_DIR)/src" \
-DLUAROCKS=$(if $(DARWINHOST),"luarocks --lua-dir=/usr/local/opt/lua@5.1",luarocks) \
-DLUAROCKS=$(if $(DARWINHOST),"luarocks --lua-dir=/usr/local/opt/lua@5.1","luarocks --lua-version=5.1") \
Copy link
Member

Choose a reason for hiding this comment

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

This looks potentially suspicious; does it work on 2.x or is that just 3.x?

Copy link
Member

Choose a reason for hiding this comment

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

That's 3.x only, IIRC.

(For ref., I still patch my local copy w/ http://trac.ak-team.com/trac/browser/niluje/Configs/trunk/Kindle/Misc/koreader-luarocks-3.patch because I'm way too lazy to figure out what's got its panties in a twist ;)).

@NiLuJe
Copy link
Member

NiLuJe commented Dec 20, 2019

IIRC, there's some fancy undocumented (on their side) flags for their specific tweaks to waveform shenanigans.

Most of it is detailed in the libremarkable sources, if memory serves ;).

In any case, for a start, that should be perfectly fine enough. I don't actually recall the extent of their tweaks and if they'd actually be useful here anyway ;).
(I seem to remember a fancier A2 which we probably won't ever care for, for instance. If, on the other hand, there's a fancier REAGL and/or making it behave requires some hoop-jumping like on some platforms (hi, Kindle), that'd be something worth looking into for later ;)).

Take that with a grain of salt, I've never ever seen a Remarkable, and I've only ever quickly skimmed libremarkable's code ;).

@tcrs
Copy link
Contributor Author

tcrs commented Jan 27, 2020

I've added event generation for USB plug/unplug and battery charging/discharging. Looks like there is a delay of up to a minute or two for the battery status to change after plug/unplug (watching the uevents with udevadm monitor shows the same behaviour so I'm pretty sure it's not something wrong in the input code - don't know if this is normal on other devices or if it's specific to the remarkable.

@Frenzie
Copy link
Member

Frenzie commented Jan 27, 2020

You mean like this? I never really looked into the raw stuff.

if (uev.action == UEVENT_ACTION_ADD
&& uev.devpath
&& (UE_STR_EQ(uev.devpath, KOBO_USB_DEVPATH_PLUG)
|| UE_STR_EQ(uev.devpath, KOBO_USB_DEVPATH_HOST))) {
ev.code = CODE_FAKE_CHARGING;
} else if (uev.action == UEVENT_ACTION_REMOVE
&& uev.devpath
&& (UE_STR_EQ(uev.devpath, KOBO_USB_DEVPATH_PLUG)
|| UE_STR_EQ(uev.devpath, KOBO_USB_DEVPATH_HOST))) {
ev.code = CODE_FAKE_NOT_CHARGING;
} else {
continue;
}

36771d5

@NiLuJe
Copy link
Member

NiLuJe commented Jan 27, 2020

If the plug/unplug events themselves are on time, I wouldn't worry about it too much ;).

But speaking of battery, there is sometimes a different state for "plugged and charging" vs. "plugged and not charging". Not sure if that's relevant here, though ;).

@tcrs
Copy link
Contributor Author

tcrs commented Jan 28, 2020

I generate the plug/unplug events from the charger, which seem to be instantaneous. The charging/not-charging comes from the battery device, and its state seems slow to change. So the plugged and charging status are separate. The state usually goes something like:

plugged
...delay..
charging
...
unplugged
...delay...
not-charging

But you could get plugged+not-charging if the battery is full probably.

Looks like kindle & kobo have quite different device setup for usb plug/unplug.

I also noticed that the Sony PRS port has a custom input generation function in input-sony-prstux.h (which I started from to make the remarkable one), but never opens the fake_event input in its frontend device lua file so I don't think it will ever actually be used (in koreader at least).

@pazos
Copy link
Member

pazos commented Jan 29, 2020

I also noticed that the Sony PRS port has a custom input generation function in input-sony-prstux.h (which I started from to make the remarkable one), but never opens the fake_event input in its frontend device lua file so I don't think it will ever actually be used (in koreader at least).

It is in koreader/koreader#4567

Having support for the SonyPRSTux platform is great. But the support wasn't intended for other people outside its contributor. We can safely assume the number of users are <= 1. Maybe I'm wrong, but we lack:

  • toolchain definition
  • platform instructions to install the software over OEM.

So the only person who is able to give feedback about its state is @v01d and we're not in a hurry to merge PRs because we're sure the author of the port is able to figure out and fix things, even when the fixes never reach this repo.

@@ -441,6 +452,12 @@ local function refresh_pocketbook(fb, refreshtype, waveform_mode, x, y, w, h)
return mxc_update(fb, C.MXCFB_SEND_UPDATE, refarea, refreshtype, waveform_mode, x, y, w, h)
end

local function refresh_remarkable(fb, refreshtype, waveform_mode, x, y, w, h)
local refarea = ffi.new("struct mxcfb_update_data[1]")
refarea[0].temp = C.TEMP_USE_AMBIENT
Copy link
Member

Choose a reason for hiding this comment

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

To match xochitl:

Suggested change
refarea[0].temp = C.TEMP_USE_AMBIENT
if waveform_mode == C.WAVEFORM_MODE_DU then
refarea[0].temp = C.TEMP_USE_REMARKABLE
else
refarea[0].temp = C.TEMP_USE_AMBIENT
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 693 to 720
self.waveform_fast = C.WAVEFORM_MODE_DU
self.waveform_ui = C.WAVEFORM_MODE_AUTO
self.waveform_flashui = self.waveform_ui
self.waveform_full = C.WAVEFORM_MODE_GC16
self.waveform_partial = C.WAVEFORM_MODE_AUTO
self.waveform_night = C.WAVEFORM_MODE_GC16
self.waveform_flashnight = self.waveform_night
self.night_is_reagl = false
Copy link
Member

Choose a reason for hiding this comment

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

To match xochitl:

Suggested change
self.waveform_fast = C.WAVEFORM_MODE_DU
self.waveform_ui = C.WAVEFORM_MODE_AUTO
self.waveform_flashui = self.waveform_ui
self.waveform_full = C.WAVEFORM_MODE_GC16
self.waveform_partial = C.WAVEFORM_MODE_AUTO
self.waveform_night = C.WAVEFORM_MODE_GC16
self.waveform_flashnight = self.waveform_night
self.night_is_reagl = false
self.waveform_fast = C.WAVEFORM_MODE_DU
self.waveform_ui = C.WAVEFORM_MODE_GL16
self.waveform_flashui = C.WAVEFORM_MODE_GC16
self.waveform_full = C.WAVEFORM_MODE_GC16
self.waveform_partial = C.WAVEFORM_MODE_GL16
self.waveform_night = C.WAVEFORM_MODE_GC16
self.waveform_flashnight = self.waveform_night
self.night_is_reagl = false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@tcrs
Copy link
Contributor Author

tcrs commented Feb 1, 2020

Thanks @NiLuJe for sorting out the waveform mapping & related stuff.

I can remove the luarocks --lua-version change as it sounds like that isn't suitable for the build setup expected (I'll keep it locally for my machine). Is it OK to rebase the branch this pull request is from?

@Frenzie
Copy link
Member

Frenzie commented Feb 1, 2020

Should be fine, here at least. :-)

tcrs and others added 11 commits February 1, 2020 12:16
Mostly copied from sony-prstux port
Assumes the koreader systemd unit is installed correctly. Launched
koreader when the middle (home) button is held for 3 seconds.

Inspired by https://github.com/dixonary/button-capture-reMarkable
but significantly simpler.
USB Plug/Unplug, charge status change. Seems to take a minute or two
from USB Unplug to the battery state changing to discharging in sysfs.
Rather than reusing kobo toolchain. Built from koxtoolchain.
After some testing NiLuJe has figured out the proper mapping

Removes a bunch of modes which are just unusable
@tcrs
Copy link
Contributor Author

tcrs commented Feb 1, 2020

Rebased on latest master, removed --lua-version build change

Makefile Outdated
@@ -29,7 +29,8 @@ all: $(OUTPUT_DIR)/libs $(if $(ANDROID),,$(LUAJIT)) \
$(if $(or $(DARWIN),$(WIN32),$(ANDROID),$(UBUNTUTOUCH),$(APPIMAGE)),,$(OUTPUT_DIR)/dropbear) \
$(if $(or $(KINDLE),$(KOBO),$(CERVANTES)),$(OUTPUT_DIR)/sftp-server,) \
$(if $(or $(DARWIN),$(WIN32)),,$(OUTPUT_DIR)/tar) \
$(if $(or $(KINDLE),$(KOBO),$(CERVANTES)),$(OUTPUT_DIR)/fbink,) \
$(if $(or $(KINDLE),$(KOBO),$(CERVANTES),$(REMARKABLE)),$(OUTPUT_DIR)/fbink,) \
Copy link
Member

Choose a reason for hiding this comment

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

That being said, my order comment applies more broadly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reordered

Makefile Outdated
@@ -95,7 +97,7 @@ libs: \
$(OUTPUT_DIR)/libs/libkoreader-input.so: input/*.c input/*.h $(if $(KINDLE),$(POPEN_NOSHELL_LIB),)
@echo "Building koreader input module..."
$(CC) $(DYNLIB_CFLAGS) -I$(POPEN_NOSHELL_DIR) -I./input \
$(if $(KOBO),-DKOBO,) $(if $(KINDLE),-DKINDLE,) $(if $(POCKETBOOK),-DPOCKETBOOK,) $(if $(SONY_PRSTUX),-DSONY_PRSTUX,) $(if $(CERVANTES),-DCERVANTES,)\
$(if $(KOBO),-DKOBO,) $(if $(KINDLE),-DKINDLE,) $(if $(POCKETBOOK),-DPOCKETBOOK,) $(if $(REMARKABLE),-DREMARKABLE,) $(if $(SONY_PRSTUX),-DSONY_PRSTUX,) $(if $(CERVANTES),-DCERVANTES,)\
Copy link
Member

Choose a reason for hiding this comment

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

Also here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reordered

set(INSTALL_CMD3 ${CMAKE_COMMAND} -E rename ${SOURCE_DIR}/Release/fbdepth ${BINARY_DIR}/fbdepth)
endif()
endif()

ko_write_gitclone_script(
GIT_CLONE_SCRIPT_FILENAME
https://github.com/NiLuJe/FBInk.git
tags/v1.20.3
origin/master
Copy link
Member

Choose a reason for hiding this comment

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

We really do prefer tags or specific commits.

Copy link
Member

Choose a reason for hiding this comment

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

master is a moving target. In this case we can prod @NiLuJe to create a new tag, generally speaking just use a hash like b06bd399611875fe98b52e5fdadd6525872dde5c.

Copy link
Member

Choose a reason for hiding this comment

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

I've just tagged v1.21.0 ;).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated to use v1.21.0

@NiLuJe
Copy link
Member

NiLuJe commented Feb 1, 2020 via email

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

Nothing jumps out at me; I'll leave merging to @NiLuJe ;-)

@NiLuJe
Copy link
Member

NiLuJe commented Feb 2, 2020

And we're cooking with gas! ;).

Thanks!

@NiLuJe NiLuJe merged commit 45347f7 into koreader:master Feb 2, 2020
NiLuJe added a commit to NiLuJe/koreader that referenced this pull request Feb 4, 2020
* Re-enable HW dithering on Kindle
  (koreader/koreader-base#1034)
* Update SQLite to 3.31.1
  (koreader/koreader-base#1035)
* reMarkable port
  (koreader/koreader-base#1023)
* zsync2
  (koreader/koreader-base#1036)
NiLuJe added a commit to koreader/koreader that referenced this pull request Feb 4, 2020
* Switch to zsync2

Requires koreader/koreader-base#1036

* Simplify FBInk syntax

The weird-ass workarounds for -s's subopts handling are
no longer needed w/ FBInk >= 1.21.0

* Update base

* Re-enable HW dithering on Kindle
  (koreader/koreader-base#1034)
* Update SQLite to 3.31.1
  (koreader/koreader-base#1035)
* reMarkable port
  (koreader/koreader-base#1023)
* zsync2
  (koreader/koreader-base#1036)

* zsync2 means we can finally have nice things

(OpenStack backed storage)

* We also no longer need that insane workaround on ARM

* And comment out @chrox's mirror, which appears to be down.

* Warn that a malformed URL will horribly blow up in fun and interesting
ways
mwoz123 pushed a commit to mwoz123/koreader that referenced this pull request Mar 29, 2020
* Switch to zsync2

Requires koreader/koreader-base#1036

* Simplify FBInk syntax

The weird-ass workarounds for -s's subopts handling are
no longer needed w/ FBInk >= 1.21.0

* Update base

* Re-enable HW dithering on Kindle
  (koreader/koreader-base#1034)
* Update SQLite to 3.31.1
  (koreader/koreader-base#1035)
* reMarkable port
  (koreader/koreader-base#1023)
* zsync2
  (koreader/koreader-base#1036)

* zsync2 means we can finally have nice things

(OpenStack backed storage)

* We also no longer need that insane workaround on ARM

* And comment out @chrox's mirror, which appears to be down.

* Warn that a malformed URL will horribly blow up in fun and interesting
ways
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

Successfully merging this pull request may close these issues.

None yet

4 participants