joukos / PaperTTY

PaperTTY - Python module to render a TTY or VNC on e-ink
953 stars 104 forks source link

Give error if `scrub` is used on a screen without partial refresh suppport #52

Closed colin-nolan closed 1 year ago

colin-nolan commented 4 years ago

It took me a while to realise why scrub didn't work on the device that I have, which doesn't support partial refresh.

This PR aims to make the issue clear to anyone trying the same thing in the future.

joukos commented 4 years ago

Hmm... if all is well, I think it should work for full refresh too, but it'll be very slow because the display is refreshed after each bar. That said, there could well be circumstances where it doesn't work (for example with my broken 2.13" which doesn't seem to be able to do a full refresh no matter what). I think I did try it with the 7.5" and 4.2", but could try again while testing the other PR.

colin-nolan commented 4 years ago

The issue is that the code doesn't support it. The stack trace for the failure goes:

scrub (size=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/papertty.py#L458-L463 scrub (fillsize=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_base.py#L58-L60 fill (fillsize=16) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_base.py#L63-L67 draw (image.size=(16,self.height)) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_full.py#L64-L66 get_frame_buffer (imwidth != self.width -> 16 != 800) https://github.com/joukos/PaperTTY/blob/e8661895193df5bfd518591cd24fd4c8d39d3749/drivers/drivers_full.py#L73-L76

joukos commented 4 years ago

Ah, I see, you're right, the code basically ignores the X,Y for full refresh drivers and expects a full-sized image when drawing with it. I think I must've tried it on partial support displays but simply told it to use full refresh.

I "fixed" it now by changing the full refresh driver's draw code to keep an internal frame buffer where it pastes the provided image and then draws the result. Running the thing with a big display takes ages though, so having a warning is probably a good idea anyway and perhaps a --force if one really wants to do that (I feel that scrub is sort of a hardware "degauss" function).

Need to wait until the scrubbing completes on my 7.5" and I'll see if anything goes wrong :)

Update: well it took about 36 minutes and apparently I got the orientation wrong for this display, so it cleared it sideways and not completely. No errors though - pasting to a buffer has the nice benefit of not crashing when doing something dumb :) I'll try to look at this a bit later, today perhaps.

joukos commented 4 years ago

Hmm, another little hurdle with this is that the scrubbing can be initiated basically in three different ways: specifying it on the command line or in terminal mode by either sending a SIGUSR1 to the process or by selecting it from the interactive menu. In the latter two an uncaught exception is not desired, so there should be a somewhat more holistic approach to this.

I tried it out a bit and did the following changes:

Perhaps doing these changes is a bit unnecessary for what is essentially a debugging function for "dirty" screens (the maximum width was there for a reason - larger values didn't produce a good result with my 2.13") and the whole function could almost be removed entirely. At least it shouldn't be mentioned as the first command to try in the documentation.

I still need to clean up a bit before merging all of that. This PR opened a little can of worms it seems :)

joukos commented 4 years ago

Sorry about this PR taking so long, I've been too occupied with other things lately. I'll try to keep it near the top of the stack of "things to do when I'm not busy or too exhausted".

colin-nolan commented 4 years ago

Don't worry about it - I only added the PR to make the error clearer!

joukos commented 1 year ago

PR #106 essentially "fixes" this in the sense that scrub just fills the display and doesn't crash, so I think I'll close this.