Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BSP LCD display offset (BSP-611) #473

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adamkaliszan
Copy link

@adamkaliszan adamkaliszan commented Jan 6, 2025

ESP-BSP Pull Request checklist

Note: For new BSPs create a PR with this link.

  • Version of modified component bumped
  • CI passing

Change description

Please describe your change here

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2025

CLA assistant check
All committers have signed the CLA.

@adamkaliszan adamkaliszan marked this pull request as draft January 6, 2025 18:22
@github-actions github-actions bot changed the title BSP LCD display offset BSP LCD display offset (BSP-611) Jan 6, 2025
@adamkaliszan adamkaliszan marked this pull request as ready for review January 6, 2025 19:41
@espzav
Copy link
Collaborator

espzav commented Jan 7, 2025

Hi @adamkaliszan, thank you for this PR. Please, could you help me understand these changes? What is the purpose?

@tore-espressif
Copy link
Collaborator

@adamkaliszan We would like to avoid having this configurable through menuconfig.

esp_lcd offers esp_lcd_panel_set_gap function

https://github.com/espressif/esp-idf/blob/b5ac4fbdf9e9fb320bb0a98ee4fbaa18f8566f37/components/esp_lcd/include/esp_lcd_panel_ops.h#L88-L101

that performs the same thing you are trying to achieve. Would that solve your problem?

@adamkaliszan
Copy link
Author

My intention was to simplify launching new board. There is no need to make so deep changes and calling esp_lcd_panel_set_gap is enough.
In my opinion it would be very helpful to call esp_lcd_panel_set_gap in function bsp_display_lcd_init.

I have modified my PR and solved my problem by using esp_lcd_panel_set_gap.

Copy link
Collaborator

@espzav espzav left a comment

Choose a reason for hiding this comment

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

@adamkaliszan Thank you for these changes. Please, bump the version of BSP. LGTM

@adamkaliszan
Copy link
Author

@adamkaliszan Thank you for these changes. Please, bump the version of BSP. LGTM

OK, great. Its done.

@espzav
Copy link
Collaborator

espzav commented Jan 22, 2025

Will be merged after #478

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants