Initial support for Waveshare 2in13 v2 e-ink screen #51

dkm · 2020-08-23T13:45:19Z

This is an early version for 2.13 v2 e-Ink screen. It's an early version but is already working. I'll continue to test/polish it, but feel free to already give comments, I'm still new to this.

caemor · 2020-08-24T12:48:16Z

src/epd2in13_v2/mod.rs

+        // 00 : normal
+        // 01 : deep sleep mode 1
+        // 11 : deep sleep mode 2
+        self.cmd_with_data(spi, Command::DEEP_SLEEP_MODE, &[0x01])?;


What is the difference between these sleep modes?

No clue yet, as I have not tried to understand this part yet :D

I've updated this part. There are 3 modes:

1 where access through MCU is alive including reading RAM and RAM content is kept.

1 where MCU/RAM access is not possible but RAM content is still kept

1 where no access is possible and RAM content is not kept.

I've added corresponding enum

Do you know if this is also available on other epd from this newer generation?

I think the controller are at least very close, but they have some subtle diff that makes them incompatible. At least where I've looked. It sure would be a good think to factor what can be factored... We would need to have the exact controller used for all displays and see how much they can share.
I know this one is close to 2in9 as command number are the same... But usually, sample code for init is different.

src/epd2in13_v2/command.rs

caemor · 2020-08-24T13:12:05Z

Everything else looks good from a first glance.

dkm · 2020-08-24T13:36:10Z

Thanks for the comment. I'm still going from code to doc to try to understand how everything works.

dkm · 2020-08-24T13:38:27Z

It's still missing some tests... I was willing to try to add something similar to https://github.com/eldruin/ds323x-rs/blob/master/tests/datetime.rs : using the Mock interface to check for expected SPI transfer for a given code sequence. If that's something you would be ok with, of course. Still new to rust, I'm trying to learn :)

caemor · 2020-08-24T14:23:07Z

It's still missing some tests... I was willing to try to add something similar to https://github.com/eldruin/ds323x-rs/blob/master/tests/datetime.rs : using the Mock interface to check for expected SPI transfer for a given code sequence. If that's something you would be ok with, of course. Still new to rust, I'm trying to learn :)

Is there a real SPI transfer with this new epd? The old ones were only able to write, but not able to read anything over SPI.
If there is, sure go for it!

dkm · 2020-08-24T14:36:11Z

It's still missing some tests... I was willing to try to add something similar to https://github.com/eldruin/ds323x-rs/blob/master/tests/datetime.rs : using the Mock interface to check for expected SPI transfer for a given code sequence. If that's something you would be ok with, of course. Still new to rust, I'm trying to learn :)

Is there a real SPI transfer with this new epd? The old ones were only able to write, but not able to read anything over SPI.
If there is, sure go for it!

I don't know yet. It looks like it's still only for writing even if it seems possible to read RAM/temp sensor (maybe waveshare removed this ?). But still, we could check that for a given code sequence, the MOSI part is correct. What do you think ?

caemor · 2020-08-24T14:50:09Z

But still, we could check that for a given code sequence, the MOSI part is correct. What do you think ?

👍 Sounds good.

src/epd2in13_v2/command.rs

codecov-commenter · 2020-08-27T21:04:40Z

Codecov Report

Merging #51 into master will decrease coverage by 3.12%.
The diff coverage is 16.52%.

@@            Coverage Diff             @@
##           master      #51      +/-   ##
==========================================
- Coverage   30.77%   27.64%   -3.13%     
==========================================
  Files          25       28       +3     
  Lines        1290     1653     +363     
==========================================
+ Hits          397      457      +60     
- Misses        893     1196     +303

Impacted Files	Coverage Δ
src/epd2in13_v2/command.rs	`0.00% <0.00%> (ø)`
src/lib.rs	`100.00% <ø> (ø)`
src/traits.rs	`0.00% <ø> (ø)`
src/epd2in13_v2/mod.rs	`1.80% <1.80%> (ø)`
src/epd2in13_v2/graphics.rs	`93.33% <93.33%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82b8c98...27e367c. Read the comment docs.

dkm · 2020-08-28T22:18:41Z

I think the status is rather OK and may at least be enough for the review.
There are some minor issues, but they are not easy to fix. Mainly, the partial refresh does not play well with the API.
For the partial refresh to work correctly, the sequence should be:

build a buffer
display it
write this buffer in the RED RAM
repeat

The RED RAM is used as a base for comparison (I guess) and should reflect what is currently on the screen. Having functions like update_partial_frame or display_frame makes this task impossible as we can't read the RAM. You can still make it work if the user code call set_partial_base_buffer after having displayed something (you still need to be able to have the resulting buffer at hand...).

To avoid people using this and wondering why the screen displays garbage, I've added an assert!() that prevents the use of update_partial_frame(). You can still call display_frame() and make the screen display incorrect things.

dkm · 2020-08-28T22:24:18Z

(side note: I've added a test similar to existing ones, but could not really test it. I've made all my tests using a tm4c123 board. I do have a raspi but don't have the setup to build/run/execute rust on it)

dkm · 2020-08-29T17:45:49Z

Hmmmm, in fact, there's still an issue that I've not fixed yet. The visible (122) versus the controller size (128) of the screen. I need to take care of this before this gets merged.

caemor · 2020-09-02T09:26:36Z

Thanks for all your work.

(side note: I've added a test similar to existing ones, but could not really test it. I've made all my tests using a tm4c123 board. I do have a raspi but don't have the setup to build/run/execute rust on it)

You can just reference a testproject of yours with the tm4c123 instead if you want.

caemor

Overall it looks really good I just had a lot of small questions since this seems to be a slightly different architecture.

caemor · 2020-09-02T09:41:56Z

examples/epd2in13_v2.rs

+            background_color = White
+        ))
+        .draw(display);
+}


Are you showing anything new/special for this display? Else I would just keep the old examples and wouldn't add a new one for better visibility.

Maybe an example how to use the different partial refresh mode of the display would be a great example and would best also added to the docs for the epd2in13

I was trying to show what kind of animation you could do using the partial refresh. It's not simply quicker that the full refresh, it is quick enough for simple animation and does not flicker at all.

That sounds great then 👍 Maybe add a small comment to the spinning animation. Would also be great to add a small gif/link to a video of this on the frontpage/readme if you have one

I've added few comments in the example. Let me know if that's enough.
I could also do the GIF sure, but I would like to first have the code ACKed :)

src/epd2in13_v2/command.rs

src/epd2in13_v2/constants.rs

src/epd2in13_v2/mod.rs

caemor · 2020-09-02T10:59:59Z

src/epd2in13_v2/mod.rs

+        Ok(())
+    }
+
+    /// Beware that this does not work well with the screen partial refresh feature.


is there any use in this function for this display since the screen partial refresh feature should be better? And did you add the function for the screen partial refresh feature somewhere?

Maybe we could also use compile_error!("Your message") to make a compile time error here if it is always better to use the different method

Yes there is, as the partial refresh should draw more power I think (the analog and clock are left enabled) and it's not perfect. Some artifacts may be visible after several partial refreshes. In some case, it can be great, for other no so great. In my simple app, I'll use a full refresh for some events (eg. clock changing) and use partial for showing animation for alarms.

src/lib.rs

caemor · 2020-09-02T11:02:45Z

src/epd2in13_v2/mod.rs

+        self.cmd_with_data(spi, Command::WRITE_VCOM_REGISTER, &[vcom.0])
+    }
+
+    fn set_gate_driving_voltage(


Some short description for these helper functions would be nice, since they are different from the normal/old control functions.

caemor · 2020-09-02T11:08:31Z

I think the status is rather OK and may at least be enough for the review.
There are some minor issues, but they are not easy to fix. Mainly, the partial refresh does not play well with the API.
For the partial refresh to work correctly, the sequence should be:
* build a buffer

* display it

* write this buffer in the RED RAM

* repeat
The RED RAM is used as a base for comparison (I guess) and should reflect what is currently on the screen. Having functions like update_partial_frame or display_frame makes this task impossible as we can't read the RAM. You can still make it work if the user code call set_partial_base_buffer after having displayed something (you still need to be able to have the resulting buffer at hand...).

To avoid people using this and wondering why the screen displays garbage, I've added an assert!() that prevents the use of update_partial_frame(). You can still call display_frame() and make the screen display incorrect things.

So the problem is that you need to do it twice because the display is showing multiple colors? I thought it was only a black/white display (version 2.13 (A) v2, but you have a B or C one? Or what is the red ram doing? (I also asked the same question in the review already, so just answer it whereever you see it first :-D )

Your pr looks really good, just add a few more testcases (mainly for all the bitfields), adding the display to the readme and a few other small things are missing (see the review).

dkm · 2020-09-02T11:29:34Z

Thanks for all your work.

(side note: I've added a test similar to existing ones, but could not really test it. I've made all my tests using a tm4c123 board. I do have a raspi but don't have the setup to build/run/execute rust on it)

You can just reference a testproject of yours with the tm4c123 instead if you want.

Ok, will do that !

dkm · 2020-09-02T12:51:42Z

So the problem is that you need to do it twice because the display is showing multiple colors? I thought it was only a black/white display (version 2.13 (A) v2, but you have a B or C one? Or what is the red ram doing? (I also asked the same question in the review already, so just answer it whereever you see it first :-D )

Clearly, my doc is not enough :). The controller is using black and red names as it's meant for multicolor displays (1 RAM for Black/White, 1 RAM for Red). IIUC, this is also used for the partial refresh feature to store the «base» image (the one that should be displayed on the screen), hence the name in the code here. I don't know if it's a controller (but undocumented) feature, or simply a trick used by the display to implement its partial refresh.

dkm · 2020-09-18T20:15:33Z

Hey @caemor , I was hacking again the driver for 2in13, and I have a small issue I don't know how to best handle it, maybe you already had some similar issue ?

The display has 122 visible pixels, but everything being grouped by bytes, it means that some bytes only contain 2 bits of info, the other 6 are discarded/not visible. When the display is not rotated, it's «easy», the display could be declared as 250*122 and the buffer should be rounded up. But when the screen is rotated, the origin starts at 128, which is not visible.
GxEPD is handling this when drawing a pixel as it is doing everything in the same function (see https://github.com/ZinggJM/GxEPD/blob/39621c2e7ad699bfe35636b848ae1f62cfc35a7e/src/GxGDEH0213B72/GxGDEH0213B72.cpp#L66 ).
My best guess would be to have something in graphics.rs to have an offset added for some rotations.

dkm · 2020-09-20T20:06:56Z

My best guess would be to have something in graphics.rs to have an offset added for some rotations.

I've changed a bit how the rotation works and how buffer is allocated:

rotation works by changing the reference. I think it's easier to grasp and less error prone. It also works easily for my case as it doesn't need any special handling
i'm rounding up the buffer size for the X axis so that no bits are left behind and no need to declare a larger-than-real size to have enough space

dkm · 2020-09-21T19:42:12Z

(screen handling moved to #52 to reduce this PR)

caemor · 2020-09-22T11:41:03Z

I think #52 looks good. What is your current progress here?

dkm · 2020-09-22T18:21:13Z

I'm just starting to hack this again after a small pause. It's mostly ok as the last real problem was the screen size that will be handled by #52. I've used the bit_field crate as you requested, hope it's correct for you ?

Then I have to sort out the example/test. You mentioned I could link my external project, but it's still in early phase as I'm still getting all the parts together (and the screen was the last one). There should hopefully be a new keyboard project using this crate, so that would make 2 users. Would that work ? I can still use the same example as the other and leave out the very specific parts (partial refresh).

dkm · 2020-09-27T14:22:18Z

I've done a last refresh on this one. It's still based on #52.
I've uncommented 2 functions and invoked them with hardware reset values. It's not needed but it demonstrates how these settings can be changed if that's ever needed.
From what I've been able to test, everything seems correct.

The «only» part that may be perfectible is the handling of update_partial_frame() when the partial refresh mode is used. That's not supported and assert!() is used to filter this... The correct solution would be to have an error returned, but the trait WaveshareDisplay explicitly uses SPI::Error and I don't know how to best insert a custom error here. I guess having an assert + extensive comments in the code is good enough...

dkm · 2020-09-27T16:38:02Z

While investigating the coverage regression, I've found some small issues... So I'll iterate this and #52 at least one more time :)

dkm · 2020-09-27T17:20:01Z

Ok, I guess I need to add some test with «dumb» init values to have good coverage... But this will be very weak as there's no way I can really test the resulting state... I can have the code executed in a test and have codecov happy. It's true that the code is more or less dead.

caemor · 2020-10-05T16:40:27Z

Ok, I guess I need to add some test with «dumb» init values to have good coverage... But this will be very weak as there's no way I can really test the resulting state... I can have the code executed in a test and have codecov happy. It's true that the code is more or less dead.

Don't worry too much about the code coverage. Its just too see if there was a big regression or big improvement somewhere :-D

caemor · 2020-10-05T17:03:49Z

I've done a last refresh on this one. It's still based on #52.
I've uncommented 2 functions and invoked them with hardware reset values. It's not needed but it demonstrates how these settings can be changed if that's ever needed.
From what I've been able to test, everything seems correct.

The «only» part that may be perfectible is the handling of update_partial_frame() when the partial refresh mode is used. That's not supported and assert!() is used to filter this... The correct solution would be to have an error returned, but the trait WaveshareDisplay explicitly uses SPI::Error and I don't know how to best insert a custom error here. I guess having an assert + extensive comments in the code is good enough...

Error handling is currently a bit complicated, since e.g. the SPI::Error (write error) depends on the device the library is running on to be compatible to each. This situation will hopefully be improved by rust-embedded/embedded-hal#229 soon

This initial support covers both the full (slow) update of the display and the partial (fast) update.

caemor · 2020-10-05T18:48:25Z

Do you think anything is missing? It looks good to me.

dkm · 2020-10-05T18:50:32Z

I think everything is here. I've squashed all WIP commits :)

caemor · 2020-10-05T19:01:35Z

Thanks for your great work! :-)

Do you you already have a specific use case for this display?

dkm · 2020-10-05T19:08:35Z

I'm working on an alarm clock for kids with colored light instead of buzzer (pink => stay in bed, green => ok to get up, …). It's still WIP, but starts to work. A colleague will use it in a DIY keyboard. You want to have pointers to the projects when they are mature enough?

caemor · 2020-10-05T20:08:02Z

Sure that would be great

dkm force-pushed the master branch from 36ff873 to 5c21f8b Compare August 23, 2020 17:22

caemor reviewed Aug 24, 2020

View reviewed changes

src/epd2in13_v2/command.rs Show resolved Hide resolved

caemor reviewed Aug 25, 2020

View reviewed changes

src/epd2in13_v2/command.rs Outdated Show resolved Hide resolved

dkm force-pushed the master branch 4 times, most recently from b51c5fe to 12bff67 Compare August 27, 2020 21:01

dkm force-pushed the master branch 3 times, most recently from 27b8531 to 81171c4 Compare August 28, 2020 21:44

dkm marked this pull request as ready for review August 28, 2020 22:11

dkm changed the title ~~WIP: early support for 2in13 e-ink~~ Initial support for Waveshare 2in13 v2 e-ink screen Aug 28, 2020

dkm marked this pull request as draft August 29, 2020 17:46

caemor requested changes Sep 2, 2020

View reviewed changes

dkm force-pushed the master branch from 81171c4 to e410eec Compare September 2, 2020 11:56

dkm force-pushed the master branch from 093d86f to 365a6ce Compare September 18, 2020 18:41

dkm force-pushed the master branch from eeebb34 to f623762 Compare September 20, 2020 20:03

dkm force-pushed the master branch 2 times, most recently from 7245707 to a3ede7e Compare September 22, 2020 06:05

dkm force-pushed the master branch 2 times, most recently from fee7e72 to 4863836 Compare September 22, 2020 17:10

dkm force-pushed the master branch 2 times, most recently from d1cbbe5 to ed86b04 Compare September 27, 2020 14:06

dkm force-pushed the master branch from 197a332 to 8c9245f Compare September 27, 2020 17:13

caemor mentioned this pull request Oct 5, 2020

Better handling for screen size not divisible by 8 #52

Merged

caemor approved these changes Oct 5, 2020

View reviewed changes

Initial support for Waveshare 2in13 v2 e-ink screen

27e367c

This initial support covers both the full (slow) update of the display and the partial (fast) update.

dkm force-pushed the master branch from 8c9245f to 27e367c Compare October 5, 2020 18:49

dkm marked this pull request as ready for review October 5, 2020 18:49

caemor merged commit 221a801 into caemor:master Oct 5, 2020

Initial support for Waveshare 2in13 v2 e-ink screen #51

Initial support for Waveshare 2in13 v2 e-ink screen #51

Conversation

dkm commented Aug 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caemor commented Aug 24, 2020

dkm commented Aug 24, 2020

dkm commented Aug 24, 2020

caemor commented Aug 24, 2020

dkm commented Aug 24, 2020

caemor commented Aug 24, 2020

codecov-commenter commented Aug 27, 2020 • edited Loading

Codecov Report

dkm commented Aug 28, 2020

dkm commented Aug 28, 2020

dkm commented Aug 29, 2020

caemor commented Sep 2, 2020 • edited Loading

caemor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caemor commented Sep 2, 2020 • edited Loading

dkm commented Sep 2, 2020

dkm commented Sep 2, 2020

dkm commented Sep 18, 2020

dkm commented Sep 20, 2020

dkm commented Sep 21, 2020

caemor commented Sep 22, 2020

dkm commented Sep 22, 2020

dkm commented Sep 27, 2020

dkm commented Sep 27, 2020

dkm commented Sep 27, 2020

caemor commented Oct 5, 2020

caemor commented Oct 5, 2020

caemor commented Oct 5, 2020

dkm commented Oct 5, 2020

caemor commented Oct 5, 2020

dkm commented Oct 5, 2020

caemor commented Oct 5, 2020

codecov-commenter commented Aug 27, 2020 •

edited

Loading

caemor commented Sep 2, 2020 •

edited

Loading

caemor commented Sep 2, 2020 •

edited

Loading