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 bq/fnac device support #745

Merged
merged 1 commit into from
Oct 31, 2018
Merged

add bq/fnac device support #745

merged 1 commit into from
Oct 31, 2018

Conversation

pazos
Copy link
Member

@pazos pazos commented Oct 19, 2018

Since those devices are almost copies of kobos (w/softfloat eabi, without simd) most code is almost there.

I refactored common kobo/bq code to input-ntx.h Nah. bq devices don't wake up when power is applied via usb.

It builds ok w/ koxtoolchain cervantes target or with a ubuntu/linaro toolchain (given that gcc > 4.8 and glibc <= 2.13)

TODO:

  • FBInk needs a bump. (current binary works, but breaks stock application on some circumstances)
    build a Docker image, need help on that. thanks @Frenzie

@NiLuJe
Copy link
Member

NiLuJe commented Oct 19, 2018

(next FBInk release probably sometime next week, because I'm expecting doom & misery with the Forma and its magic rotation handling...).

@Frenzie
Copy link
Member

Frenzie commented Oct 19, 2018

@pazos On Ubuntu you can just install Docker and for the rest you can find the relevant commands in the Makefiles and scripts.

https://github.com/koreader/virdevenv/blob/13c37dcc21d6e8acffe53e0d88e2a6e3562bd169/docker/ubuntu/kokobo/Makefile

You can probably just copy the Kobo almost wholesale.

Uploading it to Docker cloud is a touch trickier in the sense that you need an account, but otherwise also covered by the Makefile. :-)

@pazos
Copy link
Member Author

pazos commented Oct 25, 2018

Pinging @Frenzie @NiLuJe for review.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

What happens, exactly, with FBInk and the stock app?


re = ue_init_listener(&listener);
if (re < 0) {
fprintf(stderr, "[ko-input]: Failed to initilize libue listener, err: %d\n", re);
Copy link
Member

Choose a reason for hiding this comment

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

initialize ;)

NOTE: It's not actually your typo, so if you can fix the same one in input-kobo while you're there, that'd be great ;).

Copy link
Member Author

Choose a reason for hiding this comment

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

do you mean re > 0 ?

Copy link
Member

Choose a reason for hiding this comment

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

No, I meant initilize -> initialize in the fprintf ;).

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, that makes sense 👍

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

Generally LGTM otherwise, besides the small inherited typo ;).

@pazos
Copy link
Member Author

pazos commented Oct 25, 2018

What happens, exactly, with FBInk and the stock app?

FBInk v1.7.0 for Kobo [Minimalistic build], fbink works nice, clearing the screen with fbink -c "" breaks QBookApp - no more e-ink refreshes, the program keeps polling until 100% cpu and breaks my telnet connection

Didn't strace it, stock reader is completely broken in developers firmware and sometimes crashes by itself.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

With that version output, you know I'm going to ask if that happens with a proper Cervantes build ;).

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

But that sounds like a 0x0 region manages to get sent to the ioctl, which shouldn't happen, especially with clear, which enforces a full-screen region...

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

After a quick check, nope, the 0x0 region check looks solid, and Cervantes builds don't appear to bypass it (nothing does, really).

The only other thing I can think of is that something doesn't like clear's memset on smem_len, so you can try to replace https://github.com/NiLuJe/FBInk/blob/8d6c857eccf2dec0aa974a8837d355c6e14fa9f5/fbink.c#L458 by memset(fbPtr, v, fInfo.line_length * vInfo.yres_virtual);, and see if that helps?

We've established that that smem_len is weird on 16bpp Kobos already, but it doesn't appear to cause any problems on my end...

@NiLuJe
Copy link
Member

NiLuJe commented Oct 25, 2018

Did that anyway in NiLuJe/FBInk@0ae5dbc, because it can't hurt ;).

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.

lgtm (besides what's already been pointed out)

@pazos pazos force-pushed the cervantes2 branch 2 times, most recently from 47c4299 to 9c723b0 Compare October 25, 2018 16:34
@pazos
Copy link
Member Author

pazos commented Oct 25, 2018

@NiLuJe: it seems that fbink fails on travis because we need to specify C99. That happens when using linaro toolchains but not using koxtoolchain. Pinging @Frenzie too because it seems that the docker image is relying on a toolchain other than arm-cervantes-linux-gnueabi.

@pazos
Copy link
Member Author

pazos commented Oct 25, 2018

Did that anyway in NiLuJe/FBInk@0ae5dbc, because it can't hurt ;).

Will test that later. I debootstrapped a new rootfs just for the fun and have no access to stock reader app now 🙈

@Frenzie
Copy link
Member

Frenzie commented Oct 25, 2018

Pinging @Frenzie too because it seems that the docker image is relying on a toolchain other than arm-cervantes-linux-gnueabi.

I merely built and uploaded the Docker image you provided. ;-)

