Skip to content

Conversation

@helmut64
Copy link
Contributor

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

helmut64 added 16 commits April 10, 2019 13:51
Added I2C mbed uspport via SSD1306I2C.h
…h 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
the initial work was the GNU Compiler
PAGEADDR 0 resets the value, no ending page address is allowed
This overcomes the around warpping problem
…dots.

Added setGeometry high/width optional parameters to allow custom bitmaps.
…default font.

I believe this is a very important feature because it eliminates
the memory requirement for a font never being used.
@marcelstoer
Copy link
Member

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

@squix78
Copy link
Collaborator

squix78 commented Jun 3, 2019

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

@helmut64
Copy link
Contributor Author

helmut64 commented Jun 3, 2019

@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
Copy link
Member

marcelstoer commented Jun 3, 2019

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
Copy link
Contributor Author

helmut64 commented Jun 4, 2019

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
Copy link
Member

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
Copy link
Contributor Author

helmut64 commented Jun 5, 2019

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
Copy link
Contributor Author

helmut64 commented Jun 5, 2019

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

  • uint16_t cursorX/cursorX for larger text scrolling
  • GEOMETRY_RAWMODE and setGeometry
  • The font lookup function const byte c must be const uint8_t ch otherwise Umlauts do not work.
  • SSD1306Wire.h ARDUINO_ARCH_AVR

@marcelstoer
Copy link
Member

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.

@marcelstoer marcelstoer closed this Jun 5, 2019
@helmut64
Copy link
Contributor Author

helmut64 commented Jun 6, 2019

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

@helmut64
Copy link
Contributor Author

helmut64 commented Jun 6, 2019

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

@helmut64
Copy link
Contributor Author

helmut64 commented Jun 6, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants