adafruit / Adafruit_EPD

e-paper display driver for Arduino
141 stars 57 forks source link

No good way to disable external SRAM #2

Closed okofish closed 5 years ago

okofish commented 6 years ago

Adafruit_EPD.h provides a USE_EXTERNAL_SRAM macro, but #undef-ing that after the library is included won't work, as the preprocessor has already gone through the header files. As far as I can tell, to use onboard RAM a user is required to comment out USE_EXTERNAL_SRAM, so any projects using this library have to make their own copy :(

Would you accept a PR that adds an optional boolean useExternalSRAM argument to the Adafruit_EPD constructor? Memory impact should be minimal as the onboard buffers would still only be allocated if useExternalSRAM is false.

ladyada commented 6 years ago

sure we'll take a look! :) ideally you would be able to test if SRAM CS is <0 to know if its connected rather than have a new arg

okofish commented 6 years ago

Yeah that makes more sense. One gotcha is that the hardware SPI/external RAM and the software SPI/onboard RAM constructors have overlapping argument patterns (e.g. Adafruit_EPD(int, int, int8_t, int8_t, int8_t, int8_t, int8_t) is valid syntax for either constructor) so you'd have to remove the onboard RAM one, which would theoretically break existing code that uses it, but this lib is new and everyone's probably using external SRAM anyways so I doubt it'd be a problem.

ladyada commented 6 years ago

yah i'm ok changing it, or forcing the argument

romainreignier commented 6 years ago

I have faced the same issue so it is nice to see that someone is working on it. I was about to change the library using something like this in the sketch before including the library

#define USE_EXTERNAL_RAM 1
// or
#define USE_EXTERNAL_RAM 0

And then in Adafruit_EPD.h

#ifndef USE_EXTERNAL_RAM
 #define USE_EXTERNAL_RAM 1 // to keep the same default behaviour
#endif

And later in the code, replace

#ifdef USE_EXTERNAL_RAM

by

#if USE_EXTERNAL_RAM

But @okofish solution with an argument is nicer!

ladyada commented 5 years ago

done now if SRAM_CS is -1