I can sh into the Docker image this weekend, but for the moment I'm more suspicious of your Makefile here.

@pazos
Copy link
Member Author

pazos commented Oct 25, 2018

I can sh into the Docker image this weekend, but for the moment I'm more suspicious of your Makefile here.

Not sure if it works but I opened a PR in virdevenv.

@pazos
Copy link
Member Author

pazos commented Oct 26, 2018

I reverted the mxcfb headers to match the behaviour of stock reader app (which is pretty conservative and does the same for all devices, see:https://github.com/bq/cervantes-qt/blob/eink-imx508/src/plugins/gfxdrivers/einkfb/linux/mxcfb.h) and squashed commits.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 26, 2018

Okay, after taking a look at https://github.com/bq/cervantes-qt/blob/eink-imx508/src/plugins/gfxdrivers/einkfb/einkfb.cpp, I can see how my memset could potentially break Qt's little mind ;).

The current approach probably still does, I'll see about that tomorrow...

@NiLuJe
Copy link
Member

NiLuJe commented Oct 27, 2018

The clear screen issue should be taken care of in NiLuJe/FBInk@10beff8 ;).

@Frenzie
Copy link
Member

Frenzie commented Oct 27, 2018

@pazos @NiLuJe Is this PR done now?

@pazos
Copy link
Member Author

pazos commented Oct 27, 2018

@Frenzie: I would like to submit a couple more changes in the waveform_modes table since current behaviour with new devices is far from perfect. Give me a couple of hours/days 😄

@Frenzie
Copy link
Member

Frenzie commented Oct 27, 2018

No problem. This device can also do OTA updates, right? (See koreader/koreader-misc#19)

@pazos
Copy link
Member Author

pazos commented Oct 30, 2018

pinging @NiLuJe about the REAGL stuff. I tried a bunch of flags based on what we are doing on other devices but the result is not good. Also tried newer mxc_wait_for_update_complete ioctl calls without success (I mean it works but doesn't improve anything, making the UI feels slower).

@Frenzie from my end I'm done with this PR, so if you think it is ready to be merged then I will focus on submit a proper frontend stuff.

@NiLuJe
Copy link
Member

NiLuJe commented Oct 30, 2018

Yeah, I doubt you could get anything great with explicit REAGL calls, only the Aura did on Kobo's side, and even then, it was fairly weird.

Makefile.defs Outdated
else ifeq ($(TARGET), cervantes)
ARM_ARCH:=$(ARMV7_GENERIC_ARCH)
ARM_ARCH+=-mfloat-abi=softfp
ifeq ($(shell PATH='$(PATH)' $(CC) -dumpmachine 2>/dev/null), arm-linux-gnueabi)
Copy link
Member

Choose a reason for hiding this comment

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

inconsistent indentation

You generally shouldn't use spaces in Makefiles.

Copy link
Member Author

Choose a reason for hiding this comment

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

same identation as other devices, like kindlepw2, What I should do?

Copy link
Member

Choose a reason for hiding this comment

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

It's not. The other devices use tab as they should while these here are spaces.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Frenzie Could you please point me to your text editor?. I opened in vim. First two lines are tabs, the ifeq/endif stuff just spaces. The prstux stuff contains spaces too.

Can't fix an error if I can't see it 😢

Copy link
Member

@NiLuJe NiLuJe Oct 31, 2018

Choose a reason for hiding this comment

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

The kindle & kindlepw2 indents are actually also wrong, which might be the source of the confusion ;).

Note that the space/tab distinction is actually a bit of a mess in this file in general.
I wholeheartedly agree with @Frenzie, which is why I think the few bits with tabs are mostly mine, and the bits with spaces are whatever was there before ;^p.

Copy link
Member

Choose a reason for hiding this comment

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

See the android case for one done right ;).

Copy link
Member

