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(wifi_remote): Make wifi_remote depend on esp_hosted (IDFGH-13027) #596

Merged

Conversation

SohKamYung-Espressif
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jun 14, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot changed the title feat(wifi_remote): Make wifi_remote depend on esp_hosted feat(wifi_remote): Make wifi_remote depend on esp_hosted (IDFGH-13027) Jun 14, 2024
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

Could you please just squash the commits, and update the dependency condition to the latest esp_hosted version (I see that's already at 0.0.6).
LGTM otherwise, thanks for the updates!

@SohKamYung-Espressif SohKamYung-Espressif force-pushed the feat/depend_on_esp_hosted branch from bec400d to 1e558bb Compare June 18, 2024 02:46
@SohKamYung-Espressif SohKamYung-Espressif force-pushed the feat/depend_on_esp_hosted branch from f4e1525 to ac9972a Compare June 18, 2024 03:14
@david-cermak
Copy link
Collaborator

@SohKamYung-Espressif Thanks for the contribution, pipeline has passed and I'm about to merge it. but before doing so, I'd like to ask you to just check it locally that it still works (removing the project's dependency on esp_hosted and rely only on the transient one from esp_wifi_remote).
My only concern is about the __weak attribute of the WiFi definitions in wifi_remote, since it also depends on linking order of static libs and this PR alters it.

@SohKamYung-Espressif
Copy link
Contributor Author

@david-cermak Sure. Which project(s) should I test this change on (locally adding the dependency on esp_hosted into esp_wifi_remote component AND removing the dependency on esp_hosted in the project's idf_component.yml)?

I have tested this with my S3 <---> C6 SDIO test board (native wifi on S3 disabled) with the iperf example project and it builds and runs as expected.

@david-cermak
Copy link
Collaborator

Thanks for testing!

@david-cermak david-cermak merged commit 2b64e80 into espressif:master Jun 18, 2024
32 checks passed
@SohKamYung-Espressif SohKamYung-Espressif deleted the feat/depend_on_esp_hosted branch June 18, 2024 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants