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(config): Add support for disabling WiFi remote impl #3

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

david-cermak
Copy link
Collaborator

@david-cermak david-cermak commented Oct 7, 2024

Added ESP_WIFI_REMOTE_DISABLE to enable coexistence with esp-extconn within one project

  • Made ESP_WIFI_REMOTE mutually exclusive with ESP_HOST_WIFI_ENABLED
  • WiFi remote menuconfig is entirely disabled if CONFIG_ESP_HOST_WIFI_ENABLED=y

@david-cermak david-cermak self-assigned this Oct 7, 2024
@mantriyogesh
Copy link
Collaborator

@david-cermak LGTM..

@SohKamYung-Espressif
Copy link
Collaborator

I added this Kconfig.projbuild to examples/wifi/iperf/main to test it:

menu "Example Configuration"
    depends on !SOC_WIFI_SUPPORTED

    choice EXAMPLE_IPERF_OFFLOAD
        prompt "External Wireless Connnectivity Component to use"
        default EXAMPLE_IPERF_OFFLOAD_VIA_WIFI_REMOTE
        help
            External component to use to provide external wireless connectivity (Wi-Fi & Bluetooth)

        config EXAMPLE_IPERF_OFFLOAD_VIA_WIFI_REMOTE
            bool "Wifi-Remote"

        # Kconfig for extconn enables it only if SOC_WIRELESS_HOST_SUPPORTED
        config EXAMPLE_IPERF_OFFLOAD_VIA_EXT_CONN
            depends on SOC_WIRELESS_HOST_SUPPORTED
            bool "Ext-Conn"
            select ESP_WIFI_REMOTE_DISABLE

    endchoice

endmenu

Observations:

  1. Once CONFIG_ESP_WIFI_REMOTE_DISABLE=y appears in the sdkconfig, it does not seem possible to remove it by running idf.py menuconfig again (to switch back to wifi-remote). I have to do a rm sdkconfig first.
  2. The iperf example for p4 does not build correctly. I am getting this build error:
/home/kysoh/esp/gitlab_esp-idf/examples/wifi/iperf/main/iperf_example_main.c: In function 'app_main':
/home/kysoh/esp/gitlab_esp-idf/examples/wifi/iperf/main/iperf_example_main.c:88:5: error: unknown type name 'app_wifi_initialise_config_t'; did you mean 'wifi_init_config_t'?
   88 |     app_wifi_initialise_config_t config = APP_WIFI_CONFIG_DEFAULT();
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     wifi_init_config_t
/home/kysoh/esp/gitlab_esp-idf/examples/wifi/iperf/main/iperf_example_main.c:88:43: error: implicit declaration of function 'APP_WIFI_CONFIG_DEFAULT'; did you mean 'WIFI_INIT_CONFIG_DEFAULT'? [-Wimplicit-function-declaration]

Reason is because wifi_cmd.h in the wifi-cmd component has this dependency to enable the header:

#if CONFIG_ESP_WIFI_ENABLED || CONFIG_ESP_WIFI_REMOTE_ENABLED || CONFIG_ESP_HOST_WIFI_ENABLED

All conditions are not met, if wifi-remote is disabled.

Actually, I think the iperf example on master would not build correctly for P4 because of this.

@mantriyogesh
Copy link
Collaborator

Once CONFIG_ESP_WIFI_REMOTE_DISABLE=y appears in the sdkconfig, it does not seem possible to remove it by running idf.py menuconfig again (to switch back to wifi-remote). I have to do a rm sdkconfig first.

This should have some workaround as we discussed. This is issue of sdkconfig itself.

#if CONFIG_ESP_WIFI_ENABLED || CONFIG_ESP_WIFI_REMOTE_ENABLED || CONFIG_ESP_HOST_WIFI_ENABLED

We might want to add EXT_CONN specific symbol in this list. So that the build goes through for non-wifi-remote. Once confirmed, we can propose the change to concerned repo maintainer..

@mantriyogesh
Copy link
Collaborator

This should have some workaround as we discussed. This is issue of sdkconfig itself.

Workaround is also not working fine, it is only till the time we do not exit from sdkconfig.
if we 'select WIFI-REMOTE-DISABLED' and exit from menuconfig, but re-configure again using idf.py menuconfig and 'enable' again, it shows in menuconfig gui, it enabled again. But in sdkconfig.h or sdkconfig it is still 'WIFI-REMOTE-DISABLED' and not reverted. So the problem is still there

@SohKamYung-Espressif
Copy link
Collaborator

We might want to add EXT_CONN specific symbol in this list. So that the build goes through for non-wifi-remote. Once confirmed, we can propose the change to concerned repo maintainer..

Yes, if I change that check to

#if CONFIG_ESP_WIFI_ENABLED || CONFIG_ESP_WIFI_REMOTE_ENABLED || CONFIG_ESP_HOST_WIFI_ENABLED || CONFIG_ESP_EXT_CONN_ENABLE

Iperf compiles with ext-conn.

@david-cermak david-cermak force-pushed the feat/disable_wifi_remote branch from ae78f91 to 9fe73eb Compare October 9, 2024 05:17
@david-cermak david-cermak force-pushed the feat/disable_wifi_remote branch from 9fe73eb to 3c5d58f Compare October 9, 2024 15:08
@david-cermak
Copy link
Collaborator Author

Updated per offline discussion with @SohKamYung-Espressif to disable WiFi-remote whenever ESP_HOST_WIFI_ENABLED is ON. (also added a "read-only" item indicating that it's disabled and describing why)

david-cermak added a commit that referenced this pull request Oct 9, 2024
fix(ci): remove unwanted deps
0.4.1
Bug Fixes
- Remove unused Kconfig from scripts (3c5d58f)
- Disable WiFi remote impl if ESP_HOST_WIFI_ENABLED=y (1f6fe16)
@SohKamYung-Espressif
Copy link
Collaborator

LGTM. Tested with building iperf for P4.

By default, idf.py build builds and links in wifi-remote and ESP-Hosted.

Running idf.py -DSDKCONFIG_DEFAULTS="sdkconfig.ci.esp32p4_with_extconn" build links in ext-conn as expected, so CI with ext-conn should not be affected.

@mantriyogesh
Copy link
Collaborator

This change would make life extremely easy @david-cermak , thanks a lot.
As it is tested now, with multiple alternatives locally, can we push this change?

Dependent IDF MR would then come to conclusion then.

@david-cermak david-cermak merged commit ea7f100 into espressif:main Oct 10, 2024
13 checks passed
@david-cermak
Copy link
Collaborator Author

As it is tested now, with multiple alternatives locally, can we push this change?

Published as v0.4.1

Thanks for your inputs and diligent testing!

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