search

ThingPulse / esp8266-oled-ssd1306

Driver for the SSD1306 and SH1106 based 128x64, 128x32, 64x48 pixel OLED display running on ESP8266/ESP32
https://thingpulse.com
Other
2k stars 638 forks source link

Possible feature request - support for additional display types #256

Closed bratoff closed 4 years ago

bratoff commented 4 years ago

You have done a marvelous job of factoring the UI and display functionalities and I am enjoying using your library. However, I did notice that by making just a few additional members of OLEDDisplay virtual, it becomes possible to support other displays with different interfaces and geometries. I have done this on a fork of your project, using the common PCD8544 SPI display as a test case. I have also modified one of the examples from your Weather Station library to dynamically adjust its output at runtime based on the display geometry.

If you would like to incorporate my changes, you are welcome to do so. The only library files I have changed are OLEDDisplay.cpp, OLEDDisplay.h and a new file PCD8544Spi.h, which implements the new display type. The new example is WeatherStationDemoWiFiManager.

My fork can be found at https://github.com/bratoff/esp8266-oled-ssd1306

marcelstoer commented 4 years ago

Thanks a lot Bruce.

I did notice that by making just a few additional members of OLEDDisplay virtual...

I gave the changes your fork a quick glance and it seems you changed a lot more than just that.

...support other displays with different interfaces and geometries

That would indeed be fantastic. However, we can't just merge your fork like that. That being said we would of course more than welcome a PR that includes only the changes necessary to support other displays.

P.S. @G6EJD is a regular contributor to our projects and an amateur radio operator like you.

bratoff commented 4 years ago

Thanks a lot Bruce.

I did notice that by making just a few additional members of OLEDDisplay virtual...

I gave the changes your fork a quick glance and it seems you changed a lot more than just that.

...support other displays with different interfaces and geometries

That would indeed be fantastic. However, we can't just merge your fork like that. That being said we would of course more than welcome a PR that includes only the changes necessary to support other displays.

P.S. @G6EJD is a regular contributor to our projects and an amateur radio operator like you.

Marcel, My only changes were as I described. However, it looks like there have been a number of changes between the current repository and the version retrieved by the Arduino library manager, which was my starting point. I shall re-integrate with the current repository and hopefully be able to provide a clean PR fairly quickly.

bratoff commented 4 years ago

Thanks a lot Bruce.

I did notice that by making just a few additional members of OLEDDisplay virtual...

I gave the changes your fork a quick glance and it seems you changed a lot more than just that.

...support other displays with different interfaces and geometries

That would indeed be fantastic. However, we can't just merge your fork like that. That being said we would of course more than welcome a PR that includes only the changes necessary to support other displays. P.S. @G6EJD is a regular contributor to our projects and an amateur radio operator like you.

Marcel, My only changes were as I described. However, it looks like there have been a number of changes between the current repository and the version retrieved by the Arduino library manager, which was my starting point. I shall re-integrate with the current repository and hopefully be able to provide a clean PR fairly quickly.

Finally had the time to resync and re-add my changes. PR on the way.

squix78 commented 4 years ago

Just adding my five cents: there actually is a more generalized version of this library: https://github.com/thingpulse/minigrafx It has a very similar interface and supports other types displays. I plan to port the existing display drivers from this library soon to minigrafx

bratoff commented 4 years ago

Just adding my five cents: there actually is a more generalized version of this library: https://github.com/thingpulse/minigrafx It has a very similar interface and supports other types displays. I plan to port the existing display drivers from this library soon to minigrafx

Very good. I'll take a look when time permits.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.