Closed gdkrmr closed 4 years ago
Neat! I dabbled a bit with the 4.2" partial refresh a year or so ago but never had the time to finish it and it was left in a fiddly state. This surely helps with getting the support finally implemented.
I haven't had the time to really take a good look (and not sure if I will have) but some comments:
LUT_WHITE_TO_WHITE = 0x21
and the rest can be added to the same place.def send_datas(self, data):
for i in data:
self.send_data(i)
def send_lut(self, idx, data):
self.send_command(idx)
self.send_datas(data)
send_lut(self.LUT_WHITE_TO_WHITE, self.lut_ww)
. There are probably a lot of other places where similar shorthands would be useful.About the VNC problems, have you tried installing tightvncserver
? I'm not sure what you refer to with the builtin server or if Raspbian has one, I usually install the tight variety kind of out of habit :) Timeout instead of some error seems to imply a connection problem more than anything (are you using localhost
in the connection address and no firewall can be messing with it, and is the VNC process actually running and visible in netstat -tlpn
, on the correct port?) but I guess it could have to do with something going wrong with the server format or something. If you can provide some more details (the commands you run) I can make more guesses.
Thanks a lot for your work on this, it would be cool to have partial refresh working with the 4.2" since it's one of the two displays I have.
- This being a hefty driver it's probably okay to have it as a separate file too, but there's usually a lot of overlap or simply cosmetically different ways of achieving the same thing in the Waveshare drivers, so likely it can be shortened quite a bit. When I implemented the original set of drivers I actually wrote a hacky program to "analyze" the common parts in a bunch of different drivers and used that info to select what to implement as common methods. Still, some drivers are different enough to justify not spending the time to make the common methods more complex to support them.
I will have to take a thorough look at this, I have seen that the full driver also had quite a lot of separate code. There is one big conceptual difference in that the C code has a global frame buffer of the size of the display and display_partial
uses this frame buffer with the edges as coordinates.
- Some things that can (and ought to) be used are the human readable constants for what the bytes mean (if we know), some of these are already defined, such as
LUT_WHITE_TO_WHITE = 0x21
and the rest can be added to the same place.
That is definitely on the TODO list. I think it may also serve readability to have class EPDconsts
which contains all the consts so they can be put into a separate file out of the way.
In this particular driver there's a few places where a sequence of data is written and a lot of LUT writing, so maybe a couple of helper functions could make the code shorter and more readable (note: the original code would also benefit from this overall):
def send_datas(self, data): for i in range(0, len(data)): self.send_data(data[i])
def send_lut(self, idx, data): self.send_command(idx) self.send_datas(data)
- ...or similar ones, then the most repetitive parts become mostly oneliners like
send_lut(LUT_WHITE_TO_WHITE, self.lut_ww)
. There are probably a lot of other places where similar shorthands would be useful.- Before merging this, the full refresh should be made available too so that both can be used.
Of course.
- What's the status of the grayscale code, does it work output-wise?
Haven't tried, I guess that not, as this is a 1:1 translation of the C-code which I had to change a bit for the other methods (I think their notion of width
and height
was different). This would also require a change to the interface, to allow to select the grayscale rendering. I am especially interested to see how the trade-off image quality/performance plays out.
- Performance can be improved after the code reliably works otherwise so it's not a priority until then, but it's of course okay to do if you notice something safe to optimize from the get-go.
- About the orientation: if I remember correctly, it's currently a bit fiddly and I think using VNC would avoid such problems since it's much more straightforward drawing-wise.
It would be great to have some "official" specs on this. E.g.: Displays are to be used in portrait/landscape mode. The frame buffer comes in as a picture rotated 90 degree to the left.
For the 4.2inch display, the (0,0) coordinate is in the upper left corner, when the long edge with the white rubber is on the bottom.
About the VNC problems, have you tried installing
tightvncserver
? I'm not sure what you refer to with the builtin server or if Raspbian has one, I usually install the tight variety kind of out of habit :) Timeout instead of some error seems to imply a connection problem more than anything (are you usinglocalhost
in the connection address and no firewall can be messing with it, and is the VNC process actually running and visible innetstat -tlpn
, on the correct port?) but I guess it could have to do with something going wrong with the server format or something. If you can provide some more details (the commands you run) I can make more guesses.
I will try your suggestions, thanks!
Thanks a lot for your work on this, it would be cool to have partial refresh working with the 4.2" since it's one of the two displays I have. :+1:
What about something like this? Similar, probably works, but not identical. drivers_base.py:
def reset(self):
self.digital_write(self.RST_PIN, GPIO.LOW)
self.delay_ms(200)
self.digital_write(self.RST_PIN, GPIO.HIGH)
self.delay_ms(200)
drivers_4in2.py:
def reset(self):
self.digital_write(self.RST_PIN, GPIO.HIGH)
self.delay_ms(200)
self.digital_write(self.RST_PIN, GPIO.LOW)
self.delay_ms(200)
self.digital_write(self.RST_PIN, GPIO.HIGH)
self.delay_ms(200)
What about something like this? Similar, probably works, but not identical.
Hard to say without testing it... might be that either would work fine (I can't test it right now). The drivers are based on the examples supplied by Waveshare and there's usually no other reasoning for a particular display control sequence other than that it was what the example did originally. Unless there's a way to prove a particular code example is redundant or wrong, I think it's best to do what Waveshare themselves have done in case there's something nonobvious involved (and also so we can point fingers if someone's display gets fried :wink: ).
I am looking into getting the correct orientation. I found that the old epd4in2
driver (it is epd4in2old
in this PR) has the --portrait
option wrong, i.e. it is portrait by default and with the --portait
option landscape. Therefore the driver here will correct this and behave different than the old epd4in2
driver.
FYI: I also rebased this PR.
There are two functions that may be useful for other classes: set_setting
and set_resolution
. Someone may want to put these into one of the superclasses.
I am thinking about the grayscale mode and would like to add this in another PR, this requires some deeper refactoring, i.e. adding command line parameters.
The size of the driver got a lot smaller (>400 lines) and could be shortened by another 100 lines if the commented out display_gray
method gets removed, should I put it back into drivers_partial.py
?
I am looking into getting the correct orientation. I found that the old
epd4in2
driver (it isepd4in2old
in this PR) has the--portrait
option wrong, i.e. it is portrait by default and with the--portait
option landscape. Therefore the driver here will correct this and behave different than the oldepd4in2
driver.
Okay, well that might be the fluke (or one of them) I was thinking about... good that it's fixed then.
There are two functions that may be useful for other classes:
set_setting
andset_resolution
. Someone may want to put these into one of the superclasses.
Yes, a lot of stuff could be refactored and simplified with such constructs - actually one of the oldest issues (https://github.com/joukos/PaperTTY/issues/8) is precisely about that. It would be optimal to have some tests first before major refactoring, but obviously equivalent shorthands should be safe to do anyway.
I am thinking about the grayscale mode and would like to add this in another PR, this requires some deeper refactoring, i.e. adding command line parameters.
Sure, take your time and it would of course be cool to have if it can be made to work reliably and you're willing to spend your time on it :). Also a bit related is the discussion at https://github.com/joukos/PaperTTY/pull/45.
The size of the driver got a lot smaller (>400 lines) and could be shortened by another 100 lines if the commented out
display_gray
method gets removed, should I put it back intodrivers_partial.py
?
Hmm, well I think in general the code that's merged to master
shouldn't at least contain entire commented out blocks of old functions/classes/etc. "just in case they're needed (again)" since they're not part of the functionality. The grayscale code would be nice to have on a WIP branch somewhere as reference until the implementation is finished, though.
I guess we could also split each driver into their separate file just to keep things simple and orderly and so that it's easy to locate the code for a particular driver when needed. In the end I'd probably find that to be cleaner than the current sort of "partial" / "full" split, but I might need to browse the current code a bit to get a more defined opinion on it, or sleep on it. Could of course be that after thorough and experimental refactoring or the introduction of new hardware, some clear groupings of the displays based on their driver code would emerge, but that feels a bit distant still and probably wouldn't cover them all neatly anyway.
Some of the variants (B/C/D etc.) of some of the displays also differ quite a lot in their code, such as having extra LUTs etc., so I guess it would be simpler to have each of those as a separate file too - keeping it simple.
Anyway, nice work!
I did some profiling and on my Pi4B < 10% of the time is spent on setting the frame buffer, the rest is just sending data and I doubt that this can be optimized.
EDIT: I will test this with the vnc server and then do the cleanup. I guess after that it can be merged.
EDIT2:
The size of the driver got a lot smaller (>400 lines) and could be shortened by another 100 lines if the commented out
display_gray
method gets removed, should I put it back intodrivers_partial.py
?Hmm, well I think in general the code that's merged to
master
shouldn't at least contain entire commented out blocks of old functions/classes/etc. "just in case they're needed (again)" since they're not part of the functionality. The grayscale code would be nice to have on a WIP branch somewhere as reference until the implementation is finished, though.
Sounds like a good idea, I will remove the commented out code and put it into a new branch. I am not so sure how much effort I will put into making the grayscale code to work because it doesn't seem to have a partial update mode and this will tank performance for continuous updating.
might need to browse the current code a bit to get a more defined opinion on it, or sleep on it.
I guess you need to to make some design decision, let me know what you think is best:
Regarding the optimization of sending data, I was browsing the Waveshare wiki a while ago and noticed this in the 6" HD page: https://www.waveshare.com/wiki/6inch_HD_e-Paper_HAT#About_the_speed_of_SPI
I haven't really considered this much earlier since I've been mostly using a Zero and a 2.13" display, but the SPI speed is currently set to 2MHz here https://github.com/joukos/PaperTTY/blob/master/drivers/drivers_base.py#L179 and here https://github.com/joukos/PaperTTY/blob/master/drivers/driver_it8951.py#L178 (for IT8951). With the faster Pis at least it might be possible to bump the speed up reliably to hopefully get a significant performance increase with the larger panels.
I wonder if anyone has experimented with this yet? I guess I could try to dig out a RPi3 and test with the 4.2" panel if I have time.
In the C-code it is set to 20MHz (https://github.com/waveshare/e-Paper/blob/master/RaspberryPi%26JetsonNano/c/lib/Config/dev_hardware_SPI.c#L96).
I have played around with the speed and could get it up to 40MHz on my PI4, 50MHz failed. The flamegraph didn't really change though:
EDIT: I didn't measure, but also cpu usage didn't feel differente, every time somewhere between 50-60%
A different question: Should I replace the scrub
routine with clear
? clear
is the "official" way, it is much faster than the current scrub
but does not give control over the color.
Well, the scrub
is more or less a workaround for my own dysfunctional display, since clearing it otherwise simply doesn't work (the default block width is also the maximum that works for me - if I set any bigger region, the areas are not updated correctly - this caused a lot of WTFs during early development since I thought all of the displays would be so finicky).
With a correctly functioning unit, I think a full-refresh clear should be used. Having scrub
available in case of problems may be useful though, since it sort of mechanically clears the display and maybe helps to even it out in some cases (maybe).
So I guess that a call to scrub
should use clear
then and scrub --size 20
should do the partial scrubbing?
I think scrub functionality itself can be left as is - it may have some niche uses (like making my broken display usable - size 20
is also too much for it) - but it shouldn't normally be used in places where we just intend to clear the display. I suppose there could be an advanced option to select whether to use scrub
or the "correct" way (or perhaps even some third way), but it's yet one more usability detail to consider, along with deciding where and when such screen clears are actually needed.
tightvncserver
works, thanks for the tip, and it shows a picture. I couldn't really test it yet, because I cannot get the keyboard or mouse to produce input.
Easiest way is probably to connect to the VNC server from another computer and issue input from there for testing (or, the same computer if that's where it runs, just connecting to localhost).
Easiest way is probably to connect to the VNC server from another computer and issue input from there for testing (or, the same computer if that's where it runs, just connecting to localhost).
That works! Is there a way to make this "headless"?
Well, for me this can be merged. There there are some cleanup items (see first post) and you have to tell how you want the code to be organized.
The way to make things work "standalone", using just a Pi with peripherals connected to it is described in the README, in the VNC update section. Although it's a bit convoluted setup for now, requiring you to first of all run a graphical desktop on the Pi, then x11vnc
to expose that desktop via VNC (as opposed to running a separate, VNC-only session). There's a bit more stuff to fiddle with and for example setting the resolution for the main graphical session isn't documented for now, since by default it would be displayed via HDMI and needs to be manually set to something more suitable for the small e-ink. I haven't even tried that yet myself.
The reasoning for using it this way is that any connected USB input devices or other stuff would work without extra hassle and the e-ink could just be the display, but it would be easier if the peripherals could just be attached to a simpler, standalone VNC server. Not sure if I'm just missing some simple method to do that (it's likely there's some smart way I don't know), but I seem to remember that I tried to find out what this Xvnc
option was about:
-udpinputport port
UDP port for keyboard/pointer data.
... and it seemed like a dead end. Maybe should recheck.
I've also considered trying if Synergy could work for sort of getting a second desktop on e-ink, controlled with the same input devices as the main session, but haven't yet.
Now that I finally had taken my 4.2" out of storage and connected it, I tried this and it seemed to work, which is great! (I merged it with the interactive menu PR code)
The display does get washed out pretty quick though, so I figured I'd try to scrub
it, but unfortunately there's some small bug there and it crashed just before drawing the last black stripe:
pi@putka:~/code/PaperTTY $ sudo ~/.virtualenvs/papertty/bin/python3 ./papertty.py --driver EPD4in2 terminal --autofit --font /usr/share/fonts/truetype/dejavu/DejaVuSansMono.ttf --size 20 --interactive
Setting spacing to 0.
Automatic resize of TTY to 15 rows, 33 columns
Started displaying /dev/vcsa1, minimum update interval 0.1 s, open menu with Ctrl-C
Scrubbing display (SIGUSR1)...
Traceback (most recent call last):
File "./papertty.py", line 700, in <module>
cli()
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 764, in __call__
return self.main(*args, **kwargs)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 717, in main
rv = self.invoke(ctx)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 956, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 555, in invoke
return callback(*args, **kwargs)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/decorators.py", line 27, in new_func
return f(get_current_context().obj, *args, **kwargs)
File "./papertty.py", line 591, in terminal
ptty.driver.scrub()
File "/home/pi/code/PaperTTY/drivers/drivers_base.py", line 60, in scrub
self.fill(self.black, fillsize=fillsize)
File "/home/pi/code/PaperTTY/drivers/drivers_4in2.py", line 368, in fill
self.draw(x, 0, image)
File "/home/pi/code/PaperTTY/drivers/drivers_4in2.py", line 382, in draw
self.set_frame_buffer(y, x, image)
File "/home/pi/code/PaperTTY/drivers/drivers_4in2.py", line 356, in set_frame_buffer
self.frame_buffer[idiv + idxj] &= ~mask
IndexError: list index out of range
Though scrubbing it shouldn't be normally needed anyway, so for testing I overrode the scrub
method with this:
def scrub(self, fillsize=16):
"""Scrub display - only works properly with partial refresh"""
self.init(partial=False)
self.clear()
self.init(partial=self.partial_refresh)
Then started it up again and sent a SIGUSR1 to it and the display was crisp again. I didn't let it run for long though, so I'll let it run FreeDoom overnight while disabling full refreshes for that time (uh oh...). Let's hope it's still alive in the morning :sweat_smile:
By the way, I also tried to adjust the SPI speed, which didn't help any, and only then realized that the parameter is called max_speed_hz
so I guess it's just the maximum it'll go if the device supports such... I didn't delve too much into how spidev
actually works but maybe we can assume that at least this particular display won't talk very fast. The IT8951 ones apparently can handle a lot more.
Found some statistics from /sys/bus/spi/devices/spi0.0/statistics/
, such as bytes[_rx|_tx]
to get the amount of data written etc., though.
The display does get washed out pretty quick though, so I figured I'd try to
scrub
it, but unfortunately there's some small bug there and it crashed just before drawing the last black stripe:
This is an out of bounds issue when setting the frame buffer, when the scrubbing width does not divide 300. Added to the TODO list.
After a full night of Doom (~9000 full-screen frames), the display was still about as legible as it was in the beginning so nothing weird happened - not that I expected any problems though. Just cleared it with a full refresh and it's as clean as ever.
I noticed that showvnc
was indicating it did full screen refreshes every time (prints out full ...
) and this was due to it checking for a supports_partial
flag, which is not set currently in the new driver's __init__
. The image was drawn using the partial refresh code though since draw
checks separately if we want a partial refresh (and by default we do). The __init__
should have a self.supports_partial = True
there for the VNC feature to realize it since it does a check like if updates > 0 and (self.driver.supports_partial and self.partial):
.
Adding the missing flag however will cause another out-of-bounds issue when it tries to draw the partial area (I was using a 400x300 VNC display and rotating it 90 degrees):
pi@putka:~/code/PaperTTY $ sudo ~/.virtualenvs/papertty/bin/python3 ./papertty.py --driver EPD4in2 vnc --host localhost --display 1 --password blablabla --sleep 0.1 --rotate 90
Loading PIL font tom-thumb.pil. Font size is ignored.
initial (1): (300, 400)
partial (2): (232, 284, 256, 301)
Traceback (most recent call last):
File "./papertty.py", line 700, in <module>
cli()
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 764, in __call__
return self.main(*args, **kwargs)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 717, in main
rv = self.invoke(ctx)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 956, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/core.py", line 555, in invoke
return callback(*args, **kwargs)
File "/home/pi/.virtualenvs/papertty/lib/python3.5/site-packages/click/decorators.py", line 27, in new_func
return f(get_current_context().obj, *args, **kwargs)
File "./papertty.py", line 495, in vnc
ptty.showvnc(host, display, password, int(rotate) if rotate else None, invert, sleep, fullevery)
File "./papertty.py", line 322, in showvnc
self.driver.draw(diff_bbox[0], diff_bbox[1], new_vnc_image.crop(diff_bbox))
File "/home/pi/code/PaperTTY/drivers/drivers_4in2.py", line 386, in draw
self.set_frame_buffer(y, x, image)
File "/home/pi/code/PaperTTY/drivers/drivers_4in2.py", line 354, in set_frame_buffer
if pixels[j - y, self.height - (i - x) - 1] != 0:
IndexError: image index out of range
(Note to self: VNC code needs some small fixes...)
Actually, now that I looked closer I think I notice the same "burn-in" issue - during the night the mouse arrow cursor was in the center of the image and when refreshes are done I can still see it there. I'll take a look at it later.
__init__
Adding the missing flag however will cause another out-of-bounds issue when it tries to draw the partial area (I was using a 400x300 VNC display and rotating it 90 degrees):
I observed that, too. An now the terminal also crashes now.
Finally I had some time to fire up the Pi again and can't seem to find the "burned-in" cursor anymore - perhaps just having it disconnected and then using it again sorted it out.
That was a bummer, the driver wasn't actually doing partial refreshes :-). I had to do quite a bit of code refactoring, but it gave me a deeper understanding of how everything actually works. I think that the code updating the frame buffer is much more understandable now. I left some comments at the beginning of the driver.
The bad thing is that the driver is in portrait mode by default now, but I guess I can live with that. This is due to different understandings of width and height of papertty and the C code I used for this driver. I am glad that I got it working for now and this would be another PR.
Can you give it another try?
EDIT: I have also put the display_gray
method in an extra branch here and did some general cleanup.
EDIT: It mostly works, but there is still something not quite right with refreshing.
That was a bummer, the driver wasn't actually doing partial refreshes :-).
It wasn't? :) At least it didn't do a flashing full refresh. I'll try it again as soon as I have time, hopefully some evening this week.
About the "portrait" mode, it would be simpler to have "rotate-X" anyway (like in VNC mode) and not care too much about the physical layout of the various modules. Just needs to be implemented properly.
P.S. anyone reading this who has a 4.2" display is very welcome to try this too and share their experience.
The refresh bug was actually a bug from the original C code that I copied (https://github.com/waveshare/e-Paper/issues/63) and should be fixed now. I hope nothing else comes up.
UPDATE: I have run htop
for two hours and everything seems to work fine.
I finally managed to try it now and it seems pretty snappy. Typing with it works fairly ok, similar to a dial-up connection :) The image is pretty washed out at least on my unit, but that's the price of the fast refresh I suppose. I think I'll add a menu option to manually apply a full refresh as a workaround for now to keep the image a bit cleaner.
I don't think I have time to do a more thorough code review now but at least I didn't get any crashes running it in both orientations and doing a scrub, so I think it can be merged and we can sort out any lingering issues later.
Thank you very much for your work on this!
Great, thanks for the patience! I am testing with a ttf font and it looks relatively clear. The font is a bit larger than the default font, this may make a difference, also the constant redrawing of some areas that don't change makes the font look a bit heavier/bolder.
There are also some comments that start wit # TODO:
, where I have identified possible places to improve performance.
I wrote a partial update driver for the 400x300 4.2 inch monochrome display :+1: #15. Refresh rate is pretty decent, much better that the full refresh driver! I just got it working and the code still needs some love, I guess that there are still some (more likely many) issues. Does any of the other devs have the same display?
Run with:
Details:
--flipy
option for it to work properlyIt would be cool if I could get some feedback!
EDIT: TODO List, feel free to suggest more items.
self.supports_partial = True
to__init__()
Final clean-up:
epd4in2old