Implement nickel_orientation action

[pgaskin]

closes #67

[pgaskin]

So far, this works fine with ForceAllowLandscape enabled on devices without an orientation sensor. @stiivgiaco, you can test this now if you want (KoboRoot.tgz).

menu_item:main:Portrait:nickel_orientation:portrait
menu_item:main:Landscape:nickel_orientation:landscape
menu_item:main:InvPortrait:nickel_orientation:inverted_portrait
menu_item:main:InvLandscape:nickel_orientation:inverted_landscape
menu_item:reader:Portrait:nickel_orientation:portrait
menu_item:reader:Landscape:nickel_orientation:landscape
menu_item:reader:InvPortrait:nickel_orientation:inverted_portrait
menu_item:reader:InvLandscape:nickel_orientation:inverted_landscape

[stiivgiaco]

installing as we speak to my kobo clara 👍 thanks in advance @pgaskin

[stiivgiaco]

@pgaskin thanks so much for this! I have installed and tested it on my Kobo Clara HD.

All orientation options work however if a user wants to go from portrait to landscape, he/she will have to switch the orientation through the ForceAllowLandscape menu first. Meaning trying to switch from Portrait (using menu_item:main:Portrait:nickel_orientation:portrait) to Landscape (using
menu_item:main:Landscape:nickel_orientation:landscape) doesn't switch the orientation.

[pgaskin]

I have installed and tested it on my Kobo Clara HD.

Thanks for testing it!

to switch the orientation through the ForceAllowLandscape menu first

That's a known issue (see the comments in the code review above). I'll probably be adding some messages to help users who don't realize that.

[stiivgiaco]

Hadnt realised that there was the sleep functionality also implemented. Just added that to the config file and also works flawlessly. You rock!

[pgaskin]

I've tested this with:

Devices:

• Mini (no sensor), 4.13.12638
• Aura2 (no sensor), 4.22.15268
• ClaraHD (no sensor), 4.22.15268

Config:

• ForceAllowLandscape
• ForceAllowLandscape and patched
• Patched

Config:

menu_item:main:Portrait:nickel_orientation:portrait
menu_item:main:Landscape:nickel_orientation:landscape
menu_item:main:InvPortrait:nickel_orientation:inverted_portrait
menu_item:main:InvLandscape:nickel_orientation:inverted_landscape
menu_item:main:InvertPL:nickel_orientation:invert
menu_item:main:SwapPL:nickel_orientation:swap
menu_item:reader:Portrait:nickel_orientation:portrait
menu_item:reader:Landscape:nickel_orientation:landscape
menu_item:reader:InvPortrait:nickel_orientation:inverted_portrait
menu_item:reader:InvLandscape:nickel_orientation:inverted_landscape
menu_item:reader:InvertPL:nickel_orientation:invert
menu_item:reader:SwapPL:nickel_orientation:swap

---

It still needs testing on a device with an orientation sensor (I don't have any of those) with and without ForceAllowLandscape (both without the rotation patch). The main thing to check is that the orientation sensor is overridden until a different rotation option (try it with auto rotation + select inverted_portrait, auto rotation + select swap, and with locked portrait + select inverted_landscape) is selected from the rotation pop-up (the sensor should work again after that without a reboot). (that's 6 different test cases and 1 config file change halfway through).

[NiLuJe]

Without ForceAllowLandscape:

• It overrides the gyro very much so: once an orientation is set, it'll stick in the current view no matter what: if you rotate the device, that'll only take effect if you swap view (i.e., Home <-> Library).

• Swapping in the Library (from Portrait, auto-rota) enables the Landscape lock in the reader.

