Skip to content

Conversation

@marekmatej
Copy link
Contributor

Update and add missing Espressif board images.

@marekmatej marekmatej added this to the v4.0.0 milestone Nov 7, 2024
@zephyrbot zephyrbot added the size: XS A PR changing only a single line of code label Nov 7, 2024
@sylvioalves
Copy link
Contributor

esp32_devkitc_* has .jpg file but I think it is not used in the documentation. Can you update as well?

@sylvioalves sylvioalves requested a review from kartben November 7, 2024 22:31
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

Very nice!
Can you please use webp format instead, it will give much smaller files which is important to keep the main git repo size under control. Ideally you even have images with transparent background (which webp supports unlike jpg)
You can use cwebp -resize 600 0 original_image.ext to perform the conversion from the original image (either the ok mage files from this PR or, better, from the actual original / high resolution files you started from)

Thanks again!

@marekmatej marekmatej force-pushed the bugfix/esp_board_pics branch from 88369f9 to e9b9d05 Compare November 8, 2024 00:15
@marekmatej marekmatej requested a review from kartben November 8, 2024 00:16
nordicjm
nordicjm previously approved these changes Nov 8, 2024
kartben
kartben previously approved these changes Nov 8, 2024
Copy link
Contributor

@kartben kartben left a comment

Choose a reason for hiding this comment

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

great work - thanks!

Note: ESP8684-DevKitM still missing

@kartben
Copy link
Contributor

kartben commented Nov 8, 2024

Also, just opened #81126 since it would have been nice for this PR to trigger CI, except it didn't due to only touching image files.

wmrsouza
wmrsouza previously approved these changes Nov 8, 2024
Update and add missing Espressif board images.

Signed-off-by: Marek Matej <marek.matej@espressif.com>
@marekmatej
Copy link
Contributor Author

Note: ESP8684-DevKitM still missing

adding esp8684 + rebase

@marekmatej marekmatej dismissed stale reviews from wmrsouza, kartben, and nordicjm via fa75ba3 November 8, 2024 11:13
@marekmatej marekmatej force-pushed the bugfix/esp_board_pics branch from e9b9d05 to fa75ba3 Compare November 8, 2024 11:13
@kartben
Copy link
Contributor

kartben commented Nov 8, 2024

Note: ESP8684-DevKitM still missing

adding esp8684 + rebase

yay, a transparent one! :) (which I think the others could be too, btw, since the officials docs seem to have transparent hi-res PNGs that could be used as the source? https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32c3/hw-reference/esp32c3/user-guide-devkitc-02.html)

@marekmatej
Copy link
Contributor Author

Note: ESP8684-DevKitM still missing

adding esp8684 + rebase

yay, a transparent one! :) (which I think the others could be too, btw, since the officials docs seem to have transparent hi-res PNGs that could be used as the source? https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32c3/hw-reference/esp32c3/user-guide-devkitc-02.html)

Do you want me to replace all the images with the transparent bg ones?

@marekmatej
Copy link
Contributor Author

@kartben do you think this could make it to 4.0 release?

@kartben
Copy link
Contributor

kartben commented Nov 8, 2024

Note: ESP8684-DevKitM still missing

adding esp8684 + rebase

yay, a transparent one! :) (which I think the others could be too, btw, since the officials docs seem to have transparent hi-res PNGs that could be used as the source? https://docs.espressif.com/projects/esp-idf/en/v5.0/esp32c3/hw-reference/esp32c3/user-guide-devkitc-02.html)

Do you want me to replace all the images with the transparent bg ones?

Given that we wouldn't want to update them later since this forever clutters the main git repo, it would be great, yes. But I would also understand if you don't have the energy to do it.

@mmahadevan108
Copy link
Contributor

@kartben , can you please approve if this looks fine to you.

@mmahadevan108 mmahadevan108 merged commit 207da3a into zephyrproject-rtos:main Nov 9, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Boards area: Documentation platform: ESP32 Espressif ESP32 size: XS A PR changing only a single line of code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants