Input/Device: Refactor Gyro events handling #9935

NiLuJe · 2022-12-20T05:38:18Z

Handle a single, custom protocol in Input, and move the platform-specific translation to Device implementations.

This allows us to get rid of a whole bunch of nastiness from Input, as the Kindle handling in particular was... not pretty ;).

Also, tweak how input events hooks can be built to allow for a single composite hook on Kobo, instead of repeating and nesting calls.

Only tested on Kobo so far (I don't have any other gyro-enabled device anyway ;p).

Depends on koreader/koreader-base#1568 (re: #9873)

Get rid of the canToggleGSensor Device cap, it's now mandatory for hasGSensor devices. (This means Kindles can now toggle the gyro, fix "Lock auto rotation to current orientation" is unchecked, grayed out #9136).
This means that Device:toggleGSensor is now implemented by Generic.
Update the Screen & Gyro rotation constants to be clearer (c.f., Groundwork for a refactor of gyro handling koreader-base#1568) (/!\ This might conceivably break some rotation_map user-patches).
Input: Move the platform-specific gyro handling to Device implementations, and let Input only handle a single, custom protocol (EV_MSC:MSC_GYRO).
Input: Refine the rotation_map disable method implemented in 43b021d. Instead of directly poking at the internal field, use a new method, disableRotationMap (/!\ Again, this might break some rotation_map user-patches).
Input: Minor tweaks to event adjust hooks to make them more modular, allowing the Kobo implementation to build and use a single composite hook. API compatibility maintained with wrappers.

This change is

frontend/device/input.lua

Frenzie

(approval relates to the principle, no opinion as to whether the Kindle stuff was correctly transplanted :-P)

NiLuJe

Reviewed 7 of 12 files at r1, 16 of 16 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pazos, @poire-z, and @rjd22)

rjd22

Looks good to me. After this is merged I will make and new PR for the PocketBook gyro detection. For enabling and disabling the gyro I will let users do that in the config menu of PocketBook itself. That will require less supporting code from our side.

NiLuJe · 2022-12-20T22:56:02Z

For enabling and disabling the gyro I will let users do that in the config menu of PocketBook itself.

If all goes well, the standard "ignore the events" approach implemented here should Just Work (TM) anyway ;).

We're unlikely to confuse them with KEY_ evdev constants, and those are in the FFI C namespace anyway.

tiiiiiiiny bit.

Get rid of the gyro toggle flag, use pointer comparisons to infer the state instead

It's meaningless, the translation is now mandatory.

Comment it instead it's just here for documentation anyway

anything platform-specific in there once I'm done

There are only a couple platforms where this will matter anyway ;).

It's only used by the debug codepath, which sits *before* translation

It's now mandatory if hasGSensor Move toggleGSensor to Generic, as it now *is* platform-agnostic ;).

(Still needs the hasGSensor cap, though. Left for koreader#9873 to deal with ;)).

But am going to write compat wrappers instead of doing this ;p

not the class object

Make sure it always exist (again), and instead move disabling it to an actual method, instead of having to fiddle with internal instance objects directly. Will break some user patches \o/.

We'll see them on PB, because event mangling happens before waitEvent (the whole thing is synthetic, instead of mutating a real live evdev feed via event hooks).

own framebuffer ones

them. "rotated" doesn't mean anything (in fact, "portrait rotated" sounds like landscape to me :s), and doesn't actually tell you *which way* it's been rotated. The fact that UR happens to be Portrait on almost every platform we care about is a quirk of fate, that wouldn't be true on a TV, so, get rid of that assumption, at least from the names ;o). (the code... still does ^^). TL;DR: As usual, LinuxFB had it right, so, follow its inspiration, but be more wordy to make it clear we're talking about how the *device* is physically rotated in the real world.

Let's hope for the best, I guess? :D.

koreader/koreader-base#1568

NiLuJe requested review from Frenzie, pazos and poire-z as code owners December 20, 2022 05:38

rjd22 reviewed Dec 20, 2022

View reviewed changes

frontend/device/input.lua Outdated Show resolved Hide resolved

Frenzie approved these changes Dec 20, 2022

View reviewed changes

NiLuJe commented Dec 20, 2022

View reviewed changes

rjd22 approved these changes Dec 20, 2022

View reviewed changes

NiLuJe added 22 commits December 21, 2022 15:10

Simplify constant names

b20a48f

We're unlikely to confuse them with KEY_ evdev constants, and those are in the FFI C namespace anyway.

Make Device implementations do more work to optimize input mangling a

8eceaf0

tiiiiiiiny bit.

Future-proof that

931b123

Start unifying gyro handling

f281a02

More refactoring...

b33623d

Get rid of the gyro toggle flag, use pointer comparisons to infer the state instead

Drop misc_ntx_gsensor_protocol

65fda58

It's meaningless, the translation is now mandatory.

Wire it in

75eba26

This is no longer bracketed in a luacheck ignore directive

da39f07

Comment it instead it's just here for documentation anyway

Speaking of those directives, drop 'em, because the goal is to not have

19bdb9d

anything platform-specific in there once I'm done

Do away with the EV_GYRO branch by pushing more work on the Device imp

c65ab5e

There are only a couple platforms where this will matter anyway ;).

That's gone ;)

4056f05

Drop those, too.

9aa9146

It's only used by the debug codepath, which sits *before* translation

Err, no, not SDL ;p.

caad833

There was a 142% chance of me fucking that one up on the first try ;p.

f78fcbf

Port the Kindle gyro nastiness to the new API

70db7b4

get rid of the canToggleGSensor Device cap

3003a86

It's now mandatory if hasGSensor Move toggleGSensor to Generic, as it now *is* platform-agnostic ;).

And move the handleGyroEv setup to generic, too

0446b27

Wire gyro handling for PocketBook

38fd52a

(Still needs the hasGSensor cap, though. Left for koreader#9873 to deal with ;)).

Forgot to port the input hook

bb82aa1

But am going to write compat wrappers instead of doing this ;p

There, better ;)

6ceb964

And the last one ;)

6f83c92

More comments

b70aa03

NiLuJe added 10 commits December 21, 2022 15:10

Minor review pass

d27155b

Move the default event_map & event_map_adapter objects to the instance,

65b6d75

not the class object

Rejig rotation_map handling

77f10e8

Make sure it always exist (again), and instead move disabling it to an actual method, instead of having to fiddle with internal instance objects directly. Will break some user patches \o/.

Add MSC_GYRO to the debug logging

6862fcf

We'll see them on PB, because event mangling happens before waitEvent (the whole thing is synthetic, instead of mutating a real live evdev feed via event hooks).

Don't nerf that comment ;).

a77dd08

Renamed the MSC_GYRO constants in preparation of uniformization with our

fd8d6a7

own framebuffer ones

That, err, looks like a typo?

99bd459

Let's hope for the best, I guess? :D.

Update the NTX MSC_RAW comment

bcafb69

Bump base

a28413d

koreader/koreader-base#1568

NiLuJe force-pushed the gyro-refactor branch from e667742 to a28413d Compare December 21, 2022 14:12

NiLuJe merged commit 788ccac into koreader:master Dec 21, 2022

Frenzie added this to the 2022.12 milestone Dec 21, 2022

NiLuJe mentioned this pull request Dec 25, 2022

Koreader page turn buttons do not switch with rotation #9223

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Input/Device: Refactor Gyro events handling #9935

Input/Device: Refactor Gyro events handling #9935

NiLuJe commented Dec 20, 2022 •

edited

Frenzie left a comment

NiLuJe left a comment

rjd22 left a comment •

edited

NiLuJe commented Dec 20, 2022

Input/Device: Refactor Gyro events handling #9935

Input/Device: Refactor Gyro events handling #9935

Conversation

NiLuJe commented Dec 20, 2022 • edited

Frenzie left a comment

Choose a reason for hiding this comment

NiLuJe left a comment

Choose a reason for hiding this comment

rjd22 left a comment • edited

Choose a reason for hiding this comment

NiLuJe commented Dec 20, 2022

NiLuJe commented Dec 20, 2022 •

edited

rjd22 left a comment •

edited