• Which means a swap from Portrait-lock enables the Landscape lock. Again, gyro is useless in the current view (In the Reader, breaking the lock by switching to auto-rota re-enables the gyro. But before that, you couldn't even invert via gyro).
  (Icon is updated, FWIW).

---

With ForceAllowLandscape:

• Unrelated, but the Rota menu is "sticky" on the Homescreen: you have to select an option to dismiss it, tapping outside doesn't do so.

• This time, the rotation sticks permanently. I have to re-tap "auto-rotate" to actually get the gyro working. (And the icon isn't updated, i.e., that was the first test: auto -> InvP, so icon is till set to auto).

• Same behavior as previously with the Swap. Lock is enabled, gyro is down for the count, even across views (re-tapping the current lock restores the gyro, much like tapping auto-rota).
  Sidebar: the (patched) two row version of the Homescreen looks pretty neat in landscape.

• From the Home-screen, a swap from Portrait lock does NOT enable the Landscape lock, but the gyro is also stuck.
  (Huh. Trying this again, it does enable Landscape lock now o_O).
  Let's assume that was me getting cross-eyed the first time (or missclicking somewhere :D).

[NiLuJe]

So, short of the weird "not quite sticky across views" without the setting, that works pretty much as intended, I guess?

[pgaskin]

if you rotate the device, that'll only take effect if you swap view (i.e., Home <-> Library).

Can you clarify what you mean by this?

Which means a swap from Portrait-lock enables the Landscape lock. Again, gyro is useless in the current view (In the Reader, breaking the lock by switching to auto-rota re-enables the gyro. But before that, you couldn't even invert via gyro).

So that's working as intended (I eventually decided to lock it and disable invert). If you think it's a good idea, I can allow gyro inversion to work for portrait/landscape (or have an option which allows it).

(Icon is updated, FWIW).

Yes, that makes sense, since we don't actually disable the sensor (it will still attempt to change stuff and update the icon), just prevent it's readings from having any effect by limiting the mask to our new rotation.

[NiLuJe]

if you rotate the device, that'll only take effect if you swap view (i.e., Home <-> Library).

Which means a swap from Portrait-lock enables the Landscape lock. Again, gyro is useless in the current view (In the Reader, breaking the lock by switching to auto-rota re-enables the gyro. But before that, you couldn't even invert via gyro).

So that's working as intended (I eventually decided to lock it and disable invert). If you think it's a good idea, I can allow gyro inversion to work for portrait/landscape (or have an option which allows it).

(Icon is updated, FWIW).

Yes, that makes sense, since we don't actually disable the sensor (it will still attempt to change stuff and update the icon), just prevent it's readings from having any effect by limiting the mask to our new rotation.

Since the icon matches the usual "manual" lock state, it vaguely makes sense to me that the behavior should match too ;).

That said, I'm absolutely not the guy to ask about actual experience with this: I pretty much do everything in Portrait ^^.

[pgaskin]

Unrelated, but the Rota menu is "sticky" on the Homescreen: you have to select an option to dismiss it, tapping outside doesn't do so.

I've noticed that before... it's quite an unusual menu (remember the sizing issues we had a while back with the rotation patch?).

This time, the rotation sticks permanently. I have to re-tap "auto-rotate" to actually get the gyro working. (And the icon isn't updated, i.e., that was the first test: auto -> InvP, so icon is till set to auto).

Yes, this makes sense, as it's Nickel which enforces the restrictions for views (we set it either way, so it just only takes effect when the current view allows it). Regarding the gyro being disabled completely, see my above comment ("If you think it's a good idea, I can allow gyro inversion to work for portrait/landscape (or have an option which allows it)").

Same behavior as previously with the Swap. Lock is enabled, gyro is down for the count, even across views (re-tapping the current lock restores the gyro, much like tapping auto-rota).

That's good, it's what I intended.

From the Home-screen, a swap from Portrait lock does NOT enable the Landscape lock, but the gyro is also stuck.
(Huh. Trying this again, it does enable Landscape lock now o_O).
Let's assume that was me getting cross-eyed the first time (or missclicking somewhere :D).

Yes, that sounds like a misclick, as I didn't see anything which would cause this to happen.

[pgaskin]

Since the icon matches the usual "manual" lock state, it vaguely makes sense to me that the behavior should match too ;).

Yes, I guess there wouldn't usually be much reason (if any) to want to force an upside-down display if the device is upright.

That said, I'm absolutely not the guy to ask about actual experience with this: I pretty much do everything in Portrait ^^.

I can confirm that's how it would work, as I noticed it during my testing and while looking at libnickel.

I usually do stuff in Portrait unless I'm reading something landscape or I'm writing a webapp for the browser (it's usually a lot more useful in landscape). For me, my main use for this action is inverted portrait when I want to hold it from the top side or when I have a cable plugged into it.

---

Regarding the fully-locked landscape/portrait modes, do you think it'd be a good idea to add an option to allow gyro inversions or vice versa?

[NiLuJe]

Regarding the fully-locked landscape/portrait modes, do you think it'd be a good idea to add an option to allow gyro inversions or vice versa?

Definitely (c.f., #70 (comment)) ;).

And since I apparently got ghost email notifications for stuff I can't actually see (the GH website has been wonky as hell this week): that was on a Forma ;).

[pgaskin]

And since I apparently got ghost email notifications for stuff I can't actually see (the GH website has been wonky as hell this week): that was on a Forma ;).

That comment was in the review above 😉 . I got caught by that bug with auto-scrolling to the comment yesterday.

Speaking of bugs in GitHub, the cache has been very wonky recently. Also, when switching between desktop/mobile views, it seems to take you to the previous page you were on. And, references like #70 appear to be triggered in code blocks sporadically (a \ doesn't even escape them properly, but this shouldn't be needed in the first place), and GH support can't seem to reproduce it consistently.

Definitely (c.f., #70 (comment)) ;).

Which would be better: an option calledgyro_invert or rotations modes called locked_landscape/locked_portrait ?

Edit: I just remembered one of the reasons why I decided to fully lock it. I haven't gone through everything, but it appears some gyro settings are changed when switching between auto and one of the other locked views. We can't easily reproduce these changes, and I'd rather not have to walk QObjects (see the comments in the code) to get the required object to call the proper change methods on. I might leave this for later if there is demand for it.

[pgaskin]

If there aren't any further objections or possible issues/unexpected behavior, I'll merge this later today.

I'm going to put aside the idea for options to change to the usual locked orientations (where it can still invert from the gyro) for now, as they add significant complexity and/or possible issues which I have no way of testing myself. If there's demand for it, I might consider implementing them. @NiLuJe, one last question: if you use nickel_orientation:landscape and then press the Locked Landscape option in the menu (without switching to portrait/auto first or changing views), does it allow you to invert it with the gyro?

[NiLuJe]

@NiLuJe, one last question: if you use nickel_orientation:landscape and then press the Locked Landscape option in the menu (without switching to portrait/auto first or changing views), does it allow you to invert it with the gyro?

Yep, it does :).

[pgaskin]

Yep, it does :).

That's good. I guess this is ready to be merged now.