Add LcmEn2r13 driver for Heltec Vision Master E213 and improve rotated Graphics rendering#1504
Open
garydyksman wants to merge 9 commits into
Open
Add LcmEn2r13 driver for Heltec Vision Master E213 and improve rotated Graphics rendering#1504garydyksman wants to merge 9 commits into
garydyksman wants to merge 9 commits into
Conversation
Introduce driver for LCMEN2R13EFC1 2.13" e-paper display (Heltec Vision Master E213), including SPI/GPIO handling and full drawing/refresh support. Add LcmEn2r13Sample project demonstrating usage. Enhance Graphics class with rotation-aware Width/Height, glyph mirroring, improved drawing methods, and better bounds checking. No breaking changes; new driver and sample are additive.
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new e-paper display driver for the LCMEN2R13EFC1 panel is introduced along with an accompanying sample. The Graphics class is refactored to support disposal control, logical rotation-aware dimensions, and improved pixel drawing. Existing drivers are updated with formatting changes and buffer optimizations. Project and solution files are updated to include the new driver and sample. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
garydyksman
changed the title
There was a problem hiding this comment.
Pull request overview
Adds support for the Heltec Vision Master E213’s LCMEN2R13EFC1 2.13" B/W e-paper panel by introducing a new ePaper driver and a sample app, and updates the ePaper Graphics helper to improve rotation-aware rendering and primitive drawing.
Changes:
- Added
LcmEn2r13display driver (SPI init, framebuffer upload/clear, refresh, power-down). - Added
LcmEn2r13SamplenanoFramework project demonstrating initialization and drawing. - Updated
Graphicsto be rotation-aware (logical Width/Height, fixed line drawing, pixel routing viaIEPaperDisplay.DrawPixel, text wrapping/mirroring option).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| devices/LcmEn2r13Sample/Properties/AssemblyInfo.cs | Adds sample assembly metadata (currently template-like). |
| devices/LcmEn2r13Sample/Program.cs | Sample demonstrating SPI + drawing + refresh on the new panel. |
| devices/LcmEn2r13Sample/packages.config | NuGet dependencies for the sample. |
| devices/LcmEn2r13Sample/LcmEn2r13Sample.nfproj | Sample project definition and references. |
| devices/ePaper/Graphics.cs | Rotation-aware width/height, line fix, pixel routing, text wrapping + optional glyph flip. |
| devices/ePaper/ePaper.sln | Adds the new sample project to the solution. |
| devices/ePaper/ePaper.nfproj | Includes the new driver file in the ePaper library project. |
| devices/ePaper/Drivers/JD796xx/LcmEn2r13.cs | New driver implementation for LCMEN2R13EFC1 panel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Member
|
@garydyksman please accept the policy. And also address the comments from Copilot. I'll also have a look. |
Ellerbach
reviewed
Apr 2, 2026
Member
Ellerbach
left a comment
Ellerbach
left a comment
There was a problem hiding this comment.
Overall all good. Couple of comments mainly related to where to find the magic numbers. And thanks also for the improvements in the Graphics.
Member
|
@garydyksman can you please address the review comments so we can merge and have this new device ? |
- update project and sample to use the single LcmEn2r13 namespace - restore busy-pin based refresh flow for HT-VME213 - remove extra sample refreshes and clean up SPI pin usage - reintroduce Heltec LUT-based partial refresh sequence - preserve correct panel orientation during partial refresh
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
devices/ePaper/Graphics.cs (1)
162-197:⚠️ Potential issue | 🟡 Minor
DrawTextsilently clips when text overflows the bottom.Horizontal overflow wraps onto a new line using rotation-aware
Width, but there is no corresponding check againstHeight. Oncey + line + font.Heightexceeds the display, glyphs are quietly dropped byDrawPixel's bounds check. Consider either early-exiting the loop when the next line would fall outside the display, or documenting this behavior on the method so callers know content beyond the bottom edge is silently discarded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@devices/ePaper/Graphics.cs` around lines 162 - 197, DrawText currently wraps horizontally using Width but never checks vertical bounds, so glyph rows past the bottom are silently dropped by DrawPixel; update DrawText (and the wrap logic that updates line) to check against Height and early-exit when the next glyph row would be entirely outside the display (e.g., if y + line + font.Height > Height) to stop processing further characters, or alternatively add a clear XML doc comment on DrawText stating that content past the bottom is discarded; refer to DrawText, DrawPixel, Width, Height, FlipGlyphsHorizontally and font.Width/font.Height when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@devices/ePaper/Drivers/LcmEn2r13/LcmEn2r13.cs`:
- Around line 48-86: Add a short comment above the five LUT arrays
(PartialLutVcom, PartialLutWhiteToWhite, PartialLutBlackToWhite,
PartialLutWhiteToBlack, PartialLutBlackToBlack) that cites the original vendor
source (datasheet section, vendor document or Heltec sample) and include the
exact reference (document name and section or URL) so maintainers can
trace/regenerate the waveforms; while editing, verify the source to confirm
whether PartialLutWhiteToWhite and PartialLutBlackToWhite being identical is
intentional and, if it is not, correct the duplicated table per the vendor data
and note the correction in the comment.
- Around line 255-283: PerformPartialRefresh currently powers the panel on and
triggers DisplayRefresh but never issues PowerOff, leaving DC/DC converters
running; update PerformPartialRefresh to follow the same shutdown pattern as
PerformFullRefresh by sending the PowerOff command after DisplayRefresh and
waiting for Ready plus the same small delay (use WaitReady() and WaitMs(...)
consistent with PerformFullRefresh), then call UpdatePreviousFrame as
appropriate; alternatively, if leaving the panel powered is intentional, add a
clear comment in PerformPartialRefresh explaining why it must remain powered and
document expected caller behavior.
- Around line 246-249: SetPosition currently throws NotImplementedException
which violates the IEPaperDisplay contract; change SetPosition in class
LcmEn2r13 to be a silent no-op by removing the exception and returning
immediately (or delegate to existing logic such as calling SetRamPointerToOrigin
if you opt to implement it); reference the SetPosition method and, if choosing
implementation, reuse SetRamPointerToOrigin to update RAM X/Y address counters
instead of throwing so generic consumers and Graphics callers won't crash.
- Around line 552-577: Dispose(bool disposing) unconditionally disposes
_spiDevice which conflicts with caller ownership; change the logic so _spiDevice
is only disposed when the driver owns it. Update the Dispose implementation in
LcmEn2r13.Dispose(bool) to dispose _spiDevice only if the ownership flag is true
(either reuse the existing _shouldDispose flag or add a new _shouldDisposeSpi
flag set via the constructor), and ensure the constructor/fields (where
_shouldDispose is set) are adjusted accordingly so that when the caller supplies
the SpiDevice the flag is false and the driver does not call
_spiDevice.Dispose().
- Around line 441-450: WaitReady currently waits for PinValue.High which is
inverted; change the busy-pin check in WaitReady to wait for PinValue.Low (use
_busyPin.WaitUntilPinValueEquals(PinValue.Low, ...)) so it blocks until the
controller is idle (align with IEPaperDisplay and Ssd168x). Also increase the
non-busy fallback delay in the _useBusyPin == false path (replace WaitMs(1500)
with a longer timeout such as 3000–4000 ms or make a named constant) or document
that PerformFullRefresh/PowerOn/DisplayRefresh/PowerOff require longer
full-refresh timing when hardware busy pin is not used.
In `@devices/ePaper/Graphics.cs`:
- Around line 59-65: The constructor change silently alters disposal semantics:
restore the previous default by making the optional parameter default to true so
Graphics(IEPaperDisplay) continues to dispose the wrapped IEPaperDisplay; update
the constructor signature (Graphics(IEPaperDisplay ePaperDisplay, bool
disposeDisplay = true)), ensure the existing _disposeDisplay field and
Graphics.Dispose() logic still use _disposeDisplay, and add a brief comment on
the constructor explaining ownership semantics so callers who want to retain the
display can pass disposeDisplay: false.
In `@devices/LcmEn2r13Sample/packages.config`:
- Around line 3-8: Program.cs calls Thread.Sleep (e.g., lines referencing
Thread.Sleep), but packages.config is missing the nanoFramework.System.Threading
package; add an entry for the nanoFramework.System.Threading package to
packages.config (use a compatible version for netnano1.0 consistent with the
other nanoFramework packages) so the System.Threading assembly is declared and
the sample can deploy.
In `@devices/LcmEn2r13Sample/Program.cs`:
- Around line 31-59: The GpioController instance (gpio) is never disposed while
LcmEn2r13 is constructed with shouldDispose: false, leaking the controller; fix
by either owning its lifetime (change to using var gpio = new GpioController();
so gpio is disposed when out of scope) or let the driver own it by removing the
shouldDispose argument or passing shouldDispose: true to the LcmEn2r13
constructor; update references to GpioController/gpio and the LcmEn2r13(...)
call accordingly.
- Around line 77-105: The loop currently calls PerformPartialRefresh() every
iteration after the first, causing potential ghosting; modify the loop in
Program.Main that uses firstFrame, fillFirstShape and calls
display.PerformPartialRefresh()/display.PerformFullRefresh() to insert a
periodic full refresh (e.g., count iterations with an int counter like
partialCount and when partialCount % 10 == 0 call display.PerformFullRefresh()
and reset or continue) so that after N partial refreshes you perform a full
refresh; ensure the initial firstFrame logic remains and increment/reset the
counter and toggle fillFirstShape as before.
In `@devices/LcmEn2r13Sample/Properties/AssemblyInfo.cs`:
- Around line 8-13: Update the assembly metadata attributes to match the rest of
the nanoFramework.IoT.Device samples: replace AssemblyTitle and AssemblyProduct
values that currently read "CSharp.BlankApplication" with the correct sample
name, set AssemblyCompany to the consistent organization name used across the
repo, and update AssemblyCopyright from "Copyright © iO 2026" to the same
copyright string used by other samples; modify the
AssemblyDescription/AssemblyConfiguration if needed to reflect the device/sample
purpose so the AssemblyTitle, AssemblyProduct, AssemblyCompany, and
AssemblyCopyright attributes are consistent with the project's conventions.
---
Outside diff comments:
In `@devices/ePaper/Graphics.cs`:
- Around line 162-197: DrawText currently wraps horizontally using Width but
never checks vertical bounds, so glyph rows past the bottom are silently dropped
by DrawPixel; update DrawText (and the wrap logic that updates line) to check
against Height and early-exit when the next glyph row would be entirely outside
the display (e.g., if y + line + font.Height > Height) to stop processing
further characters, or alternatively add a clear XML doc comment on DrawText
stating that content past the bottom is discarded; refer to DrawText, DrawPixel,
Width, Height, FlipGlyphsHorizontally and font.Width/font.Height when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0c1abe72-f189-4f84-a124-5c2c71b52689
📒 Files selected for processing (12)
devices/LcmEn2r13Sample/LcmEn2r13Sample.nfprojdevices/LcmEn2r13Sample/Program.csdevices/LcmEn2r13Sample/Properties/AssemblyInfo.csdevices/LcmEn2r13Sample/packages.configdevices/ePaper/Drivers/JD796xx/Gdew0154m09.csdevices/ePaper/Drivers/JD796xx/JD79653A.csdevices/ePaper/Drivers/LcmEn2r13/Command.csdevices/ePaper/Drivers/LcmEn2r13/LcmEn2r13.csdevices/ePaper/Graphics.csdevices/ePaper/ePaper.nfprojdevices/ePaper/ePaper.slndevices/ePaper/nuget.config
- align LcmEn2r13 frame flow with flush-then-refresh semantics - preserve partial refresh support and document Heltec-based LUT/init sequences - stop disposing caller-owned SpiDevice and fix DrawBitmap non-rotated fast path - clean up the sample metadata, package lock, and StyleCop dependency - update the sample to exercise full then partial refresh shape toggling
Author
|
@dotnet-policy-service agree |
Author
@josesimoes i have updated my PR. I have had an absolute blast working on the nanoFramework. I decided to use these ESP32 devices as a hackathon project for my team, I don't think they know what is possible with .Net. If you are interested i can give you more details and give feedback after the event. |
Member
Thanks! Yes, .NET nanoFramework is an AMAZING* platform :-) And yes, we're always interested to get some feedback and more details about such events! And you also have couple of feedback from Copilot! |
Member
|
@garydyksman nice!! Agreed: .NET nanoFramework can offer a great learning playground for development team. Would love to know more abou the event. Please do provide details. Here or just email me directly, or over Discord. 😉 |
- LcmEn2r13: no-op SetPosition, PowerOff after partial refresh, longer busy-pin fallback, init/prep doc pointers. - Graphics: default disposeDisplay true, DrawText stops when past logical height. - Jd79653A.EndFrameDraw: flush only (match Ssd168x); IEPaperDisplay WaitReady remark clarified. - Move LcmEn2r13Sample under ePaper/Samples; fix nfproj paths; add System.Threading package. - Sample: periodic full refresh; Graphics does not dispose driver when display is owned. - SSD1680/1681 samples: disposeDisplay false with nested usings. Made-with: Cursor
- Add nanoFramework copyright headers to LcmEn2r13Sample Program and AssemblyInfo. - Align AssemblyInfo with other ePaper samples (copyright string, drop Product and fixed AssemblyVersion). - LcmEn2r13.Clear: call UpdatePreviousFrame after writing white to RAM so partial refresh diffing stays consistent. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@devices/ePaper/Drivers/JD796xx/JD79653A.cs`:
- Line 69: The buffer allocation for _whiteBuffer in JD79653A.cs uses floor
division and can underrun when Width*Height isn't divisible by 8; change the
allocation to use ceiling division so partial final bytes are included (e.g.,
compute packedSize = (Width * Height + 7) / 8 or use an equivalent
Math.Ceiling-based expression) and replace the current new byte[(Width * Height)
/ 8] with new byte[packedSize]; ensure the same fix is applied for any similar
_blackBuffer or packed-buffer allocations in the class.
In `@devices/ePaper/Drivers/LcmEn2r13/LcmEn2r13.cs`:
- Around line 360-361: Add a named constant for the magic command 0x3C used in
the initialization sequence to match the existing style and improve readability:
define a descriptive constant (e.g., VendorCommandUnknown3C or CMD_0x3C) in the
LcmEn2r13 class and replace the literal used in the SendCommand call, and
optionally add a short comment explaining its purpose or that it is
vendor-specific where SendCommand(0x3C) and SendData(0x01) currently appear to
clarify intent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: nanoframework/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1175d99c-74b3-41e3-ae6a-c1013724e009
📒 Files selected for processing (12)
devices/ePaper/Drivers/IePaperDisplay.csdevices/ePaper/Drivers/JD796xx/JD79653A.csdevices/ePaper/Drivers/LcmEn2r13/LcmEn2r13.csdevices/ePaper/Graphics.csdevices/ePaper/Samples/LcmEn2r13Sample/LcmEn2r13Sample.nfprojdevices/ePaper/Samples/LcmEn2r13Sample/Program.csdevices/ePaper/Samples/LcmEn2r13Sample/Properties/AssemblyInfo.csdevices/ePaper/Samples/LcmEn2r13Sample/packages.configdevices/ePaper/Samples/LcmEn2r13Sample/packages.lock.jsondevices/ePaper/Samples/SSD1680Sample/Program.csdevices/ePaper/Samples/SSD1681Sample/Program.csdevices/ePaper/ePaper.sln
| PagedFrameDrawEnabled = enableFramePaging; | ||
|
|
||
| _whiteBuffer = new byte[Width * Height]; | ||
| _whiteBuffer = new byte[(Width * Height) / 8]; |
There was a problem hiding this comment.
Round up the packed buffer size.
Line 69 uses floor division, so geometries whose total pixel count is not divisible by 8 will allocate one byte too few and truncate the final partial byte during upload. Use ceiling division here instead.
Proposed fix
- _whiteBuffer = new byte[(Width * Height) / 8];
+ _whiteBuffer = new byte[((Width * Height) + 7) / 8];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| _whiteBuffer = new byte[(Width * Height) / 8]; | |
| _whiteBuffer = new byte[((Width * Height) + 7) / 8]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@devices/ePaper/Drivers/JD796xx/JD79653A.cs` at line 69, The buffer allocation
for _whiteBuffer in JD79653A.cs uses floor division and can underrun when
Width*Height isn't divisible by 8; change the allocation to use ceiling division
so partial final bytes are included (e.g., compute packedSize = (Width * Height
+ 7) / 8 or use an equivalent Math.Ceiling-based expression) and replace the
current new byte[(Width * Height) / 8] with new byte[packedSize]; ensure the
same fix is applied for any similar _blackBuffer or packed-buffer allocations in
the class.
Comment on lines
+360
to
+361
| SendCommand(0x3C); | ||
| SendData(0x01); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a named constant for command 0x3C.
Unlike other commands in the initialization sequence, 0x3C is used directly without a named constant or comment explaining its purpose. Consider adding a VendorCommandUnknown3C constant or a brief comment for consistency with the documented approach.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@devices/ePaper/Drivers/LcmEn2r13/LcmEn2r13.cs` around lines 360 - 361, Add a
named constant for the magic command 0x3C used in the initialization sequence to
match the existing style and improve readability: define a descriptive constant
(e.g., VendorCommandUnknown3C or CMD_0x3C) in the LcmEn2r13 class and replace
the literal used in the SendCommand call, and optionally add a short comment
explaining its purpose or that it is vendor-specific where SendCommand(0x3C) and
SendData(0x01) currently appear to clarify intent.
Ellerbach
reviewed
Apr 28, 2026
Member
Ellerbach
left a comment
Ellerbach
left a comment
There was a problem hiding this comment.
Really nice! few little nits and questions.
| _resetPin.Write(PinValue.High); | ||
| } | ||
|
|
||
| _dataCommandPin = _gpioController.OpenPin(dataCommandPin, PinMode.Output); |
Member
There was a problem hiding this comment.
check that the pin is a valid one (should be >= 0) and throw if not (technically it will be done in there but still a bit better)
| PagedFrameDrawEnabled = enableFramePaging; | ||
|
|
||
| _whiteBuffer = new byte[Width * Height]; | ||
| _whiteBuffer = new byte[(Width * Height) / 8]; |
| /// Positioned streaming writes are not used by this driver; callers should use <see cref="DrawPixel"/> or <see cref="Graphics"/>. | ||
| /// </remarks> | ||
| public void SetPosition(int x, int y) | ||
| { |
Member
There was a problem hiding this comment.
maybe throw a not implemented? So at least from a developer point of view it's clear?
| ///////////////////////////////////////////////////////////////// | ||
| // This attribute is mandatory when building Interop libraries // | ||
| // update this whenever the native assembly signature changes // | ||
| [assembly: AssemblyNativeVersion("0.0.0.0")] |
Member
There was a problem hiding this comment.
this can be removed as no dependency from native
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Implements SPI command/data handling, panel initialization, framebuffer upload, clear, full refresh, and power-down support.
Improves Graphics by fixing line drawing, using rotation-aware logical width and height, routing pixel drawing through IEPaperDisplay.DrawPixel(), improving text wrapping, and adding optional per-glyph horizontal flipping for mirrored text cases.
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
system, has no impact on code or features)
Checklist: