atomic14 / diy-esp32-epub-reader

An ESP32 e-reader
MIT License
318 stars 43 forks source link

m5paper support #58

Closed cgreening closed 2 years ago

martinberlin commented 2 years ago

No worries I will never merge something if you mark it as "draft" :) I'm thinking in the direction that is not the double SPI config (Something you could also check disabling the SDCard and using SPIFFS)

But that epdiy is somehow instantiated and taking all DMA channels available before the M5 SPI config initializes. Only a theory but if the two SPI's are clearly differentiated and using different pins they should not conflict.

martinberlin commented 2 years ago

@cgreening test is failing:

In file included from src/main.cpp:16:
lib/Epub/Renderer/EpdiyRenderer.h:6:10: fatal error: EpdFrameBufferRenderer.h: No such file or directory

I think it’s because it’s using the Lilygo EPD47 environment. Is posible to set the environment dynamically?

on the other hand I get what you commented about preprocessor if conditionals. The question is if it’s worth the whole refactoring. To be sincere using EPDiy as a GFX library is the wrong concept because they are much more efficient and capable GFX components. But on the other hand like this you are saving a big refactoring and apart of M5 all the other epapers use EPDiy as a base component so I think although is not the cleanest alternative it’s one that can solve the problem and let you test to the end if M5 proves to work well. Then maybe at some point we can do a bigger refactoring. And for that we can draw a nice class dependence chart and try to figure out what it will be the best way to reorganize all the mess :)

cgreening commented 2 years ago

ok, I think this is ready to go - or as far as I can take it. It seems to be working nicely on the M5Paper. I can't get the SD Card to work reliably and I haven't implemented touch yet. But it works.

There is some work to do to make it more efficient at screen updates - I haven't implemented update-area yet so it always updates the entire screen.

I've tested it on the lilygo and it still seems to function as normal. I'm hoping that a small change I've made to the framebuffer dehydration may have made that work more reliably which should make going in and out of deep sleep a more seamless experience.

martinberlin commented 2 years ago

Nice I will try to flash this branch on both lily and EPDiy environments to have a second check. Have you checked my review comments?

tests still reports:

lib/m5Paper/src/M5EPD_Driver.cpp:1:10: fatal error: esp_log.h: No such file or directory

cgreening commented 2 years ago

Tests should be fixed now!

martinberlin commented 2 years ago

@cgreening note than default_env in platformio.ini should go back to lilygo_t5_47 before merging

cgreening commented 2 years ago

I’ll do that now - also need to fix the build as it’s failing on the certificate bundle.

On Thu, Oct 28, 2021 at 08:59, Martin @.***> wrote:

  • Tested this in LilyGo EPD47 and works correctly
  • Test with EPDiy V6

@cgreening https://github.com/cgreening note than default_env in platformio.ini should go back to lilygo_t5_47 before merging

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/atomic14/diy-esp32-epub-reader/pull/58#issuecomment-953595653, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFMTABM4PXEJD3D2AC7WALUJEGIZANCNFSM5G2FLZTA .

martinberlin commented 2 years ago

Well I think this is ready then. Just go and merge it. Today I’ll be adding the SD to the EPDiy board