Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor edits needed for Teensy #154

Open
PaulStoffregen opened this issue Jul 3, 2023 · 4 comments
Open

Minor edits needed for Teensy #154

PaulStoffregen opened this issue Jul 3, 2023 · 4 comments
Assignees
Labels

Comments

@PaulStoffregen
Copy link

PaulStoffregen commented Jul 3, 2023

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:

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

lexus2k commented Jul 4, 2023

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

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

PaulStoffregen commented Jul 4, 2023

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants