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

bsp: New BSP for M5Stack Core (BSP-594) #452

Merged
merged 11 commits into from
Dec 12, 2024

Conversation

Ocean-lhy
Copy link
Contributor

@Ocean-lhy Ocean-lhy commented Dec 4, 2024

ESP-BSP Pull Request checklist

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

  • Version of modified component bumped
  • CI passing

Change description

  • bsp: New BSP for M5Core
  • doc(m5stack_core_2): added configuration methods for 1.0 and 1.1

@CLAassistant
Copy link

CLAassistant commented Dec 4, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title bsp: New BSP for M5Stack Core bsp: New BSP for M5Stack Core (BSP-594) Dec 4, 2024
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.

I make only fast review for now. I left some comments.
Would it work with my old M5Stack Core (2018.3)?

bsp/m5stack_core/idf_component.yml Outdated Show resolved Hide resolved
bsp/m5stack_core/idf_component.yml Outdated Show resolved Hide resolved
bsp/m5stack_core/include/bsp/m5stack_core.h Outdated Show resolved Hide resolved
bsp/m5stack_core/m5stack_core.c Outdated Show resolved Hide resolved
bsp/m5stack_core/include/bsp/m5stack_core.h Outdated Show resolved Hide resolved
1. BSP version change to 1.0.0
2. remove useless part
3. use new I2C driver
4. use general buttons
@Ocean-lhy
Copy link
Contributor Author

I make only fast review for now. I left some comments. Would it work with my old M5Stack Core (2018.3)?

Thanks for your review. I've updated the code based on your suggestions. PTAL and resolve the conversation if the issue has been resolved.

By the way, I think the bsp will work on your old M5Stack Core, although I don't have the original M5Stack Core on hand. I checked for version changes and there were no changes that would cause compatibility issues. Perhaps you can test it with a simple lvgl demo.

Copy link
Collaborator

@tore-espressif tore-espressif 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 much for this BSP!

I had a brief look.
I found old M5Stack Core in my drawer, I'll test it soon

bsp/m5stack_core/include/bsp/m5stack_core.h Outdated Show resolved Hide resolved
bsp/m5stack_core/m5stack_core.c Outdated Show resolved Hide resolved
bsp/m5stack_core/m5stack_core.c Outdated Show resolved Hide resolved
1. remove esp_codec_dev
2. remove idf5
3. add i2c device deinit
4. modify description
@Ocean-lhy
Copy link
Contributor Author

I submitted the code according to feedback to see if the problem was improved?

@tore-espressif
Copy link
Collaborator

@Ocean-lhy The code changes LGTM!

To fix your CI issues, please install pre-commit with pip install pre-commit && pre-commit install
you can then run it on you last eg 4 commits with pre-commit run --from-ref HEAD~4 --to-ref HEAD

Could you also please add this commit ? 3da9487
It enables examples/display example for M5Stack Core board. It is then build in CI to gurantee error free compilation

@Ocean-lhy
Copy link
Contributor Author

@tore-espressif I really appreciate your guidance. I have solved the CI problem according to your instructions.

Regarding commit 3da9487, I have pulled it to make sure the CI is error-free.

bsp/m5stack_core/m5stack_core.c Outdated Show resolved Hide resolved
bsp/m5stack_core/include/bsp/m5stack_core.h Outdated Show resolved Hide resolved
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.

@Ocean-lhy Thank you for all changes! LGTM

@tore-espressif tore-espressif merged commit f770b17 into espressif:master Dec 12, 2024
18 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.

4 participants