Return value of button_scan

[shermp]

So, I was testing button_scan with kobo-rclone this evening, and things just were not working right. After looking at the code, I realised what the problem is. There is currently now way to tell if a button was found or not based on the return code. I had naively expected it to return a negative number if no button was found.

The only time button_scan returns with anything other than EXIT_SUCCESS is if the button press "fails", or it isn't run on a Kobo.

I hate to suggest it, but maybe button_scan needs to return a "magic number" depending on the final state. Maybe a few are needed.

Thoughts?

[NiLuJe]

Yeah, at the very least it should throw -1 too... I'll take a look at it later today :).

[NiLuJe]

Okay, I went with -EXIT_FAILURE too, for a couple of reasons:

• The rest of the library sends error diagnostics to stderr, so I followed suit here.
• The failure codepath on button input will never actually be reached on current Kobo devices: we don't have any sane way of making sure a press was successful (short of re-checking screen content, which I'm not particularly keen on doing ;)), so the only thing than can fail is opening the input device, whose path hasn't changed for ever (unlike on Kindles, where getting the right one on a new device is always fun), so that -EXIT_FAILURE will actually probably never be reached by anyone, so no need to make it throw something custom.
• That leaves a single failure codepath, the no button found one, which was indeed missing, and now throws -EXIT_FAILURE too ;).

[NiLuJe]

Okay, I figure knowing if the tap failed is kind of interesting, so I did that with a double-scan after a few ms of sleeping...

Had to tweak the signature to inhibit logging completely, unfortunately :/.

Now returns -(ENOTSUP) when the tap failed (or appeared to, at least, tapping outside the popup will dismiss it, and we'll detect that as okay).

[NiLuJe]

Sidebar: My H2O reboots after a echo "usb plug remove" >| /tmp/nickel-hardware-status instead of recovering. (After having accepted a echo "usb plug add" >| /tmp/nickel-hardware-status prompt, which throws me in the unmounted state and where everything still works).

Might be because I'm in USBNet mode, though.

[NiLuJe]

Attempted to make that second screen-check smarter, by adapting it to the "Connected" screen (to avoid false-positives on popup dismiss).

That may translate to false-negatives on some devices instead, though, despite it doing the job on my H2O ;).

[NiLuJe]

Well, it works, but we need to sleep 5s instead of 100ms, so, not sure which one's preferable...

Relevant changeset: https://github.com/NiLuJe/FBInk/compare/6b1d5a1bad5b9a77ef675e7d98525b11ec17ee50...272430c04fa7f4e15ea2a6aa254039b09eb1af75

[shermp]

I wouldn't bother too much about testing whether the button press happened correctly or not, as there are other ways of testing,

For example, in kobo-rclone, I'm testing whether /mnt/onboard gets unmounted or not, if it doesn't, then I abort, as I have to assume the button press never happened.

[NiLuJe]

@shermp : Good point! I'll probably revert to the original naive test, because I'm decidedly not a fan of that 5s sleep...

[shermp]

I just had an idea with regards to testing the connected state:

The connected screen is mostly full black, including the edges, which are normally white. So, choose a pixel on the top or bottom edge (which usually have status bar etc), and test it in a loop until it turns black. (or times out)

Only downside to this method, is the need to choose a device specific pixel, but other than that, I would think it would be very reliable.

[NiLuJe]

Yeah, good idea, going with let's say the middle pixel of the final line is easy to compute (viewWidth/2, viewHeight) ands is pretty much guaranteed to flip from white to black everywhere.

That still leaves either a sleep or a timeout, though, but at least the test itself is potentially completely reliable, unlike my attempt at repurposing the button logic to the computer+cable pictogram ;).

[shermp]

I would go with a loop that tries x iterations, with a small (say 100-250ms) sleep between each iteration. Testing one pixel shouldn't take long I wouldn't have thought...

[NiLuJe]

Yeah, that's what I was aiming towards.

Plus, I get to avoid re-purposing & re-entering fbink_button_scan, so, yay!

[NiLuJe]

That seems to work pretty well, and indeed got rid of the annoying mangling of fbink_button_scan itself, thanks! :)

[shermp]

You're welcome. Looks like it should indeed do the trick.

[NiLuJe]

I went with an 8s timeout, which may be a bit large, given that it appeared to have only taken ~1.5s to get a black pixel during my tests...

[shermp]

Actually, just another thought. Is there ever a situation where the italic 'f' character in "Page x of x" overflows into the very bottom row of pixels?

If so, perhaps you the pixel to test should not be the centre, but offset a bit (say 40% or 60% of screen width)

[NiLuJe]

On which screen? In the library, there's a considerable margin before it hits bottom on my end. In the reader, I, err, never ever had it enabled, so I wouldn't really know :D...

But that leads me to another question: does the echo into the nickel pipe actually throw you back to the Homescreen?

[NiLuJe]

It doesn't, okay, so, yeah, the only thing I can think of that could put a black pixel there is:

kePub + full-screen reading mode + a veeery unfortunate broken cut-off section of letter.

[NiLuJe]

But once you start worrying about that, I don't think any other edges could be much safer, since with a patched FW, you can pretty much entirely eradicate margins on all four sides...

[shermp]

Of course, the counter argument being that the user will almost certainly be launching the script/program using kfmon, therefore have to be in the home/booklist screen anyway.

[NiLuJe]

True ;). I'll check with Large Print enabled for peace of mind, and the only other thing that might affect this is the "Bad Eyes" modification, but I've never used it...

EDIT: Still a healthy margin with LP ;).

EDIT²: At a very quick glance, the patch appears to keep a modicum of border, too :).

[NiLuJe]

I guess I could also hide the "sleepy" behavior behind an extra arg to fbink_button_scan, in case the behavior is unwanted? (i.e., skip the "connected" check).

[shermp]

Options never hurt :)

[shermp]

I got around to testing the changes you made for the original issue, and it all works fine here. Thanks!

With scan_button, I can now remove all that pesky touch hacks I had in kobo-rclone. I also don't need to check for model numbers now, so yay.

[NiLuJe]

And I just pushed a bunch of changes that made scanning faster on 32bpp fbs (... as a side-effect, the goal was faster blitting of images ^^) ;).

So unless I get another itch to scratch, this should be ready for field testing soon ;).

[shermp]

You have been busy!

And here I thought I was being safe by waiting a day :)

[NiLuJe]

Oops, turns out the two pass approach was slightly broken on 16bpp FW because of a stupid oversight ;).

That button size recap finally came in handy, because

Matched on a 184x1 button! :)

sure caught my eye ;).

[shermp]

Oops :)

Oh well, at least you spotted it!

[NiLuJe]

Oohkay, finally stopped having new stuff to try (I think? :D), so 1.3.0 is out, and I just finished the thread requesting button_scan testers ;).

[shermp]

Cool. Thanks for the heads up!