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

Add new device: Elecrow 5.0inch esp32 display #812

Merged
merged 21 commits into from
Sep 3, 2024

Conversation

bruxy70
Copy link
Contributor

@bruxy70 bruxy70 commented Aug 17, 2024

5-inch 800*480 resolution LCD display with ESP32 S3 microcontroller

Including the minimal configuration.
Plus a more extensive example provided by Elecrow technician for the video I am making (don't want to keep it for myself).
With the reference to Elecrow wiki with all parameters.

Copy link

netlify bot commented Aug 17, 2024

Deploy Preview for esphome-devices ready!

Name Link
🔨 Latest commit c1bd72c
🔍 Latest deploy log https://app.netlify.com/sites/esphome-devices/deploys/66d697ae2b235500084e576e
😎 Deploy Preview https://deploy-preview-812--esphome-devices.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 96 (🟢 up 4 from production)
Accessibility: 86 (no change from production)
Best Practices: 100 (no change from production)
SEO: 82 (no change from production)
PWA: 70 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

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

I just tried this out and it doesn't work. 🙈

  • The lvgl image configurations are invalid as they do not have a src.
  • Dont use the esp32s3box, use the generic esp32-s3-devkitc-1

Example configurations should not have secrets and passwords and should not make any assumptions about data that may or may not be available.

It is fine to have an lvgl example in here, but maybe it could be a bit more useful with buttons etc to show the touch working.

You should not add extra "soft" sensors like uptime, wifi info etc. Only hardware sensors present on a board should be included.

If you want a 1s update interval for the counter, use the interval component.

@esphome
Copy link

esphome bot commented Aug 22, 2024

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@esphome esphome bot marked this pull request as draft August 22, 2024 02:36
@bruxy70 bruxy70 marked this pull request as ready for review August 23, 2024 09:06
@esphome esphome bot requested a review from jesserockz August 23, 2024 09:06
@bruxy70
Copy link
Contributor Author

bruxy70 commented Aug 23, 2024

Hi. Thanks for the feedback. For lvgl part I did include the official example I got from the manufacturer. But I agree with you that it was too complicated and partially incorrect, so I made own example with one button and one meter.
I was thinking whether to include the lvgl snippet in general, but I think this is realistically much closer to the real use-case that the generic set of ESPHome graphics functions, and lvgl works much better on the display anyway.

@sprior
Copy link

sprior commented Aug 25, 2024

I have been following this merge because I'm using a slight variation of the Elecrow display. I was just able to get the LVGL version of the example code working with one exception - the 3 power_meter_input_img's are not defined in the example. I got it working by commenting out the "- image:" sections of each but it would be nice if this part of the example was fixed so we could see what it's supposed to look like.

@bruxy70
Copy link
Contributor Author

bruxy70 commented Aug 25, 2024

@sprior Sorry, I do not follow. I did change the lvgl example based on @jesserockz feedback 2 days ago - it does not contain the 3 images you mention.
There is now one and that one is defined. I also removed the API to HA. And added a button.
Or if I do not see it, what line number do you refer to?

@sprior
Copy link

sprior commented Aug 25, 2024

@bruxy70 Ok thanks, I was looking at the version on the website, didn't see the fixed version. I'm working with the MaTouch ESP32-S3 Parallel TFT with Touch 7“ (1024x600) board which so far I've got working with:
`display:

  • platform: rpi_dpi_rgb
    id: main_display
    color_order: RGB
    invert_colors: True
    update_interval: never
    auto_clear_enabled: false # takes too long to clear the display
    dimensions:
    width: 1024
    height: 600
    de_pin: 40
    hsync_pin: 39
    vsync_pin: 41
    pclk_pin: 42
    pclk_frequency: 16MHz

    hsync_front_porch: 40
    hsync_pulse_width: 48
    hsync_back_porch: 28
    vsync_front_porch: 13
    vsync_pulse_width: 3
    vsync_back_porch: 45

    data_pins:
    red:
    - 45 #r1
    - 48 #r2
    - 47 #r3
    - 21 #r4
    - 14 #r5
    green:
    - 5 #g0
    - 6 #g1
    - 7 #g2
    - 15 #g3
    - 16 #g4
    - 4 #g5
    blue:
    - 8 #b1
    - 3 #b2
    - 46 #b3
    - 9 #b4
    - 1 #b5`

And uses different pins for I2C:
i2c: sda: GPIO17 scl: GPIO18 scan: true

@sprior
Copy link

sprior commented Aug 26, 2024

@bruxy70 I just noticed that in the latest version of your index.md file you still mention esp32s3box up in the top Drivers section.

@bruxy70
Copy link
Contributor Author

bruxy70 commented Aug 26, 2024

@bruxy70 I just noticed that in the latest version of your index.md file you still mention esp32s3box up in the top Drivers section.

Thanks. I missed that one. Fixed. The examples are tested and working, so I hope it should now be ready to go?

@bruxy70
Copy link
Contributor Author

bruxy70 commented Aug 31, 2024

The status says change requested. But I made the changes. What do I have to update to update the status?

@jesserockz
Copy link
Member

The status says change requested. But I made the changes. What do I have to update to update the status?

Nothing. Someone (if not me) will come back around when time permits =)

Copy link
Member

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes/simplifications.
Compiled and installed this yaml as is and working nicely.

Copy link
Member

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

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

Minor capitalisation and spelling

src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
src/docs/devices/Elecrow-5inch-ESP32-display/index.md Outdated Show resolved Hide resolved
@jesserockz jesserockz merged commit bb6dec6 into esphome:main Sep 3, 2024
6 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.

3 participants