Skip to content

Make ESP8266 compatible with bare metal profile #12022

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.

Sign up for GitHub

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
Dec 11, 2019
Merged

Make ESP8266 compatible with bare metal profile #12022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 6 additions & 10 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_PRESENT)
#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_API_PRESENT)
#ifndef __STDC_FORMAT_MACROS
#define __STDC_FORMAT_MACROS
#endif
Expand Down Expand Up @@ -57,7 +57,6 @@ ESP8266::ESP8266(PinName tx, PinName rx, bool debug, PinName rts, PinName cts)
_closed(false),
_error(false),
_busy(false),
_reset_check(_rmutex),
_reset_done(false),
_conn_status(NSAPI_STATUS_DISCONNECTED)
{
Expand Down Expand Up @@ -272,14 +271,15 @@ bool ESP8266::reset(void)
continue;
}

_rmutex.lock();
Copy link
Contributor

@VeijoPesonen VeijoPesonen Dec 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Converting this as PlatformMutex would be the right solution. Scratch that. RTOS Mutex is rendered out in brametal builds. This code should be kept.

while ((rtos::Kernel::get_ms_count() - start_time < ESP8266_BOOTTIME) && !_reset_done) {
while (!_reset_done) {
_process_oob(ESP8266_RECV_TIMEOUT, true); // UART mutex claimed -> need to check for OOBs ourselves
_reset_check.wait_for(100); // Arbitrary relatively short delay
if (_reset_done || (rtos::Kernel::get_ms_count() - start_time >= ESP8266_BOOTTIME)) {
break;
}
rtos::ThisThread::sleep_for(100);
}

done = _reset_done;
_rmutex.unlock();
if (done) {
break;
}
Expand Down Expand Up @@ -1040,11 +1040,7 @@ void ESP8266::_oob_watchdog_reset()

void ESP8266::_oob_ready()
{

_rmutex.lock();
_reset_done = true;
_reset_check.notify_all();
_rmutex.unlock();

for (int i = 0; i < SOCKET_COUNT; i++) {
_sock_i[i].open = false;
Expand Down
6 changes: 2 additions & 4 deletions components/wifi/esp8266-driver/ESP8266/ESP8266.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#ifndef ESP8266_H
#define ESP8266_H

#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_PRESENT)
#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_API_PRESENT)
#include <stdint.h>

#include "drivers/UARTSerial.h"
Expand All @@ -27,8 +27,8 @@
#include "platform/ATCmdParser.h"
#include "platform/Callback.h"
#include "platform/mbed_error.h"
#include "rtos/ConditionVariable.h"
#include "rtos/Mutex.h"
#include "rtos/ThisThread.h"

// Various timeouts for different ESP8266 operations
#ifndef ESP8266_CONNECT_TIMEOUT
Expand Down Expand Up @@ -428,7 +428,6 @@ class ESP8266 {
PinName _serial_rts;
PinName _serial_cts;
rtos::Mutex _smutex; // Protect serial port access
rtos::Mutex _rmutex; // Reset protection
Copy link
Contributor

@michalpasztamobica michalpasztamobica Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Why is _rmutex removed while _smutex can stay? I thought that "bare-metal" means "no RTOS", means no thread synchronisation mechanisms?

Copy link
Contributor

Choose a reason for hiding this comment

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

I helped prepare this.

_rmutex wasn't doing anything, as far as I can see. It was a remnant of the also-redundant condition variable bits for reset, which seemed to be imagining some other thread running OOB handlers.

The piece of code waiting for reset is inside _smutex, so it all happens synchronously, blocking inside that code.


// AT Command Parser
mbed::ATCmdParser _parser;
Expand Down Expand Up @@ -479,7 +478,6 @@ class ESP8266 {
bool _closed;
bool _error;
bool _busy;
rtos::ConditionVariable _reset_check;
bool _reset_done;

// Modem's address info
Expand Down
14 changes: 13 additions & 1 deletion components/wifi/esp8266-driver/ESP8266Interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_PRESENT)
#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_API_PRESENT)

#include <string.h>
#include <stdint.h>
Expand Down Expand Up @@ -64,7 +64,9 @@ ESP8266Interface::ESP8266Interface()
_pwr_pin(MBED_CONF_ESP8266_PWR),
_ap_sec(NSAPI_SECURITY_UNKNOWN),
_if_blocking(true),
#if MBED_CONF_RTOS_PRESENT
_if_connected(_cmutex),
#endif
_initialized(false),
_connect_retval(NSAPI_ERROR_OK),
_disconnect_retval(NSAPI_ERROR_OK),
Expand Down Expand Up @@ -104,7 +106,9 @@ ESP8266Interface::ESP8266Interface(PinName tx, PinName rx, bool debug, PinName r
_pwr_pin(pwr),
_ap_sec(NSAPI_SECURITY_UNKNOWN),
_if_blocking(true),
#if MBED_CONF_RTOS_PRESENT
_if_connected(_cmutex),
#endif
_initialized(false),
_connect_retval(NSAPI_ERROR_OK),
_disconnect_retval(NSAPI_ERROR_OK),
Expand Down Expand Up @@ -267,7 +271,9 @@ void ESP8266Interface::_connect_async()
_esp.uart_enable_input(false);
_software_conn_stat = IFACE_STATUS_DISCONNECTED;
}
#if MBED_CONF_RTOS_PRESENT
_if_connected.notify_all();
#endif
} else {
// Postpone to give other stuff time to run
_connect_event_id = _global_event_queue->call_in(ESP8266_INTERFACE_CONNECT_INTERVAL_MS,
Expand Down Expand Up @@ -329,10 +335,12 @@ int ESP8266Interface::connect()
"connect(): unable to add event to queue. Increase \"events.shared-eventsize\"\n");
}

#if MBED_CONF_RTOS_PRESENT
while (_if_blocking && (_conn_status_to_error() != NSAPI_ERROR_IS_CONNECTED)
&& (_connect_retval == NSAPI_ERROR_NO_CONNECTION)) {
_if_connected.wait();
}
#endif

_cmutex.unlock();

Expand Down Expand Up @@ -418,7 +426,9 @@ void ESP8266Interface::_disconnect_async()

_power_off();
_software_conn_stat = IFACE_STATUS_DISCONNECTED;
#if MBED_CONF_RTOS_PRESENT
_if_connected.notify_all();
#endif

} else {
// Postpone to give other stuff time to run
Expand Down Expand Up @@ -479,11 +489,13 @@ int ESP8266Interface::disconnect()
"disconnect(): unable to add event to queue. Increase \"events.shared-eventsize\"\n");
}

#if MBED_CONF_RTOS_PRESENT
while (_if_blocking
&& (_conn_status_to_error() != NSAPI_ERROR_NO_CONNECTION)
&& (_disconnect_retval != NSAPI_ERROR_OK)) {
_if_connected.wait();
}
#endif

_cmutex.unlock();
if (!_if_blocking) {
Expand Down
6 changes: 5 additions & 1 deletion components/wifi/esp8266-driver/ESP8266Interface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
#ifndef ESP8266_INTERFACE_H
#define ESP8266_INTERFACE_H

#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_PRESENT)
#if DEVICE_SERIAL && DEVICE_INTERRUPTIN && defined(MBED_CONF_EVENTS_PRESENT) && defined(MBED_CONF_NSAPI_PRESENT) && defined(MBED_CONF_RTOS_API_PRESENT)
#include "drivers/DigitalOut.h"
#include "drivers/Timer.h"
#include "ESP8266/ESP8266.h"
Expand All @@ -30,7 +30,9 @@
#include "features/netsocket/WiFiAccessPoint.h"
#include "features/netsocket/WiFiInterface.h"
#include "platform/Callback.h"
#if MBED_CONF_RTOS_PRESENT
Copy link

Choose a reason for hiding this comment

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

Why is this needed? same MACRO guardian is already in that header file. And if the header file does not compile as such (includes rtos headers outside MACRO) those headers should be moved inside MBED_CONF_RTOS_PRESENT macro.

#include "rtos/ConditionVariable.h"
#endif
#include "rtos/Mutex.h"

#define ESP8266_SOCKET_COUNT 5
Expand Down Expand Up @@ -452,7 +454,9 @@ class ESP8266Interface : public NetworkStack, public WiFiInterface {
struct _channel_info _ch_info;

bool _if_blocking; // NetworkInterface, blocking or not
#if MBED_CONF_RTOS_PRESENT
rtos::ConditionVariable _if_connected;
#endif

// connect status reporting
nsapi_error_t _conn_status_to_error();
Expand Down