Skip to content

ili9341: Add color mode and inversion parameters#13105

Merged
aabadie merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/ili9341_add_config
Jan 13, 2020
Merged

ili9341: Add color mode and inversion parameters#13105
aabadie merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/ili9341_add_config

Conversation

@bergzand
Copy link
Copy Markdown
Member

@bergzand bergzand commented Jan 13, 2020

Contribution description

This PR enhances the ILI9341 driver with additional config parameters:

  • Add RBG mode in addition to the default BGR mode
  • Add display inverted mode, for when the TFT display is connected in inverted mode.

Testing procedure

Check if compilation of the test still works. I think @aabadie has an stm32f429i-disc1 available for the run-time checks :)

Furthermore on the PineTime, with RGB mode and inversion enabled, the colors should be correct.

Issues/PRs references

None, but maybe we should rename this driver to something more generic, e.g. spi_display and alias both ili9341 and st7789v modules to this.

@bergzand bergzand added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: drivers Area: Device drivers labels Jan 13, 2020
@bergzand bergzand requested review from aabadie and kaspar030 January 13, 2020 10:29
@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 13, 2020
@bergzand bergzand mentioned this pull request Jan 13, 2020
Merged
@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jan 13, 2020

Looking good. Should e.g., ili9341_invert_on()/off() take the new param into account for inverting?
As is, in the test application, ili9341_invert_on() has no effect, ili9341_invert_off() leads to inverted screen, on the pinetime.

@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented Jan 13, 2020

Looking good. Should e.g., ili9341_invert_on()/off() take the new param into account for inverting?

I think so yes. The behavior should be inverted when inversion is enable in the parameters.

@kaspar030
Copy link
Copy Markdown
Contributor

kaspar030 commented Jan 13, 2020

I think so yes. The behavior should be inverted when inversion is enable in the parameters.

+1

Please add and squash! I'll give it another test and we can merge this.

@bergzand bergzand force-pushed the pr/ili9341_add_config branch from 307eb10 to 146137e Compare January 13, 2020 16:09
@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented Jan 13, 2020

Please add and squash! I'll give it another test and we can merge this.

Amended the enhancement, thanks for testing of course!

kaspar030
kaspar030 approved these changes Jan 13, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK. Fixes color and inversion on PineTime.

@aabadie aabadie merged commit cfbab79 into RIOT-OS:master Jan 13, 2020
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kaspar030 kaspar030 kaspar030 approved these changes

@aabadie aabadie Awaiting requested review from aabadie

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

4 participants

@bergzand @kaspar030 @aabadie @fjmolinas