Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

driver implementation for waveshare 4.2" rev2.2 #64

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

surdouski
Copy link

@surdouski surdouski commented May 22, 2024

Couldn't find a working driver anywhere online for micropython (or any other language) for partial refresh for the waveshare 4.2" (rev2.2), so I wrote one. In all the partial refresh code I can find anywhere, there is a bug in it (writes the waveform command twice instead of the second one needing to be the data entry command). All the materials I could find copy the things written verbatim from the C-source, and I suppose no one tested it either, so all the materials I could find have that bug.

I did not test the fast or 4gray, so they are commented out below. I might have use for them in the future, but I am a software developer and I seem to get caught up really easily on small things that I'm guessing wouldn't be a problem for actual firmware developers.

Also, just wanted to say thanks for all your work on the micropython libraries/utilities.

UPDATE:
There was some weird timing bug inside of the partial refresh, which I was unable to get rid of because I don't know how it's being introduced. The current version works though, confirmed that the partial refresh on the aclock demo successfull refreshes without a full screen refresh on each tick of the hand moving.

@peterhinch
Copy link
Owner

I'm not clear what hardware you are using. The reference in the code seems to point to a bare EPD. Are you using this version with a built-in controller?

Please can you explain the benefits of the hardware you're using compared to the already supported 4.2" display. This is an excellent unit with low ghosting and is compatible with a wide range of hosts. At present my recommendation is to use this unit.

I have a difficulty accepting a PR for hardware which I don't possess - it makes it hard or impossible for me to deal with requests for support. If you can make a good case for the hardware you are using I might consider buying one.

@surdouski
Copy link
Author

surdouski commented May 23, 2024

The reason I bought this hardware initially is because I thought it was fully supported by your library. Unfortunately the listings are confusing, and as someone that has only briefly entered the hardware space, I was convinced that I was receiving the model mentioned in your current driver. Also, I made this mistake[?] twice; two different suppliers both shipped me this version instead of the version I expected to get.

The link that you mentioned, "this version," appears to be the one. On the back of the device you can see the "Rev2.2" on the back of the screen underneath the word "Module" and above the "v2" sticker.

I do respect the fact that you don't want to accept a PR when you can't test it yourself; it's part of the reason I probably find your repo's so reliable. I would have a hard time making a case for it with my experience -- the best case for supporting it would be that it is confusing trying to find the device you do currently support.

Edit: figured i'd an include an image of this one's ghosting since you mentioned ghosting of the other one. this is almost a full rotation's worth (you can see the small area where no ghosting yet).
image

@peterhinch
Copy link
Owner

For future reference this doc lists supported displays with links to manufacturers' sites.

Ghosting is a perennial problem with these displays. The one I reference above has the lowest level I've yet seen. In the early days ghosting was awful and refresh times were long, so I designed this clock display. It has the attribute that changes are additive so ghosting cannot occur. Once per hour a full refresh takes place. It's still available as part of nano-gui eclock.py, if only because every engineer has to invent a clock at some point in their life :)

As for the PR, I'm afraid it's "thanks but no thanks", for the reason stated and because the 4.2" display that I do support is so good.

@peterhinch peterhinch closed this May 24, 2024
@peterhinch
Copy link
Owner

In the light of #68 I'm reopening this. I'm trying to procure a V2 display to evaluate/test.

@surdouski
Copy link
Author

I remembered that my local revisions were more correct than the previous iteration. Sorry that it's so messy at the moment, I haven't cleaned it up at all; I'll see if I can get around to that soon.

@peterhinch
Copy link
Owner

The code has the following comment at the start which I don't understand. It seems to imply that partial refresh doesn't work, while your photo shows that it clearly does work. If the Waveshare code is broken, where did you find information to correct it?

# Note, at the time of writing this, none of the source materials have working
# code that works with partial refresh, as the C code has a bug and all the other
# materials use that reference material as the source of truth.

@peterhinch
Copy link
Owner

How would you like to proceed? I've had a preliminary look at the code and there are a few things that I'd query. The usual approach is for me to do a detailed code review with you implementing any agreed changes. Do you have the time and inclination to do this or would you rather I take the code as written and develop from there?

@surdouski
Copy link
Author

The code has the following comment at the start which I don't understand. It seems to imply that partial refresh doesn't work, while your photo shows that it clearly does work. If the Waveshare code is broken, where did you find information to correct it?

This comment is in not in reference to the code I had written. The C code and any libraries that used that C code as reference (including any python code) were all incorrect. I believe I had to look at the datasheet to determine what the correct commands were. I could investigate further, but my memory is failing to give me more detail at the moment.

How would you like to proceed? I've had a preliminary look at the code and there are a few things that I'd query. The usual approach is for me to do a detailed code review with you implementing any agreed changes. Do you have the time and inclination to do this or would you rather I take the code as written and develop from there?

