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

Wiznetif: report module power state. #2258

Merged
merged 3 commits into from
Dec 23, 2020
Merged

Conversation

XuGuohui
Copy link
Member

Problem

The device may timeout waiting the ethernet interface to be off() when sleep is requested and the ethernet interface is present.

Solution

Report the Wiznetif power state to system network manager.

Note that the W5500 module is actually on still even if we've called the corresponding API to turn it off. Because there is no hardware pins to control the power to the W5500 module, nor we implement the low power mode for W5500.

Example App

#include "application.h"

STARTUP(System.enableFeature(FEATURE_ETHERNET_DETECTION));

SerialLogHandler l(LOG_LEVEL_ALL);

SYSTEM_MODE(MANUAL);

void setup() {
    while (!Serial.isConnected());
    Log.info("Application started.");

    System.sleep(SystemSleepConfiguration().mode(SystemSleepMode::ULTRA_LOW_POWER).duration(5s));
}

void loop() {
}

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@XuGuohui XuGuohui added this to the 3.0.0 milestone Dec 22, 2020
@XuGuohui XuGuohui modified the milestones: 3.0.0, 2.0.2 Dec 22, 2020
return ret;
}

void WizNetif::notifyPowerState() {
Copy link
Member

Choose a reason for hiding this comment

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

Let's add an argument here and task it with updating internal pwrState_ as well. There shouldn't be any reason to do that externally and then still call into this method.

}

void WizNetif::notifyPowerState(if_power_state_t state) {
pwrState_ = state;
Copy link
Member

Choose a reason for hiding this comment

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

This could probably be:

if (pwrState_ == state) {
    return;
}
pwrState_ = state;
// Notify

So that you don't have to check externally.

Copy link
Member Author

Choose a reason for hiding this comment

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

That might miss a power state change event. Image that the pwrState_ is changed to UP on initialization, then powerUp() won't notify the power state.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, let's leave it as-is then 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not a good example. If the pwrState_ is changed to IF_POWER_STATE_UP somewhere but without notifying the state, somewhere else call notifyPowerState(IF_POWER_STATE_UP) won't notify the power state. Just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

While this remind me that we should always notify the power state if possible, especially in powerDown() that may block the waitInterfaceOff(). I'll revise it a little bit.

@XuGuohui XuGuohui merged commit 8a83aeb into develop Dec 23, 2020
@XuGuohui XuGuohui deleted the feature/wiznetif/ch70409 branch December 23, 2020 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants