lexus2k / ssd1306

Driver for SSD1306, SSD1331, SSD1351, IL9163, ILI9341, ST7735, PCD8544, Nokia 5110 displays running on Arduino/ESP32/Linux (Rasperry) platforms
MIT License
651 stars 125 forks source link

Minor edits needed for Teensy #154

Open PaulStoffregen opened 1 year ago

PaulStoffregen commented 1 year ago

This library needs minor changes to work with Teensy. But these edits aren't Teensy-specific and probably will help with other modern boards. Problem was reported on this forum thread:

https://forum.pjrc.com/threads/67585-util-delay-h-no-such-file?p=328244&viewfull=1#post328244

Here is a first attempt to fix:

https://github.com/lexus2k/ssd1306/compare/master...PaulStoffregen:ssd1306:master

Quick summary of these changes:

  1. Default config in ssd1306_hal/arduino/io.h enables AVR-specific features. Just checking #if defined(__AVR__) allows non-AVR hardware to work by default.
  2. ssd1306_hal/arduino/platform.cpp attempts to include Wire1.h, which doesn't exist on many platforms. Adding a __has_include check solves this.
  3. ssd1306_platform_i2cInit() doesn't handle boards with 3 or 4 I2C ports. Simple to add.
  4. ssd1306_platform_i2cInit() defaults to last port on boards with 2+ I2C ports, when busID is -1. Reorder if-else to default to main Wire port.
  5. ssd1306_platform_i2cInit() unhandled cases leave a NULL pointer, which later crashes when used. Add fallback test to default to main Wire port.

Confirmed working with Teensy 3.0 and Teensy 4.1 using ssd1306_demo example.

ssd1308_t30

ssd1308_t41

I hope you will consider merging some form of these minor edits, so Teensy boards can work. Other non-AVR boards will very likely also benefit from these edits.

I could send a pull request if you would like. I can also arrange to send you Teensy 4.0 and Teensy 4.1 hardware for future development and testing.

lexus2k commented 1 year ago

Mostly ssd1306 is C-style library and I don't think that __has_include is supported by all compilers. Regarding your fix: TWI is not supported for all AVR controllers as well as AVR SPI. This will break other platforms. I agree with other your proposals.

PaulStoffregen commented 1 year ago

The __has_include check is written to first check whether the compiler supports __has_include.

#if !defined __has_include || __has_include(<Wire1.h>)

The #include <Wire1.h> is skipped only when the compiler both supports __has_include and when __has_include indicates no Wire1.h file exists.

PaulStoffregen commented 1 year ago

As the code #else case is written today, all platforms (not matched by the specific cases) without TWI using are already broken, because CONFIG_TWI_I2C_AVAILABLE is always defined. Checking only for __AVR__ shouldn't break anything that isn't already broken, but it would indeed leave unknown AVR platforms without TWI broken.

Maybe a better test would look something like this?

    #if defined(__AVR__) && defined(TWBR) && defined(TWCR)
        #define CONFIG_AVR_SPI_AVAILABLE
    #endif
PaulStoffregen commented 1 year ago

If it would help, I can arrange to send you a free Teensy 4.0 and Teensy 4.1 for software testing.