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

Mbed os support #243

Closed helmut64 closed 5 years ago

helmut64 commented 5 years ago

All changes have been tested to work on Arduino (ESP32) and mbed-os Added display library support for mbed-os. At preset with SSD1306 with I2C, more displays and SPI will follow later.

Added for mbed a very simple Arduino String emulation which allows to call drawString using UTF8 chars.

Changed the Arduino byte into an uint8_t to be mbed compatible.

Support the mbed-os Stream class (Arduino uses Print)

mbed-os does not support C++11 initialisers in .h class files therefore I initialise variables in the constructor.

Changed initialisers functions (fontTableLookupFunction, loadingDrawFunction) to be separate functions to support the mbed-os compiler

To use the library for mbed-os copy the following files into your mbed-os project: OLEDDisplay.cpp OLEDDisplayFonts.h OLEDDisplayUi.h OLEDDisplay.h OLEDDisplayUi.cpp SSD1306I2C.h

marcelstoer commented 5 years ago

Looks like a great extension of this library. Can I ask you to fix the merge conflicts in README.md?

squix78 commented 5 years ago

Hi @helmut64 Many thanks for this contribution. And sorry for the longer delay... Daniel

helmut64 commented 5 years ago

@marcelstoer @squix78 Moin, thank you for getting back to me. Of course I love to contribute here to this great project. I will work on my patches to fix conflicts to get them merged when you are ready. Helmut

marcelstoer commented 5 years ago

Of course I love to contribute here to this great project.

👍 👏

That latest commit caused a bit of a mess. You've now got clearPixel() twice in your code and that broke the build.

I suggest you revert that (merge) commit and rebase your changes on the current master. Whether you also want to squash your commits is up to you. We can easily do this here on GitHub while merging the PR.

helmut64 commented 5 years ago

Hi Marcel, this was a conflict from my last merge, sorry for it. I updated it. I am not so confident with rebase therfore I just removed the duplicate line.

marcelstoer commented 5 years ago

I see... Two of the last last four commits in your branch are potentially problematic.

The other two are then just means to deal with those two.

I tried to address this in a separate branch. I took your branch, removed the last four commits, rebased it on master and pushed it to our repo. This retains you as the author of the changes so you can get proper OSS credits/karma for them 😉I'd now like to offer https://github.com/ThingPulse/esp8266-oled-ssd1306/compare/helmut64-mbed_os_support_clean?expand=1 as an alternative to this PR. Please take a look and check whether everything you expect to be there is actually there.

helmut64 commented 5 years ago

Dear Marcel, I don’t mind if I get mentioned in the Readme or source files, you are welcome to remove my name from this. I just like to help to enhance this library and to add support for mbed-os. Of course I am capable to do mbed-os and Arduino support when time allows.

I was hoping to get the mbed-os support merged into the head version.

helmut64 commented 5 years ago

PS: There are changes which are independent of mbed-os, e.g.:

marcelstoer commented 5 years ago

There must be a misunderstanding (or two) somewhere.

you are welcome to remove my name from this.

The exact opposite was my intention. I wanted to keep you as the author of the changes. #251 also still shows the "Copyright (c) 2019 by Helmut Tschemernjak" comments you added.

I don’t mind if I get mentioned in the Readme or source files,

See above. I will also add your name to the "Credits" section in the README.

I was hoping to get the mbed-os support merged into the head version.

Me too 😄 -> #251

There are changes which are independent of mbed-os

Yep, I noticed during review. It would now be a bit time consuming and yield little benefit to sort this out. Hence, we won't. However, in general we do appreciate if you create 1 PR for 1 feature. It's easier to review and reason about.

helmut64 commented 5 years ago

I will do better single pull requests in the future, thanks for working on it.

helmut64 commented 5 years ago

I verified that the new version works on the Arduino ESP32 and mbed-os, all fine. Thank you for your support.

helmut64 commented 5 years ago

I verified that the new version works on the ESP32 and mbed-os, all fine. Thank you for your support.