tinygo-org / drivers

TinyGo drivers for sensors, displays, wireless adaptors, and other devices that use I2C, SPI, GPIO, ADC, and UART interfaces.
https://tinygo.org
BSD 3-Clause "New" or "Revised" License
587 stars 180 forks source link

st7789: fix incorrect Rotation configuration #582

Closed aykevl closed 1 year ago

aykevl commented 1 year ago

The rotation as configured using st7789.Config was rotated 180°: 0° was configured as 180°, 90° was configured as 270°, etc. Presumably with the original test display, the ribbon cable was seen as the top of the screen while if you look at product photos it is usually at the bottom. Example: https://www.buydisplay.com/wide-angle-1-3-inch-240x240-color-ips-tft-display-st7789-controller Only Adafruit seems to sell these displays with the ribbon at the top: https://www.adafruit.com/product/3787

This patch fixes this mistake. It should be noted that this is backwards incompatible: all code that uses a st7789 will have to be modified to use the correct rotation instead of the previous incorrect rotation.

If this is too big of a change, we could just keep things as-is and pretend that all displays are upside down.

Also see: https://github.com/tinygo-org/drivers/issues/573#issuecomment-1593446268

conejoninja commented 1 year ago

I tested this and it works, since it fixes #573 I'm inclined to approved it even thought it's not backward compatible (solution is easy) Will it be released before gopherconEU? (to change the demo/tutorial code)

deadprogram commented 1 year ago

Will it be released before gopherconEU? (to change the demo/tutorial code)

I think it is better to do it now vs. waiting until afterwards.

conejoninja commented 1 year ago

Then, lgtm!

aykevl commented 1 year ago

since it fixes #573

It does not, but to fix #573 I needed a decision on whether to merge this or not (because a fix for #573 needs to know the rotation). I'm fine with reverting this if the backwards compatibility break is too big.

conejoninja commented 1 year ago

Already merged, so... :woman_shrugging: I prefer to fix this now instead of carrying to wrong rotation longer. Any broken code just need to rotate the display 180º, super easy

aykevl commented 1 year ago

Okay, then we'll leave it.