Skip to content

Update WiFiGeneric.cpp #7044

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

Merged
merged 3 commits into from
Jul 28, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions libraries/WiFi/src/WiFiGeneric.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,15 +664,6 @@ bool wifiLowLevelInit(bool persistent){

wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT();

if(!WiFiGenericClass::useStaticBuffers()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this was added to reduce the memory footprint of the WiFi driver as it increased considerably from IDF v3 to IDF v4.

Refer to #5791 for this. @SuGlider fyi...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes... Default now is WiFi Dynamic Allocation in order to reduce HEAP consumption.
If the user wants to go with Static Allocation, the application shall use WiFiGenericClass::useStaticBuffers(true); before setting the WiFi Mode with WiFiGenericClass::mode()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SuGlider Although the customers can call WiFiGenericClass::useStaticBuffers(true) to solve the problem, but I think we also need to change the value cfg.cache_tx_buf_num = 1 to at least 4, this will ensure the customers can send several packets at the same time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, Agree. Please commit this change. This will cost 5K RAM in the heap. It is worth it.

cfg.static_tx_buf_num = 0;
cfg.dynamic_tx_buf_num = 32;
cfg.tx_buf_type = 1;
cfg.cache_tx_buf_num = 1; // can't be zero!
cfg.static_rx_buf_num = 4;
cfg.dynamic_rx_buf_num = 32;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I reject this PR and changes.... as explained in
espressif/esp-idf#8992 (comment)
espressif/esp-idf#8992 (comment)

esp_err_t err = esp_wifi_init(&cfg);
if(err){
log_e("esp_wifi_init %d", err);
Expand Down