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

SH1106 compatibility (BSP-473) #308

Closed
wants to merge 1 commit into from

Conversation

mspivak
Copy link

@mspivak mspivak commented Mar 9, 2024

I successfully used this driver with a SH1106 display by only changing the display offset. I'd like to make this change available to everyone. This is not a new BSP, nor I would create an entirely new component for this small change, so I'm submitting this pull request to know if you want to take if from here or should I continue my plan.

I propose:

  • Changing the name of the component to esp_lcd_sh110x
  • Extend sh110x_panel_t to add offset_param two-byte param.
  • Update ESP_LCD_IO_I2C_SH1107_CONFIG to add the new offset_param set to 0xd3 0x60.
  • Create ESP_LCD_IO_I2C_SH1106_CONFIG with offset_param set to 0xd3 0x00.
  • Update docs accordingly.

Please confirm my plans are OK and I'll make them and re-submit the pull request.

Thanks!

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 Mar 9, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title SH1106 compatibility SH1106 compatibility (BSP-473) Mar 9, 2024
@tore-espressif
Copy link
Collaborator

Hi @mspivak thank you for your contribution!

We want to keep the API as flexible as possible; that means we are avoiding any hard-coded changes.

Could you please check whether function esp_lcd_panel_set_gap does what you want? See docs here https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/peripherals/lcd.html#_CPPv421esp_lcd_panel_set_gap22esp_lcd_panel_handle_tii

@mspivak
Copy link
Author

mspivak commented Mar 11, 2024

To clarify, this is not my final PR, just wanted to check the development workflow on my PR description before I really spend time on it. My proposal actually involves removing a hard-coded value in favour of a new, configurable offset_param parameter. Just OK me on this and I'll send a proper PR.

I tried esp_lcd_panel_set_gap, it's not what I want to do. My change involves a different startup sequence.

@VojtechBartoska VojtechBartoska added Area: Components Issue is related to any component Resolution: Awaiting response Issue is awaiting response from author labels Mar 13, 2024
@tore-espressif
Copy link
Collaborator

Closing due to lack for feedback. Feel free to reopen if there is anything new!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Components Issue is related to any component Resolution: Awaiting response Issue is awaiting response from author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants