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

Support ps-max-modem #273

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Support ps-max-modem #273

merged 1 commit into from
Sep 29, 2023

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Sep 29, 2023

We have support for ps-min-modem power-safe mode for quite a long time - this also adds support for ps-max-modem

Like ps-min-modem it's activated by a feature.

No power-safe
no-ps

ps-min-modem
ps-min-modem

ps-max-modem
max-modem

Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Very nice! LGTM!

@bjoernQ bjoernQ merged commit 0d502eb into main Sep 29, 2023
14 checks passed
@bjoernQ bjoernQ deleted the feature/ps-max-modem branch September 29, 2023 13:08
@bjoernQ bjoernQ mentioned this pull request Sep 29, 2023
@@ -153,6 +153,10 @@ struct Config {
heap_size: usize,
#[default(DEFAULT_TICK_RATE_HZ)]
tick_rate_hz: u32,
#[default(3)]
listen_interval: u16,
#[default(6)]
Copy link
Contributor

Choose a reason for hiding this comment

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

@bjoernQ If this default is used, starting AP panics. Quoting from https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/network/esp_wifi.html#_CPPv426esp_wifi_set_inactive_time16wifi_interface_t8uint16_t

ESP_ERR_INVALID_ARG: invalid argument, For Station, if sec is less than 3. For SoftAP, if sec is less than 10.

I believe a different config should be provided for AP and STA modes.

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Oct 11, 2023

I already found the problem - will create a PR tomorrow

@bugadani
Copy link
Contributor

Ah okay I was about to edit my comment saying that AP panics regardless of the beacon interval, but I guess that's not needed now :)

@bjoernQ
Copy link
Contributor Author

bjoernQ commented Oct 12, 2023

#281 should be a fix

Somewhat embarrassing since I knew before it's not fine to set it for AP mode 🤦‍♂️

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