Skip to content

ESP8266 unlocks deep sleep when disconnected #11514

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 1 commit into from
Sep 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1275,4 +1275,9 @@ bool ESP8266::set_country_code_policy(bool track_ap, const char *country_code, i
return done;
}

int ESP8266::uart_enable_input(bool enabled)
{
return _serial.enable_input(enabled);
}

#endif
8 changes: 8 additions & 0 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.h
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,14 @@ class ESP8266 {
static const int8_t WIFIMODE_STATION_SOFTAP = 3;
static const int8_t SOCKET_COUNT = 5;

/**
* Enables or disables uart input and deep sleep
*
* @param lock if TRUE, uart input is enabled and deep sleep is locked
* if FALSE, uart input is disabled and deep sleep is unlocked
*/
int uart_enable_input(bool lock);
Copy link
Member

Choose a reason for hiding this comment

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

Public function should have doxygen and also be bundled with rest of the methods (after flush).

Copy link
Contributor

Choose a reason for hiding this comment

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

Should be fixed now (@dmaziec1 worth adding a comment for reviewers, what has been addressed)


private:
// FW version
struct fw_sdk_version _sdk_v;
Expand Down
58 changes: 51 additions & 7 deletions components/wifi/esp8266-driver/ESP8266Interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ ESP8266Interface::ESP8266Interface()
_sock_i[i].open = false;
_sock_i[i].sport = 0;
}
_esp.uart_enable_input(false);
}
#endif

Expand Down Expand Up @@ -125,6 +126,7 @@ ESP8266Interface::ESP8266Interface(PinName tx, PinName rx, bool debug, PinName r
_sock_i[i].open = false;
_sock_i[i].sport = 0;
}
_esp.uart_enable_input(false);
}

ESP8266Interface::~ESP8266Interface()
Expand Down Expand Up @@ -224,13 +226,15 @@ void ESP8266Interface::_connect_async()
nsapi_error_t status = _init();
if (status != NSAPI_ERROR_OK) {
_connect_retval = status;
_esp.uart_enable_input(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a feeling this should be done already inside _init() once it's detected that something has gone wrong with initialization. Same goes for enabling the serial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uart input enabling/disabling depends on _software_conn_stat value, so it would be better to enable or disable input once when it changes. _init should rather rely on the driver to have uart input enabled (depending on the conn_stat), than try and influence it on its own.
Also, in this way we will avoid repetition of the same condition statement before each return.
@VeijoPesonen, does this make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Uart input enabling/disabling depends on _software_conn_stat value, so it would be better to enable or disable input once when it changes.

Ok, point taken :)

Also, in this way we will avoid repetition of the same condition statement before each return.

Not needed based on your response.

_software_conn_stat = IFACE_STATUS_DISCONNECTED;
//_conn_stat_cb will be called from refresh_conn_state_cb
return;
}

if (!_esp.dhcp(true, 1)) {
_connect_retval = NSAPI_ERROR_DHCP_FAILURE;
_esp.uart_enable_input(false);
_software_conn_stat = IFACE_STATUS_DISCONNECTED;
//_conn_stat_cb will be called from refresh_conn_state_cb
return;
Expand All @@ -253,6 +257,7 @@ void ESP8266Interface::_connect_async()
_connect_retval = NSAPI_ERROR_CONNECTION_TIMEOUT;
}
if (_connect_retval != NSAPI_ERROR_OK) {
_esp.uart_enable_input(false);
_software_conn_stat = IFACE_STATUS_DISCONNECTED;
}
_if_connected.notify_all();
Expand Down Expand Up @@ -304,6 +309,7 @@ int ESP8266Interface::connect()
_cmutex.lock();
}
_software_conn_stat = IFACE_STATUS_CONNECTING;
_esp.uart_enable_input(true);
_connect_retval = NSAPI_ERROR_NO_CONNECTION;
MBED_ASSERT(!_connect_event_id);
_conn_timer.stop();
Expand Down Expand Up @@ -404,6 +410,7 @@ void ESP8266Interface::_disconnect_async()
}

_power_off();
_software_conn_stat = IFACE_STATUS_DISCONNECTED;
_if_connected.notify_all();

} else {
Expand All @@ -418,8 +425,8 @@ void ESP8266Interface::_disconnect_async()
}
}
_cmutex.unlock();
_software_conn_stat = IFACE_STATUS_DISCONNECTED;

_esp.uart_enable_input(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be done already inside _power_off().

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this can stay as it is based on your previous comment about not turning the serial on inside _init().

if (_disconnect_event_id == 0) {
if (_conn_stat_cb) {
_conn_stat_cb(NSAPI_EVENT_CONNECTION_STATUS_CHANGE, _conn_stat);
Expand Down Expand Up @@ -481,17 +488,31 @@ int ESP8266Interface::disconnect()

const char *ESP8266Interface::get_ip_address()
{
if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(true);
}

const char *ip_buff = _esp.ip_addr();
if (!ip_buff || strcmp(ip_buff, "0.0.0.0") == 0) {
return NULL;
ip_buff = NULL;
}
if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(false);
}

return ip_buff;
}

const char *ESP8266Interface::get_mac_address()
{
return _esp.mac_addr();
if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(true);
}
const char *ret = _esp.mac_addr();

if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(false);
}
return ret;
}

const char *ESP8266Interface::get_gateway()
Expand All @@ -506,7 +527,17 @@ const char *ESP8266Interface::get_netmask()

int8_t ESP8266Interface::get_rssi()
{
return _esp.rssi();
if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(true);
}

int8_t ret = _esp.rssi();

if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(false);
}

return ret;
}

int ESP8266Interface::scan(WiFiAccessPoint *res, unsigned count)
Expand All @@ -523,13 +554,25 @@ int ESP8266Interface::scan(WiFiAccessPoint *res, unsigned count, scan_mode mode,
return NSAPI_ERROR_PARAMETER;
}

if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(true);
}

nsapi_error_t status = _init();
if (status != NSAPI_ERROR_OK) {
return status;
if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(false);
}
}

return _esp.scan(res, count, (mode == SCANMODE_ACTIVE ? ESP8266::SCANMODE_ACTIVE : ESP8266::SCANMODE_PASSIVE),
t_max, t_min);
int ret = _esp.scan(res, count, (mode == SCANMODE_ACTIVE ? ESP8266::SCANMODE_ACTIVE : ESP8266::SCANMODE_PASSIVE),
t_max, t_min);

if (_software_conn_stat == IFACE_STATUS_DISCONNECTED) {
_esp.uart_enable_input(false);
}
return ret;
}

bool ESP8266Interface::_get_firmware_ok()
Expand All @@ -555,6 +598,7 @@ nsapi_error_t ESP8266Interface::_init(void)
if (!_initialized) {
_pwr_pin.power_off();
_pwr_pin.power_on();

if (_reset() != NSAPI_ERROR_OK) {
return NSAPI_ERROR_DEVICE_ERROR;
}
Expand Down