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

[icd] Removed ICD related methods (former SED) from NoThreadImpl #145

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

Conversation

kkasperczyk-no
Copy link
Owner

There are several methods related to ICD (former SED) used for setting expected communication intervals. Currently they are implemented only for Thread devices and for Wi-Fi the implementation returns unsupported feature through NoThreadImpl. Including this sleep intervals could be useful for some low power Wi-Fi modes too.

Changes:

  • Removed methods implementation from NoThread impl. Now after enabling CHIP_DEVICE_CONFIG_ENABLE_SED (I believe it should be renamed to ICD in the future), the implementation will have to be provided by the proper platform.
  • Implemented methods for nrfconnect Wi-Fi platform.

#if CHIP_DEVICE_CONFIG_ENABLE_SED
CHIP_ERROR ConnectivityManagerImplWiFi::_GetSEDIntervalsConfig(ConnectivityManager::SEDIntervalsConfig & SEDIntervalsConfig)
{
// Wi-Fi uses legacy power save mode that has fixed inactivity interval
Copy link
Collaborator

Choose a reason for hiding this comment

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

The interval is actually AP-controlled. We use constants just because we don't have an API for that yet.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Right, I mean, that the intervals for our Wi-Fi platform are fixed and cannot be changed dynamically, like for Thread.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be possible with the anticipated Wi-Fi net_mgmt API extensions. Then we will implement the method for getting these values in the WiFiManager.

Copy link
Owner Author

@kkasperczyk-no kkasperczyk-no Jan 16, 2023

Choose a reason for hiding this comment

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

Ok, I added "for now" to the comment then :)

#if CHIP_DEVICE_CONFIG_ENABLE_SED
CHIP_ERROR ConnectivityManagerImplWiFi::_GetSEDIntervalsConfig(ConnectivityManager::SEDIntervalsConfig & SEDIntervalsConfig)
{
// Wi-Fi uses legacy power save mode that has fixed inactivity interval
Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be possible with the anticipated Wi-Fi net_mgmt API extensions. Then we will implement the method for getting these values in the WiFiManager.

@kkasperczyk-no kkasperczyk-no force-pushed the sed_intervals_wifi branch 3 times, most recently from 2044552 to d4c1add Compare January 16, 2023 11:49
There are several methods related to ICD (former SED) used for
setting expected communication intervals. Currently they are
implemented only for Thread devices and for Wi-Fi the
implementation returns unsupported feature through NoThreadImpl.
Including this sleep intervals could be useful for some low power
Wi-Fi modes too.

Changes:
* Removed methods implementation from NoThread impl. Now after
enabling CHIP_DEVICE_CONFIG_ENABLE_SED (I believe it should be
renamed to ICD in the future), the implementation will have to
be provided by the proper platform.
* Implemented methods for nrfconnect Wi-Fi platform.
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