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
1.99k stars 637 forks source link

Fix for nonESP32 with one HW I2C #298

Closed benoitm974 closed 4 years ago

benoitm974 commented 4 years ago

Please note this is blind change as I don't have a non ESP32 arduino to test

benoitm974 commented 4 years ago

Thanks for the feedback ! glad it worked.

marcelstoer commented 4 years ago

I confirmed this fixes the issue. As you're at it please properly format the fragment as below (note the spaces)

#if !defined(ARDUINO_ARCH_ESP32)
      this->_wire = &Wire;
#else
      this->_wire = (_i2cBus == I2C_ONE) ? &Wire : &Wire1;
#endif
benoitm974 commented 4 years ago

I confirmed this fixes the issue. As you're at it please properly format the fragment as below (note the spaces)

#if !defined(ARDUINO_ARCH_ESP32)
      this->_wire = &Wire;
#else
      this->_wire = (_i2cBus == I2C_ONE) ? &Wire : &Wire1;
#endif

Done.

millenseer commented 4 years ago

Thanks for your continuous and amazing work!

thijstriemstra commented 4 years ago

can someone put out a new release please? cc @marcelstoer

marcelstoer commented 4 years ago

I'm reluctant to cut a release without #296 being addressed. It appears to be a regression but I currently don't have a clue.

thijstriemstra commented 4 years ago

A platformio build on multiple boards would help, and I see there's a .travis.yml file present but the build is not triggered on travis somehow, there are no 'green ticks' behind the commits indicating it ran a travis build. Can you verify/look into this @marcelstoer? Make sure this repository has travis enabled? I can help looking into why builds fail, but only if I can see if they fail (hence travis needs to enabled).

marcelstoer commented 4 years ago

I know, I looked into this the other day.

Travis CI is enabled and it does run for all commits (and PRs). The build badge at the top of the README at https://github.com/ThingPulse/esp8266-oled-ssd1306 is real. However, for whatever reason GitHub only triggers the builds, I verified this, but fails to pull in status information about those builds. I've seen this with some other projects I maintain and I even contacted GitHub support about this issue. They never replied but over there the feature came back after a while out of nowhere.

thijstriemstra commented 4 years ago

Ah I missed the badge. I'll open a pull request that tests against other boards. In theory, #296 should be caught by adding the correct device and a test case.

thijstriemstra commented 4 years ago

I'll open a pull request that tests against other boards.

See #300