Closed aaronr8684 closed 2 years ago
I don't have any of these type of color displays however something about that line of code looked familiar to me from another PR. I managed to dig up #50. That PR modifies that same line of code and was verified to work with a different tri color display
@dr-boehmerie - you originally had done that work, are you able to test your display with the code fix here? I'd hate to fix one display only to break another. If needed we can split them apart if they really require to different palettes to work.
Let me take a look at the driver for the 5.83" yellow screen. I wouldn't be surprised at all if the two screen drivers are handled completely different based on the other code I've seen them put out. I'd hate to do it, but we might need to break out the TriColor class into two (or create some special case if's) like you do for the 3.7".
@robweber So looking through a few of the drivers, I think we (@dr-boehmerie and myself) both had the wrong approach based on the screens we had access to. Some screens invert the values of each pixel during the buffer creation and then, inverted back as needed in the display method. So instead of manually creating a buffer (and choosing between 0 and 1 for the color white), if we create an image (blank white) like the rest of the class does and then just process it through the buffer as it was intended to be run by the drivers. I've updated the PR to reflect this.
Thanks for taking a deep dive on this. It's always sort of eye opening to me how disjointed the waveshare code is from one driver to another. You'd think they'd lean on inheritance a bit more and try to make things consistent.
Your fix makes sense to me. Much easier to depend on the driver buffer methods to do the work in the way they expect. I'd still like confirmation the color pieces still work across devices if possible before merging it all in. Wish I had ways to test this myself but I can only justify so many EPDs around the house!
Well consider the 7in5b tested with the fix. If we can't find anyone else to test a yellow screen, I might just pick one up. I just have to find a "justified" project 😅
@robweber I've been poking around and it looks like this was (more or less) how it was handled before it was switched in #8 to increase speed. Then it was switched from 0 to 255 to handle the 5.83" yellow screen.
Looking at Waveshare's docs and repo, it appears that the official test files all (at least the dozen I checked, including the epd5in83bc) handle "blanking" in the same way before drawing on the images for displaying the test patterns.
The epd5in83c in mode=bw works just fine with the change in line 204 (creating a separate img_white).
But using it in mode=yellow, and changing lines 211+214, results in a yellow only image shown. Part of it is slightly darker because the yellow dots have a very small black dot visible inside them as well. It seems the black and the yellow image have just the same pixels set, apart from a few yellow-only pixels, depending on the source image.
Reverting line 211 (separation of img_black) does the trick, and the image shown is fine, again.
If waveshare drivers differ in their expectation of what value an active pixel should have, perhaps a "deviceQuirksMap" may help 😉
I think the big change was the lines 204-207, so if the other changes get dropped for now, at least all screens have BW support again. We can dig deeper into the WS library and see if there is a way to code it to handle all scenarios. If you don't mind, I might ping you @dr-boehmerie if I have some more code to test if I don't just pick up a yellow screen of my own.
It is annoying that a single company can't figure out how to make consistent code across their screens. 😮
I agree - very annoying that can't be more consistent across the devices.
Thanks for doing all this digging. Just based on a quick read it seems that these changes will work across the devices in that specific class. I'll review it closer to be sure before merging anything in.
In summary...
The changes in the bw section are good and confirmed to work with both our screens.
The changes in the color section ('else' block) work for red screens (or at least some of them), but break at least the epd5in83c. It's up to you if you want to keep those commits or revert them. It appears based on the availability of the red vs yellow screens that red is more popular lately, but I have only tested on a single red screen so it's entirely possible that the 7.5" red is the exception instead of all the red screens.
I had a chance to look at this all in more detail. If the BW modes are shown to work then let's keep that for sure. Regarding the colors I went back and looked at the Waveshare drivers for each of the devices (epd7in5bc and epd7in5b_V2). There are some very big differences in the Waveshare code between the two versions; which I'm sure is why we're seeing different behaviors. I'm not even going to pretend that I follow the first one. I would hazard a guess that V2 editions are just different than the others.
I like the suggestion of using some kind of mapping of the differences rather than trying to break up everything into even more class variations. I've gone ahead and added a version
identifier to the device mapping options within each class. Basically 1 = V1, 2 = V2, etc, etc. @aaronr8684 - if you could rebase your code with these changes you should be able to do a check for if version = 1
, use the original code and if version = 2
use your new code. Hopefully this accounts for the differences between the boards and both device types will work properly. You should be able to use the following to get the version for comparison:
self.deviceMap[self._device_name]['version']
Does that sound like it would work?
Would this work to merge in "as is" in the PR and then I'll revert the color code? The big fix here is the BW code anyway and then if you find a different solution for the color piece we can layer that in later. From the notes above it looks like the old color code was working for both screen types.
Sounds good to me!
@missionfloyd No because some drivers do the inversion in the getbuffer, some in the display, and some do both. So while the result is the same if the image is ONLY inverted in the getbuffer call, not every driver expects that behavior.
The 7in5b_V2 screen wasn't working and I was able to change a few lines of code to get it to work as expected. I also added some changes in the README to clarify driver support. Let me know if you need anything else from me for this PR.
I tested with half a dozen sample images (including one that was all red) and here is an example of the output in 'red' mode.