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(esp32_p4_function_ev_board): add example of wifi connection #407

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

igrr
Copy link
Member

@igrr igrr commented Oct 11, 2024

This PR adds a simple example of connecting to Wi-Fi on ESP32-P4 EV board.

This example adds esp_wifi_remote as a dependency and sets the required sdkconfig options.

The goal is to allow users to quickly get started with Wi-Fi connectivity on this board by running

idf.py create-project-from-example espressif/esp32_p4_function_ev_board:wifi_connection

@igrr
Copy link
Member Author

igrr commented Oct 11, 2024

@tore-espressif WDYT about adding such an example to bsp/esp32_p4_function_ev_board/examples directory?

Base automatically changed from bugfix/p4_ev_board_sd_slot to master October 11, 2024 13:04
@tore-espressif
Copy link
Collaborator

If ESP32-P4 + ESP32-C6 hardware setup is planned for all our P4 devkits, I (as a user) would expect this to be in esp-idf examples.

If the ESP32-C6 modem extension is not going to be typical usecase, we can leave the example here

@igrr
Copy link
Member Author

igrr commented Oct 11, 2024

I think we will keep ESP32-C6 on ESP32-P4 EV board, but we can't be sure that makers of other P4 boards will also add ESP32-C6. If they do, still they might opt to use a different set of pins for SDIO.

On IDF side, so far support for P4 is being added only in iperf example. Still, it will take some time to merge this and this will only be available in IDF master branch for some time.

So I thought that making this example easily available via the component manager would be a reasonable solution in the meantime.

@igrr
Copy link
Member Author

igrr commented Oct 11, 2024

@tore-espressif Might need to ask for your help with the pre-commit hook. I'm guessing that the script is considering example README.md file as a BSP readme file... Could you suggest what I can change to make this check succeed?

@tore-espressif
Copy link
Collaborator

@igrr Please ignore the pre-commit fail, probably some issue on our side

@tore-espressif
Copy link
Collaborator

Might need to ask for your help with the pre-commit hook. I'm guessing that the script is considering example README.md file as a BSP readme file... Could you suggest what I can change to make this check succeed?

We can force merge, if you need it now. I'll take a look on Monday

@igrr
Copy link
Member Author

igrr commented Oct 11, 2024

No, this one is not urgent at all, let's find the proper solution next week. (I have looked at the script for a bit and I think I see how to make it work, just don't have time to do it right now.)

version: "*"
override_path: "../../../"
espressif/esp_wifi_remote:
version: "*"
Copy link
Member Author

Choose a reason for hiding this comment

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

Probably makes sense to pin at least some version of esp_wifi_remote (and maybe esp_hosted?) here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, do you plan update and set some major version here?

@tore-espressif
Copy link
Collaborator

@igrr I rebased your PR, the pre-commit check is passing now

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.

@igrr Thank you for this example, good work! LGTM

version: "*"
override_path: "../../../"
espressif/esp_wifi_remote:
version: "*"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree, do you plan update and set some major version here?

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