-
Notifications
You must be signed in to change notification settings - Fork 124
Support for EPDIY Hardware #77
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
base: main
Are you sure you want to change the base?
Conversation
read height/width from bmp header
…thout custom compile
don't require a flashing refresh when painting on white
|
Sry, overlooked the var name change. should be fine now. :) |
|
thanks @Lukas-Heiligenbrunner ! i think we're OK folding this in, just wanna get a few other outstanding PRs merged first, push a new release, then can review this independently. adding some docs in the meantime would be nice, even if just a link to a Gist. we could add them to the official DIY docs: https://docs.usetrmnl.com/go/diy/introduction |
furkanmustafa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made a basic review (just by reading code, I haven't tested anything, including my suggestions).
It seems to me that this PR achieves non-intrusion of defaults perfectly, when added, it would not break or change any current behavior with TRMNL devices. It can be merged as-is. ✔️
My (partly incomplete) suggestions point towards a direction of more generalization of arbitrary device support, with having TRMNL specifics as defaults everywhere possible.
And if possible, the following would be even better;
- much less mentions of
#ifdef EPDIY - use something like
DEVICE_TYPE=EPDIYinstead ofEPDIY - use feature toggling instead, wherever possible, like
#if PIN_INTERRUPT >= 0- (and use
PIN_INTERRUPT=-1to disable it for a device configuration)
- (and use
- define even more system specifics as device configuration variables,
- like
ADC_DIVIDER=64.000(/128)*2) (andADC_DIVIDER=91.2981(/128)*1.402)) for EPDIY)
- like
- Allow downloading display native bpp images, and skip bpp conversion if the remote image is like that.
PS. My review itself might require a review.
| #ifdef EPDIY | ||
| #else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifdef EPDIY | |
| #else | |
| #ifndef EPDIY |
| { | ||
| Log.info("%s [%d]: Goto Sleep...\r\n", __FILE__, __LINE__); | ||
| #ifdef EPDIY | ||
| epd_poweroff(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| epd_poweroff(); | |
| epd_poweroff(); |
| BlackImage = NULL; | ||
| #endif | ||
| Log.info("%s [%d]: display\r\n", __FILE__, __LINE__); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifdef EPDIY | ||
| // image buffer (62 bytes for metadata | ||
| uint8_t buffer[((uint32_t) EPDIY_WIDTH * EPDIY_HEIGHT / 8) + 62]; | ||
| #else | ||
| uint8_t buffer[48062]; // image buffer | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(does not need to be addressed in this PR, but could be a general improvement)
48062 could also be expanded into meaningful constant calculation,
and EPDIY specifics can be eliminated here that way.
eg;
| #ifdef EPDIY | |
| // image buffer (62 bytes for metadata | |
| uint8_t buffer[((uint32_t) EPDIY_WIDTH * EPDIY_HEIGHT / 8) + 62]; | |
| #else | |
| uint8_t buffer[48062]; // image buffer | |
| #endif | |
| // in build settings: | |
| // TODO: define DISPLAY_WIDTH, DISPLAY_HEIGHT, DISPLAY_BPP with TRMNL defaults (800x480 1bpp) | |
| // TODO: define IMAGE_BPP (default = DISPLAY_BPP) | |
| // in `bl.h`: | |
| // in bytes - 62 bytes - header; 48000 bytes - bitmap (480*800 1bpp) / 8 | |
| #define DISPLAY_BMP_IMAGE_SIZE (((uint32_t)DISPLAY_WIDTH * DISPLAY_HEIGHT * IMAGE_BPP / 8) + 62) | |
| uint8_t buffer[DISPLAY_BMP_IMAGE_SIZE]; |
| #ifndef EPDIY | ||
| if (width != 800 || height != 480 || bitsPerPixel != 1 || imageDataSize != 48000 || colorTableEntries != 2) | ||
| return BMP_BAD_SIZE; | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifndef EPDIY | |
| if (width != 800 || height != 480 || bitsPerPixel != 1 || imageDataSize != 48000 || colorTableEntries != 2) | |
| return BMP_BAD_SIZE; | |
| #endif | |
| if (width != DISPLAY_WIDTH || | |
| height != DISPLAY_HEIGHT || | |
| bitsPerPixel != IMAGE_BPP || | |
| imageDataSize != (DISPLAY_WIDTH*DISPLAY_HEIGHT*IMAGE_BPP/8) || | |
| colorTableEntries != (1<<IMAGE_BPP)) | |
| return BMP_BAD_SIZE; |
| #ifdef EPDIY | ||
| void convert_1bit_to_4bit(const uint8_t *fb_1bit, uint8_t *fb_4bit, int width, int height) { | ||
| int byte_width = ((width + 31) / 32) * 4; // Each byte in 1-bit framebuffer stores 8 pixels | ||
| int row_size_out = width / 2; // Each row in the 4-bit framebuffer (2 pixels per byte) | ||
|
|
||
| for (int y = 0; y < height; y++) { | ||
| int out_row_index = (height - 1 - y) * row_size_out; // Flip Y-axis | ||
|
|
||
| for (int x = 0; x < width; x += 2) { | ||
| int byte_index = (y * byte_width) + (x / 8); | ||
| int bit_index1 = 7 - (x % 8); | ||
| int bit_index2 = 7 - ((x + 1) % 8); | ||
|
|
||
| uint8_t pixel1 = (fb_1bit[byte_index] >> bit_index1) & 1; | ||
| uint8_t pixel2 = (fb_1bit[byte_index] >> bit_index2) & 1; | ||
|
|
||
| fb_4bit[out_row_index++] = (pixel1 ? 0x0F : 0x00) << 4 | (pixel2 ? 0x0F : 0x00); | ||
| } | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional is not necessary. Compiler would eliminate dead code if unused anyway.
| #ifdef EPDIY | |
| void convert_1bit_to_4bit(const uint8_t *fb_1bit, uint8_t *fb_4bit, int width, int height) { | |
| int byte_width = ((width + 31) / 32) * 4; // Each byte in 1-bit framebuffer stores 8 pixels | |
| int row_size_out = width / 2; // Each row in the 4-bit framebuffer (2 pixels per byte) | |
| for (int y = 0; y < height; y++) { | |
| int out_row_index = (height - 1 - y) * row_size_out; // Flip Y-axis | |
| for (int x = 0; x < width; x += 2) { | |
| int byte_index = (y * byte_width) + (x / 8); | |
| int bit_index1 = 7 - (x % 8); | |
| int bit_index2 = 7 - ((x + 1) % 8); | |
| uint8_t pixel1 = (fb_1bit[byte_index] >> bit_index1) & 1; | |
| uint8_t pixel2 = (fb_1bit[byte_index] >> bit_index2) & 1; | |
| fb_4bit[out_row_index++] = (pixel1 ? 0x0F : 0x00) << 4 | (pixel2 ? 0x0F : 0x00); | |
| } | |
| } | |
| } | |
| #endif | |
| void convert_1bit_to_4bit(const uint8_t *fb_1bit, uint8_t *fb_4bit, int width, int height) { | |
| int byte_width = ((width + 31) / 32) * 4; // Each byte in 1-bit framebuffer stores 8 pixels | |
| int row_size_out = width / 2; // Each row in the 4-bit framebuffer (2 pixels per byte) | |
| for (int y = 0; y < height; y++) { | |
| int out_row_index = (height - 1 - y) * row_size_out; // Flip Y-axis | |
| for (int x = 0; x < width; x += 2) { | |
| int byte_index = (y * byte_width) + (x / 8); | |
| int bit_index1 = 7 - (x % 8); | |
| int bit_index2 = 7 - ((x + 1) % 8); | |
| uint8_t pixel1 = (fb_1bit[byte_index] >> bit_index1) & 1; | |
| uint8_t pixel2 = (fb_1bit[byte_index] >> bit_index2) & 1; | |
| fb_4bit[out_row_index++] = (pixel1 ? 0x0F : 0x00) << 4 | (pixel2 ? 0x0F : 0x00); | |
| } | |
| } | |
| } |
| if(width == 0 || height == 0) { | ||
| width = 800; | ||
| height = 480; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if(width == 0 || height == 0) { | |
| width = 800; | |
| height = 480; | |
| } | |
| if(width_b == 0 || height_b == 0) { | |
| width_b = width; | |
| height_b = height; | |
| } |
| uint16_t width = display_width(); | ||
| uint16_t height = display_height(); | ||
|
|
||
| #ifdef EPDIY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifdef EPDIY | |
| #if IMAGE_BPP == 1 && DISPLAY_BPP == 4 |
| Log.info("%s [%d]: Paint_NewImage %s\r\n", __FILE__, __LINE__, _err); | ||
| delete[] image_buffer_4bpp; | ||
| epd_poweroff(); | ||
| #else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #else | |
| #elseif IMAGE_BPP != DISPLAY_BPP | |
| #error Unsupported display configuration for BPP conversion | |
| #else |
| #ifdef EPDIY | ||
| #else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #ifdef EPDIY | |
| #else | |
| #ifndef EPDIY |
EPDIY is a open hardware project for epaper displays:
https://github.com/vroland/epdiy
I've ported your firmware to this platform.
I tried to be as non-intrusive as possible.
All of the changes are only compiled if you target the epdiy platformio target.
Display size + display type are configureable with an env variable.
Text rendering is still missing and I'll add this later.
I've used this for the last several weeks and it works great so far.
Please review my code. If you guys are not interested in this I'll continue it in a soft-fork. :)
And should i add documentation somewhere? ;)
I've emailed @ryanckulp if you guys are interested in porting this back.
Thanks for your time.
-Lukas