lexus2k / lcdgfx

Driver for LCD displays running on Arduino/Avr/ESP32/Linux (including Rasperry) platforms
MIT License
356 stars 51 forks source link

fix deprecated HSPI_HOST and VSPI_HOST #97

Closed vintzvintz closed 1 year ago

vintzvintz commented 1 year ago

Should fix #95, at least it compiles as an IDF v4.4 component. This PR is quite straightforward, but be aware I was not able to test for regressions on real SPI oled hardware.

Another busId mapping would be busId valid values 2 and 3, because SPI0_HOST is not software-accessible and SPI1_HOST is not easily available as general purpose SPI controller. Using the same numbering scheme as in official ESP documentation would be more intuitive. But such a change in ldcgfx API is likely to break existing projects.

So i tried to keep the same mapping : busId=0 -> HSPI_HOST, now SPI2_HOST busId=1 -> VSPI_HOST, now SPI3_HOST (default)

@Lexus2K : this is up to you

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

lexus2k commented 1 year ago

Will it be enough only to replace only constants VSPI_HOST and HSPI_HOST and not to change other code? Look at the failures below

vintzvintz commented 1 year ago

Yes i guess it would be OK to just replace the constants in-place. I just did that first, as a quick workaround ( i use I2C, not SPI, in my project). In ESP-IDF, VSPI_HOST and HSPI_HOST are already aliases for the "true" SPI2_HOST and SPI3_HOST constants, defined in an enum in a IDF header file.

But i can't see (yet) how my commit introduce "One Definition Rule" violations.

EDIT : Just ran the same CI tests on the fix_spi_host branch on my forked repo, It passed OK without any OneDefinitionRuleViolations. https://github.com/vintzvintz/lcdgfx/actions/runs/3619755497 My experience and knowledge are too weak to understand this, i'm just a casual hobbyist.