I would be happy to take the approach of review and me implementing any agreed upon changes. I'm still early on in my learnings of firmware, but I'm unemployed at the moment so I have free time to spare. If you'd prefer, I can try to clean things up a bit more first to make it a bit more coherent -- the recent push I made were changes made after you had declined the PR, so I hadn't expected anyone to see them (but they are working).

@peterhinch
Copy link
Owner

I would be happy to take the approach of review and me implementing any agreed upon changes.

Excellent.

I have ordered a V2 board direct from Waveshare. This struck me as the only way to be sure of getting a V2 board, but I've no idea how long delivery is likely to take. So I can't contribute much other than general observations. Here are some initial thoughts based on a fairly cursory examination.

  1. I agree with your comments re the commented-out init_fast code. Speed of instantiation is not an issue. I think we can lose that code.
  2. It would be good if the greyscale code worked based on a 2-bit framebuffer as per pico_epaper_42_gs.py. You'd need a constructor arg to specify greyscale mode. I wonder if partial updates work in greyscale mode?
  3. _as_show needs attention. Currently the PR yields after every line, so a full refresh will yield 300 times. If you had another task that blocked for 20ms it would push the refresh time out to 6s. I used to yield every 16 lines to mitigate this, but I have just pushed an update that measures the blocking time and only yields when a threshold is exceeded. See new code. I suggest we adopt this approach as it's more platform-independent and is user configurable.
  4. The do_refresh method is needed for micro-gui, complete with its unused arg.

What chip does the display use? I assume it's not a UC8176 judging by the lower baudrate, I'd like to grab a datasheet.

@peterhinch
Copy link
Owner

Re greyscale I can see three options:

  1. Specify greyscale as a constructor arg.
  2. Subclass the main driver.
  3. Create separate 1-bit and 2-bit drivers as per V1.

I'll leave it up to you to decide - I'd be happy with any of these options. Or you may want to ignore greyscale and just write a 1-bit driver.

In terms of the general way forward I'll step back until you have some code ready for review. Feel free to continue the discussion if there are any issues that crop up.

1 similar comment
@peterhinch
Copy link
Owner

Re greyscale I can see three options:

  1. Specify greyscale as a constructor arg.
  2. Subclass the main driver.
  3. Create separate 1-bit and 2-bit drivers as per V1.

I'll leave it up to you to decide - I'd be happy with any of these options. Or you may want to ignore greyscale and just write a 1-bit driver.

In terms of the general way forward I'll step back until you have some code ready for review. Feel free to continue the discussion if there are any issues that crop up.

- remove commented out init_fast
- uncomment demo_mode check
- add MAXBLOCK property
- check MAXBLOCK ticks between last await in _as_show_full
@surdouski
Copy link
Author

What chip does the display use? I assume it's not a UC8176 judging by the lower baudrate, I'd like to grab a datasheet.

I'm seeing... YE08 01K AOJK ? Attaching image because I'm not sure which are the significant numbers or if I'm reading them correctly.
image

@surdouski
Copy link
Author

surdouski commented Jul 5, 2024

  1. I agree with your comments re the commented-out init_fast code. Speed of instantiation is not an issue. I think we can lose that code.

👍

  1. _as_show needs attention. Currently the PR yields after every line, so a full refresh will yield 300 times. If you had another task that blocked for 20ms it would push the refresh time out to 6s. I used to yield every 16 lines to mitigate this, but I have just pushed an update that measures the blocking time and only yields when a threshold is exceeded. See new code. I suggest we adopt this approach as it's more platform-independent and is user configurable.

Updated to use this. I have noticed in my code that I have code that is fairly split between asynchronous updates vs full updates. I think my issue is that my "partial" and "full" code are separated in a way that is very confusing. I will see if I can reason out the logic for updates vs setting and displaying the updates -- although I do remember this being hard for me last time, as whenever I tried changing the way I ran the commands, the epaper did not display correctly.

  1. The do_refresh method is needed for micro-gui, complete with its unused arg.

👍 Hopefully I did this right, I only have it using _as_show_full until I figure out the thing in the previous comment.

@surdouski
Copy link
Author

Re greyscale I can see three options:

  1. Specify greyscale as a constructor arg.
  2. Subclass the main driver.
  3. Create separate 1-bit and 2-bit drivers as per V1.

I'll leave it up to you to decide - I'd be happy with any of these options. Or you may want to ignore greyscale and just write a 1-bit driver.

I haven't touched this yet because I think the split between the "full" and "partial" logic and it's resolution could change my opinion on what the best way to do this is. However, my current thought is that "create separate 1-bit and 2-bit drivers as per V1" is probably best because it's already the established paradigm.

- add _as_show_partial method
- check for partial vs full in do_refresh
@surdouski
Copy link
Author

surdouski commented Jul 5, 2024

My previous comment on confusion over partial vs full refresh hopefully is mostly subdued with this recent commit. I'll wait here for any feedback, otherwise I can see about getting greyscale to work.

Side note: I'll remove any excess print statements at the end, for now I find them very helpful in debugging and I'd have to keep removing and including them otherwise.

@peterhinch
Copy link
Owner

Having thought about this some more, I fully agree with separate drivers for 1-bit and 2-bit modes. I suggest we get the 1-bit driver released first. On this basis the commented out 4gray code can be eliminated from the 1-bit driver. Have you considered using my _linv method (or similar) for fast inversion? An aim is to minimise latency when a micro-gui user performs an operation like pressing a button.

I am rather puzzled by set_full. Firstly why do we need the two args (absent in the V1 driver)? Secondly, resetting the chip seems rather drastic. The Waveshare code doesn't seem to do this. Resetting the chip may cause the mode change to be slow.

I'm also puzzled by the partial=False constructor arg. The V1 driver initialises the display in full mode. In general applications clear a display on initialisation - which implies full mode. On startup micro-gui does this automatically. I don't object to it given the default - but can you foresee an application which would want old display content to be retained on power-up?

Re the chip, I'm 99% sure that the chip you photographed is a level shifter for the white connector. The wiki indicates that a level shifter is provided for (5V) Arduino, and the PCB traces show links to the white connector. I think the controller chip is on the flexible PCB out of sight. The wiki includes a spec for the UC8176 plus another unspecified chip spec but neither seem to correspond with their own code. I'll try raising a request with Waveshare.

- remove update_partial and force_update in set_full method
- remove clear on init
- amend set_full method
- add set partial code to set_partial method
- remove set partial code from show_partial
- wait on self._busy_pin instead of not self._busy_pin
@surdouski
Copy link
Author

I am rather puzzled by set_full. Firstly why do we need the two args (absent in the V1 driver)? Secondly, resetting the chip seems rather drastic. The Waveshare code doesn't seem to do this. Resetting the chip may cause the mode change to be slow.

Removed the args. I was confused on my first go when writing this driver, as I hadn't done so before. The fact that things are working as expected in this current iteration of PR means that some of my previous assumptions were incorrect and that I now have a better understanding.

Also, I updated it so that set_full doesn't do a hardware, nor software, reset. Should the software reset ever be used? I'm not entirely sure, and if so, when?

I'm also puzzled by the partial=False constructor arg. The V1 driver initialises the display in full mode. In general applications clear a display on initialisation - which implies full mode. On startup micro-gui does this automatically. I don't object to it given the default - but can you foresee an application which would want old display content to be retained on power-up?

The only thing I could think of wanting to retain old content was if the thing controlling the epaper device had to reset for some reason unrelated to graphics, but didn't want user operation to be affected by it. That being said, I'm not sure it's worth the additional complexity, so I changed this.

Have you considered using my _linv method (or similar) for fast inversion? An aim is to minimise latency when a micro-gui user performs an operation like pressing a button.

I may need further assistance on this; I'm not sure what I would need to do.

@surdouski
Copy link
Author

In regards to the most recent commit: the display was sitting on my desk and noticed that it wasn't updated after the first couple times. Updated some things and let it run for a few minutes on async and sync this time; seems to be working correctly now.

@peterhinch
Copy link
Owner

peterhinch commented Jul 7, 2024

Should the software reset ever be used?

My display drivers only do this on initialisation. The Waveshare drivers I've ported are similar.

The _linv function is a fast way of doing a bit inversion and copy of a buffer. It takes a source bytearray and a destination (typically a memoryview slice of the display buffer) and copies the contents of the source to the destination, inverting the bits. This is done 32 bits at a time: the length arg is thus the length of the source buffer // 4. Paste the following to see how it works:

@micropython.viper
def _linv(dest: ptr32, source: ptr32, length: int):
    n: int = length - 1
    z: uint32 = int(0xFFFFFFFF)
    while n >= 0:
        dest[n] = source[n] ^ z
        n -= 1

a = bytearray(20)  # Source
b = bytearray(40)  # Destination
m = memoryview(b)  # Memoryview into destination
_linv(m[12:], a, len(a) >> 2)
print(b)

Viper code can be a little unfamiliar but it is astonishingly fast! It can be comparable to Assembler.
[EDIT]
Re latest change it sounds as if they have changed the sense of the busy pin. See my code comment - arguably the V1 display was wrong.

@surdouski
Copy link
Author

In regards to the viper code, I think I'm understanding now, thanks. I read up on some documentation as well.

I added _linv and updated the reset to issue a sw reset command.

The only way I could think to re-use the code for the checking between ticks was to have it be a generator function. However, I'm not entirely happy with my approach and think it is probably more confusing than helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants