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

feat(esp_qemu_rgb_panel): implement 16bpp QEMU RGB panel (IEC-77) #287

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

horw
Copy link
Member

@horw horw commented Dec 27, 2023

Checklist

  • Component contains License
  • Component contains README.md
  • Component contains idf_component.yml file with url field defined
  • Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Change description

Please describe your change here

@CLAassistant
Copy link

CLAassistant commented Dec 27, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title feat(esp_qemu_rgb_panel): implement 16bpp QEMU RGB panel feat(esp_qemu_rgb_panel): implement 16bpp QEMU RGB panel (IEC-77) Dec 27, 2023
Copy link
Collaborator

@o-marshmallow o-marshmallow left a comment

Choose a reason for hiding this comment

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

Thank you for this improvement!
I left a few remarks

esp_lcd_qemu_rgb/interface/esp_lcd_qemu_rgb.h Outdated Show resolved Hide resolved
esp_lcd_qemu_rgb/interface/esp_lcd_qemu_rgb.h Outdated Show resolved Hide resolved
@horw horw force-pushed the feat/qemu-rgb-16bpp branch 2 times, most recently from 81e7816 to fe47e94 Compare December 27, 2023 03:09
@horw
Copy link
Member Author

horw commented Dec 27, 2023

Thank you for this improvement! I left a few remarks

Thanks for the review! The remark has been resolved.

@suda-morris
Copy link
Collaborator

should we upgrade the component version?

@horw horw force-pushed the feat/qemu-rgb-16bpp branch 3 times, most recently from 78f4e51 to e76bef8 Compare January 10, 2024 09:43
@horw horw requested a review from o-marshmallow January 10, 2024 09:44
@o-marshmallow
Copy link
Collaborator

Changes, LGTM!

@igrr
Copy link
Member

igrr commented Jan 10, 2024

Please also bump the version in idf_component.yml.

@horw horw force-pushed the feat/qemu-rgb-16bpp branch from e76bef8 to cba281d Compare January 10, 2024 10:20
@horw
Copy link
Member Author

horw commented Jan 10, 2024

Please also bump the version in idf_component.yml.

Yes sure, has been bumped to 1.0.1.

@igrr
Copy link
Member

igrr commented Jan 10, 2024

@horw Looks good, just two final things:

  • Please click through the CLA (link in the 2nd comment)
  • Please fetch the latest master and rebase your branch.

@horw horw force-pushed the feat/qemu-rgb-16bpp branch from cba281d to eb423d5 Compare January 11, 2024 02:04
@igrr
Copy link
Member

igrr commented Jan 16, 2024

@horw There are still some failing jobs, could you please take a look? For example here pytest-embedded has failed to install.

@horw horw force-pushed the feat/qemu-rgb-16bpp branch 5 times, most recently from d414d28 to ae1ce09 Compare January 17, 2024 03:41
@horw
Copy link
Member Author

horw commented Jan 17, 2024

@igrr, from what I can see, the main issue with the failed job was primarily related to cryptography. Therefore, I have changed the pip install flag from --only-binary to --prefer-binary for the cryptography package.

@igrr
Copy link
Member

igrr commented Jan 17, 2024

Looks like there is one more thing to update, it is this sentence in the readme file:

Please note that the virtual RGB panel currently only supports ARGB8888 (32-bit) color mode.

@horw horw force-pushed the feat/qemu-rgb-16bpp branch from ae1ce09 to f5eac11 Compare January 17, 2024 07:21
@horw horw force-pushed the feat/qemu-rgb-16bpp branch from f5eac11 to c399bb3 Compare January 17, 2024 07:24
@igrr igrr merged commit 4e5c5ad into espressif:master Jan 22, 2024
71 checks passed
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.

5 participants