clear() results in TypeError for WaveShare epd2in7

[txoof]

Display Type: waveshare_epd.epd2in7 Version pulled: 2021.05.08 Main Branch

The omni-epd-test and example code both successfully display rectangles and the nebula image respectively without issue.

Calling the clear() method on a displayfactory object raises a TypeError:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-21-7ef3a3b8a5ed> in <module>
----> 1 epd.clear()

~/.local/share/virtualenvs/epdlib-AIMLabQa/lib/python3.7/site-packages/omni_epd/displays/waveshare_display.py in clear(self)
     64         Most devices utilize the same clear function
     65         """
---> 66         self._device.Clear()
     67 
     68     def close(self):

TypeError: Clear() missing 1 required positional argument: 'color'

The root cause is that the epd2in7 B&W driver expects a color value. This is extra frustrating because the 2in7 doesn't even use the color value.

I've started rewriting the waveshare EPD drivers to clean up some of this mess, but there's so much inconsistency, copy/pasta, and lack of proper documentation that I haven't made much progress beyond the 2in7 and 5in83.

Minimal example:

import sys
from omni_epd import displayfactory, EPDNotFoundError
from PIL import Image

displayName = "waveshare_epd.epd2in7"
print('Loading display')
try:
    epd = displayfactory.load_display_driver(displayName)
except EPDNotFoundError:
    print(f"Couldn't find {displayName}")
    sys.exit()

# if now load an image file using the Pillow lib
print('Loading image')
image = Image.open('../PIA03519_small.jpg')
# resize for your display
image = image.resize((epd.width, epd.height))
# prepare the epd, write the image, and close
print('Writing to display')
epd.prepare()
epd.display(image)
epd.close()

epd.prepare()
epd.clear()
epd.close()

[robweber]

Thanks for this, noticed a similar issue with the 3.7in display as well. Tagging as a bug and we'll get it fixed up. Are you planning on trying to get your fixes into the main waveshare lib? That might help with some of the blatant problems.

[txoof]

I've submitted a few prs in the past and they've been largely ignored. Not even rejected, just totally ignored. I don't think there's much motivation at WS to do any improvement.

I was considering just doing a general rewrite and seeing if I can martial some others to join in. I've done a little work on the 2in7 and 5in83 that's an ok start.

I only have a access to 3 screens, so i can only realistically work on those. I'm also not super skilled with this, just an enthusiastic amateur.

Would you have any interest in helping out with this? It could solve a lot of the mess we are experiencing with these libraries.

[txoof]

It looks like there's a new maintainer, @SSYYL, that's been pretty responsive. I've made a clean PR to fix up the 2in7 library; hopefully it's accepted.

[robweber]

That's an interesting idea - fixing up the Waveshare lib would be a big help overall. You could also start to use actual version numbers on things and get it pushed to PyPI. That's one big sticking point with that library is that it's hard to keep updated without any indication of if it's been changed. On the other hand it's personally frustrating to have to put that much effort into it when it kind of seems like Waveshare's job to provide a useable library - which apparently they feel is "good enough". I'm curious if your PR gains any traction on the main repo - it might be possible to get some fixes into that lib directly, which would probably be the best first option. Can you post a link to the PR - I'd like to keep an eye on it and see where it goes as well.

I'm going to make an effort to review the current Waveshare examples for the devices as it exists today and correct any issues with the devices to at least make it working for now.

[txoof]

@robweber,

I haven't done many PRs and I'm rather novice when it comes to submitting PRs, so any advice you can provide is greatly appreciated. Here's a link to the two tiny PRs I've provided for this particular issue:

• Remove needed color arg in Clear() method
• Align example code with PR #161

If these PRs don't get any traction, would you be interested in collaborating in rewriting the libraries? I've done a fair amount of pouring over the spec sheets and have a pretty good idea of how they're laid out, but some parts are still pretty mysterious. Having another set of eyes and brains to help out is always welcome.

Also, every panel has a different spec sheet. It should be no surprise: there's very little consistency between models.

[SSYYL]

Hello, Thank you for your help. I know that Waveshare Lib was a bit of a mess. At present, it is only available to the extent. Now I am mainly working on some new projects. If you have any ideas about waveshare lib, you can submit them to PRs and I will reply to you as soon as possible. Thanks.

[robweber]

@txoof - my only comment is that PR 162 includes the same code as PR 161. You probably only need the first one since it fixes both the lib and the example.

Based on the above comment what are your thoughts?

[txoof]

I have been reading guides on how to pr, and many suggest 1 change, 1 pr. I wasn't sure if this was two changes or one, so i submitted two different prs. Bad form?

[robweber]

I suppose every maintainer will have a slightly different response to that but in general I would want to see a PR address 1 change at a time; but not like a literal 1 change to the code. More like 1 feature change if that makes sense. It may take several commits that touch multiple places but they all are "1 change" to the codebase. In this case updating the clear() method and it's examples, or even other documentation around it, would be 1 change. Some repos will further request that you squash commits so it's literally 1 commit as well but that depends on the project. I usually like to see the history and can squash at the time of merge if needed.

For these two PRs 162 encompasses the same file fix as 161 so you could just close that one and leave 162 in my opinion.

[txoof]

Thanks for the feedback. I've tried to do that in PR #164; hopefully the maintainer will merge that and add a consistent color argument.

[txoof]

Good news: the pr was accepted into the main branch of the library. I'll pull that into my virtualenv and try this again.

[robweber]

Nice - I've modified and merged the code on this side too. #45 should have cleared up any discrepancies between the different clear() methods.