Choose a reason for hiding this comment

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

@pazos

First two lines are tabs, the ifeq/endif stuff just spaces. The prstux stuff contains spaces too.

So prstux is wrong too. ;-) But please leave fixing the others for a separate PR.

@NiLuJe

See the android case for one done right ;).

By pure coincidence that's the one I looked at. (It also happens to be the one I'm responsible for.)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, scratch that I suppose. I see you already fixed it. It'd be cleaner separately though.

Copy link
Member Author

Choose a reason for hiding this comment

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

No problem, reverted Makefile.defs and squashed again. 👍

@@ -23,7 +23,7 @@ ep_get_binary_dir(BINARY_DIR)
# but fails to actually honor the specified/detected Python version in every cases,
# sometimes just hoping there's a python binary somewhere in $PATH (which, granted, there should be)...
# Enforce that to python3 for containers where that's the only available Python version.
if(($ENV{KINDLE}) OR ($ENV{KOBO}) OR ($ENV{APPIMAGE}))
if(($ENV{KINDLE}) OR ($ENV{KOBO}) OR ($ENV{CERVANTES}) OR ($ENV{APPIMAGE}))
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I'm too blame for putting AppImage at the end. Could you please fix it?

@@ -31,9 +31,9 @@ set(CFG_OPTS "${CFG_OPTS} -DCMAKE_C_COMPILER='${CMAKE_C_COMPILER}' -DCMAKE_C_COM
set(CFG_OPTS "${CFG_OPTS} -DCMAKE_EXE_LINKER_FLAGS='${LDFLAGS}'")

# c.f., http://trac.ak-team.com/trac/browser/niluje/Configs/trunk/Kindle/Misc/CMakeCross.txt
if((DEFINED ENV{KINDLE}) OR (DEFINED ENV{LEGACY}) OR (DEFINED ENV{KOBO}) OR (DEFINED ENV{POCKETBOOK}) OR (DEFINED ENV{UBUNTUTOUCH}) OR (DEFINED ENV{SONY_PRSTUX}))
if((DEFINED ENV{KINDLE}) OR (DEFINED ENV{LEGACY}) OR (DEFINED ENV{KOBO}) OR (DEFINED ENV{POCKETBOOK}) OR (DEFINED ENV{UBUNTUTOUCH}) OR (DEFINED ENV{SONY_PRSTUX}) OR (DEFINED ENV{CERVANTES}))
Copy link
Member

Choose a reason for hiding this comment

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

Just adding things to the end works up to the point where it becomes illegible. ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

switching to two lines then

Copy link
Member

Choose a reason for hiding this comment

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

I mean alphabetical; my text editor auto-wraps just fine. :-)

set(CFG_OPTS "${CFG_OPTS} -DCMAKE_SYSTEM_NAME='Linux'")
endif((DEFINED ENV{KINDLE}) OR (DEFINED ENV{LEGACY}) OR (DEFINED ENV{KOBO}) OR (DEFINED ENV{POCKETBOOK}) OR (DEFINED ENV{UBUNTUTOUCH}) OR (DEFINED ENV{SONY_PRSTUX}))
endif((DEFINED ENV{KINDLE}) OR (DEFINED ENV{LEGACY}) OR (DEFINED ENV{KOBO}) OR (DEFINED ENV{POCKETBOOK}) OR (DEFINED ENV{UBUNTUTOUCH}) OR (DEFINED ENV{SONY_PRSTUX}) OR (DEFINED ENV{CERVANTES}))
Copy link
Member

Choose a reason for hiding this comment

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

Here too.

@Frenzie Frenzie merged commit f13f1ce into koreader:master Oct 31, 2018
@Frenzie
Copy link
Member

Frenzie commented Oct 31, 2018

Thanks!

Frenzie pushed a commit to koreader/koreader that referenced this pull request Oct 31, 2018
Adds support for devices found in https://blog.bq.com/es/bq-ereaders-developers-program/. Tested on BQ Cervantes 4 (last BQ device from 2017).

It adds a new touch input event handler (discussed in #4275) which should work on other single touch devices (ie: Kobo Touch, Mini, Glo, Aura HD) but wasn't tested.

Includes base bump with: [feat] Add BQ/Fnac device support (koreader/koreader-base#745)
@pazos pazos deleted the cervantes2 branch October 31, 2018 23:42
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

3 participants