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

lcd: fix incompatible feature #244

Merged

Conversation

Lzw655
Copy link
Collaborator

@Lzw655 Lzw655 commented Oct 30, 2023

ESP-BSP Pull Request checklist

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

  • Version of modified component bumped
  • CI passing

Change description

@Lzw655
Copy link
Collaborator Author

Lzw655 commented Oct 30, 2023

@tore-espressif @espzav PTAL, thank you!

@Lzw655 Lzw655 force-pushed the bugfix/fix_lcd_incompatible_feature branch from 5e4e362 to 41ec602 Compare November 1, 2023 09:52
@Lzw655 Lzw655 force-pushed the bugfix/fix_lcd_incompatible_feature branch from 41ec602 to f2e2d73 Compare November 1, 2023 13:35
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.

@Lzw655 Thank you for fixes and changes! LGTM

@espzav espzav merged commit c4f0cd7 into espressif:master Nov 1, 2023
20 checks passed
.quadhd_io_num = -1, \
.quadwp_io_num = -1, \
.max_transfer_sz = SPI_LL_DMA_MAX_BIT_LEN >> 3, \
#define GC9A01_PANEL_BUS_SPI_CONFIG(sclk, mosi, max_trans_sz) \
Copy link
Member

Choose a reason for hiding this comment

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

@Lzw655 @espzav Adding another parameter looks like a breaking API change. Should the version be changed to 2.0.0 instead of 1.2.0?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, I thought it didn't need to be changed since the 1.1.0 version has been marked yanked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@igrr Can we remove the 1.1.0 version from the ESP Registry?

Copy link
Member

Choose a reason for hiding this comment

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

@tore-espressif Could you please help with this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought it didn't need to be changed since the 1.1.0 version has been marked yanked.

Breaking changes are allowed only between major releases, eg. 1.1.0 -> 2.0.0.

Can we remove the 1.1.0 version from the ESP Registry?

1.1.0 was yanked. We want to avoid deleting broken versions, to keep sync between git and component-registry.

@Lzw655 Ideally, you can push a fix that would garantuee compatibility -> 1.2.1. Or we can re-release this as 2.0.0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought it didn't need to be changed since the 1.1.0 version has been marked yanked.

Breaking changes are allowed only between major releases, eg. 1.1.0 -> 2.0.0.

Can we remove the 1.1.0 version from the ESP Registry?

1.1.0 was yanked. We want to avoid deleting broken versions, to keep sync between git and component-registry.

@Lzw655 Ideally, you can push a fix that would garantuee compatibility -> 1.2.1. Or we can re-release this as 2.0.0

Ok, I will submit a PR to bump their versions to 2.0.0.

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.

Compilation error with ESP IDF 5.1.1 (BSP-411) Compilation error on ESP IDF 5.0.4 (BSP-410)
4 participants