Bugfix Adafruit_ILI9341_STM

[WereCatf]

readcommand8() toggles _sclk, but when using hwSPI it's set to 0, ie. readcommand8() ends up toggling GPIO0, which it obviously shouldn't be doing. Also, when SPI-transactions had been enabled the SPI-bus speed was set to only 8MHz -- now we set it to 36MHz on STM32F1.

[rogerclarkmelbourne]

Makes sense

I'll need to double check what sets _sck. (perhaps defaulting to a completely invalid pin number, e.g. -1 would be better than defaulting to 0, but then we'd need to test for -1)

I'll need to test it and I will also reformat the line, as I'm not that keen on code where the body of the if is on the same line and doesn't have brackets around it

I know it compiles to the same code, but not having brackets around the body of the if is one way bugs get introduced into code (its a classic) albeit this tends to be when the body of the if is indented on the line below the if.

[WereCatf]

I went and changed it, I remembered that whoever made this version of the library ripped out all the soft-SPI stuff and _sck is only used for soft-SPI, so the line can simply be commented out altogether.

[rogerclarkmelbourne]

LOL

I did wonder why you mentioned soft-SPI, as I wasnt aware the library supported that ;-)

I'll double check it doesnt cause any issues, and then hopefully action the PR.

[WereCatf]

My version of the library is the stock library, just with STM32F1 - and ESP8266 - optimizations and a few extra functions, so it still supports soft-SPI, too, and that's where the brainfart came from :)

[rogerclarkmelbourne]

OK
I've taken a look, and I think its best to go with something like your original fix

However I think the logic was the wrong way around.

the define is for Use Hardware SPI.

So I presume that we only need to play with _sck if hwSPI is false, so something like this

if (!hwSPI)
{
digitalWrite(_sclk, LOW);
}

The only slight problem is, I can't see an easy way to test this, as nothing in the repo actually seems to use readcommand8()

Also I notice there is a PR open in the original Adafruit lib for this

https://github.com/adafruit/Adafruit_ILI9341/pull/2/files

[WereCatf]

https://github.com/rogerclarkmelbourne/Arduino_STM32/blob/master/STM32F1/libraries/Adafruit_ILI9341_STM/examples/graphicstest/graphicstest.ino does use the command, feel free to play around with it.

Alas, as I said, whoever made this version of the library already made soft-SPI quite unworkable as they removed the hwSPI()-check from drawFastVLine, drawFastHLine and fillScreen -- they all should be fixed if you wanna keep soft-SPI support around, IMHO.

[rogerclarkmelbourne]

arrgg

Thanks

I did a local search in the C, CPP, and H files for references to that command not in .INO (as its not in a standard file pattern in notepad++

Agreed. SWSPI looks rather messed up and should probably be removed, especially as the STM32F103C has 2 choices of SPI pins (SPI1 and SPI2), so there isnt a lot of excuse for wanting SW SPI

[WereCatf]

Well, I can clean up my pull-request and rather make another one that just removes all soft-SPI-stuff, if you'd like. Not today, though, I'm trying to get my server back on its feet.

[rogerclarkmelbourne]

Why did you change the code here

+#ifdef STM32F1

• SPI.beginTransaction(SPISettings(36000000, MSBFIRST, SPI_MODE0));
  +#else
  SPI.beginTransaction(SPISettings(8000000, MSBFIRST, SPI_MODE0));
  SPI.beginTransaction(SPISettings(8000000, MSBFIRST, SPI_MODE0));
  +#endif

I presume this is because the SPI settings were for AVR where clock is 16Mhz hence DIV2 is 8Mhz, ?

This doesn't seem related to the other change.

[WereCatf]

No, it's not related to the other change, it's just a bugfix-change, as the title says; there's no point in running the SPI-bus at 8MHz when the STM32 can do a lot more than that and the ILI9341 can handle it. The change obviously only applies if using SPI-transactions.