diff --git a/cpu/esp_common/esp-now/esp_now_netdev.c b/cpu/esp_common/esp-now/esp_now_netdev.c index 5b48dbe34240e455added3e7680d1b177d4d5906..5e0e3b848e601cd6cb69bfab6d336903f4aa29b8 100644 --- a/cpu/esp_common/esp-now/esp_now_netdev.c +++ b/cpu/esp_common/esp-now/esp_now_netdev.c @@ -160,9 +160,6 @@ static void IRAM_ATTR esp_now_scan_peers_done(void) _esp_now_scan_peers_done = true; - /* set the time for next scan */ - xtimer_set(&_esp_now_scan_peers_timer, esp_now_params.scan_period); - mutex_unlock(&_esp_now_dev.dev_lock); } @@ -170,7 +167,10 @@ static void esp_now_scan_peers_start(void) { DEBUG("%s\n", __func__); + /* start the scan */ esp_wifi_scan_start(&scan_cfg, false); + /* set the time for next scan */ + xtimer_set(&_esp_now_scan_peers_timer, esp_now_params.scan_period); } static void IRAM_ATTR esp_now_scan_peers_timer_cb(void* arg) @@ -180,7 +180,7 @@ static void IRAM_ATTR esp_now_scan_peers_timer_cb(void* arg) esp_now_netdev_t* dev = (esp_now_netdev_t*)arg; if (dev->netdev.event_callback) { - dev->scan_event = true; + dev->scan_event++; dev->netdev.event_callback((netdev_t*)dev, NETDEV_EVENT_ISR); } } @@ -191,6 +191,8 @@ static const uint8_t _esp_now_mac[6] = { 0x82, 0x73, 0x79, 0x84, 0x79, 0x83 }; / #endif /* ESP_NOW_UNICAST */ +static bool _in_recv_cb = false; + static IRAM_ATTR void esp_now_recv_cb(const uint8_t *mac, const uint8_t *data, int len) { #if ESP_NOW_UNICAST @@ -200,40 +202,51 @@ static IRAM_ATTR void esp_now_recv_cb(const uint8_t *mac, const uint8_t *data, i } #endif - mutex_lock(&_esp_now_dev.rx_lock); - critical_enter(); + /* + * The function `esp_now_recv_cb` is executed in the context of the `wifi` + * thread. The ISRs handling the hardware interrupts from the WiFi + * interface pass events to a message queue of the `wifi` thread which is + * sequentially processed by the `wifi` thread to asynchronously execute + * callback functions such as `esp_now_recv_cb`. + * + * It should be therefore not possible to reenter function + * `esp_now_recv_cb`. To avoid inconsistencies this is checked by an + * additional boolean variable . This can not be realized by a mutex + * because `esp_now_recv_cb` would be reentered from same thread context. + */ + if (_in_recv_cb) { + return; + } + _in_recv_cb = true; /* - * The ring buffer uses a single byte for the pkt length, followed by the mac address, - * followed by the actual packet data. The MTU for ESP-NOW is 250 bytes, so len will never - * exceed the limits of a byte as the mac address length is not included. + * Since it is not possible to reenter the function `esp_now_recv_cb`, and + * the functions netif::_ recv and esp_now_netdev::_ recv are called + * directly in the same thread context, neither a mutual exclusion has to + * be realized nor have the interrupts to be deactivated. + * Therefore we can read directly from the `buf` and don't need a receive + * buffer. */ - if ((int)ringbuffer_get_free(&_esp_now_dev.rx_buf) < 1 + ESP_NOW_ADDR_LEN + len) { - critical_exit(); - mutex_unlock(&_esp_now_dev.rx_lock); + if (_esp_now_dev.rx_len) { DEBUG("%s: buffer full, dropping incoming packet of %d bytes\n", __func__, len); + _in_recv_cb = false; return; } -#if 0 /* don't printf anything in ISR */ - printf("%s\n", __func__); - printf("%s: received %d byte from %02x:%02x:%02x:%02x:%02x:%02x\n", - __func__, len, - mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); - od_hex_dump(data, len, OD_WIDTH_DEFAULT); -#endif - - ringbuffer_add_one(&_esp_now_dev.rx_buf, len); - ringbuffer_add(&_esp_now_dev.rx_buf, (char*)mac, ESP_NOW_ADDR_LEN); - ringbuffer_add(&_esp_now_dev.rx_buf, (char*)data, len); + _esp_now_dev.rx_mac = (uint8_t*)mac; + _esp_now_dev.rx_data = (uint8_t*)data; + _esp_now_dev.rx_len = len; + /* + * Since we are not in the interrupt context, we do not have to pass + * `NETDEV_EVENT_ISR` first. We can call the receive function directly. + */ if (_esp_now_dev.netdev.event_callback) { - _esp_now_dev.recv_event = true; - _esp_now_dev.netdev.event_callback((netdev_t*)&_esp_now_dev, NETDEV_EVENT_ISR); + _esp_now_dev.netdev.event_callback((netdev_t*)&_esp_now_dev, + NETDEV_EVENT_RX_COMPLETE); } - critical_exit(); - mutex_unlock(&_esp_now_dev.rx_lock); + _in_recv_cb = false; } static volatile int _esp_now_sending = 0; @@ -317,7 +330,8 @@ esp_now_netdev_t *netdev_esp_now_setup(void) extern portMUX_TYPE g_intr_lock_mux; mutex_init(&g_intr_lock_mux); - ringbuffer_init(&dev->rx_buf, (char*)dev->rx_mem, sizeof(dev->rx_mem)); + /* initialize buffer */ + dev->rx_len = 0; esp_system_event_add_handler(_esp_system_event_handler, NULL); @@ -416,11 +430,9 @@ esp_now_netdev_t *netdev_esp_now_setup(void) dev->netdev.driver = &_esp_now_driver; /* initialize netdev data structure */ - dev->recv_event = false; - dev->scan_event = false; + dev->scan_event = 0; mutex_init(&dev->dev_lock); - mutex_init(&dev->rx_lock); /* initialize ESP-NOW and register callback functions */ result = esp_now_init(); @@ -558,93 +570,51 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info) esp_now_netdev_t* dev = (esp_now_netdev_t*)netdev; - mutex_lock(&dev->rx_lock); - - uint16_t size = ringbuffer_empty(&dev->rx_buf) - ? 0 - : (ringbuffer_peek_one(&dev->rx_buf) + ESP_NOW_ADDR_LEN); + /* we store source mac address and received data in `buf` */ + uint16_t size = dev->rx_len ? ESP_NOW_ADDR_LEN + dev->rx_len : 0; - if (size && dev->rx_buf.avail < size) { - /* this should never happen unless this very function messes up */ - mutex_unlock(&dev->rx_lock); - return -EIO; - } - - if (!buf && !len) { - /* return the size without dropping received data */ - mutex_unlock(&dev->rx_lock); - return size; - } - - if (!buf && len) { - /* return the size and drop received data */ - if (size) { - ringbuffer_remove(&dev->rx_buf, 1 + size); + if (!buf) { + /* get the size of the frame */ + if (len > 0 && size) { + /* if len > 0, drop the frame */ + dev->rx_len = 0; } - mutex_unlock(&dev->rx_lock); return size; } - if (buf && len && !size) { - mutex_unlock(&dev->rx_lock); - return 0; + if (len < size) { + /* buffer is smaller than the number of received bytes */ + DEBUG("[esp_now] No space in receive buffers\n"); + /* newest API requires to drop the frame in that case */ + dev->rx_len = 0; + return -ENOBUFS; } - if (buf && len && size) { - if (size > len) { - DEBUG("[esp_now] No space in receive buffers\n"); - mutex_unlock(&dev->rx_lock); - return -ENOBUFS; - } - - /* remove already peeked size byte */ - ringbuffer_remove(&dev->rx_buf, 1); - ringbuffer_get(&dev->rx_buf, buf, size); - - uint8_t *mac = buf; + /* copy the buffer */ + memcpy(buf, dev->rx_mac, ESP_NOW_ADDR_LEN); + memcpy((char *)buf + ESP_NOW_ADDR_LEN, dev->rx_data, dev->rx_len); + dev->rx_len = 0; - DEBUG("%s: received %d byte from %02x:%02x:%02x:%02x:%02x:%02x\n", - __func__, size - ESP_NOW_ADDR_LEN, - mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); + uint8_t *mac = buf; + DEBUG("%s: received %d byte from %02x:%02x:%02x:%02x:%02x:%02x\n", + __func__, size - ESP_NOW_ADDR_LEN, + mac[0], mac[1], mac[2], mac[3], mac[4], mac[5]); #if defined(MODULE_OD) && ENABLE_DEBUG - od_hex_dump(buf + ESP_NOW_ADDR_LEN, size - ESP_NOW_ADDR_LEN, OD_WIDTH_DEFAULT); + od_hex_dump(buf + ESP_NOW_ADDR_LEN, size - ESP_NOW_ADDR_LEN, OD_WIDTH_DEFAULT); #endif #if ESP_NOW_UNICAST - if (esp_now_is_peer_exist(mac) <= 0) { - _esp_now_add_peer(mac, esp_now_params.channel, esp_now_params.key); - } + if (esp_now_is_peer_exist(mac) <= 0) { + _esp_now_add_peer(mac, esp_now_params.channel, esp_now_params.key); + } #endif #ifdef MODULE_NETSTATS_L2 - netdev->stats.rx_count++; - netdev->stats.rx_bytes += size; + netdev->stats.rx_count++; + netdev->stats.rx_bytes += size; #endif - mutex_unlock(&dev->rx_lock); - return size; - } - - mutex_unlock(&dev->rx_lock); - return -EINVAL; -} - -static inline int _get_iid(esp_now_netdev_t *dev, eui64_t *value, size_t max_len) -{ - CHECK_PARAM_RET(max_len >= sizeof(eui64_t), -EOVERFLOW); - - /* interface id according to */ - /* https://tools.ietf.org/html/rfc4291#section-2.5.1 */ - value->uint8[0] = dev->addr[0] ^ 0x02; /* invert bit1 */ - value->uint8[1] = dev->addr[1]; - value->uint8[2] = dev->addr[2]; - value->uint8[3] = 0xff; - value->uint8[4] = 0xfe; - value->uint8[5] = dev->addr[3]; - value->uint8[6] = dev->addr[4]; - value->uint8[7] = dev->addr[5]; - - return sizeof(eui64_t); + return size; } static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) @@ -692,10 +662,6 @@ static int _get(netdev_t *netdev, netopt_t opt, void *val, size_t max_len) res = sizeof(dev->addr); break; - case NETOPT_IPV6_IID: - res = _get_iid(dev, val, max_len); - break; - #ifdef MODULE_NETSTATS_L2 case NETOPT_STATS: CHECK_PARAM_RET(max_len == sizeof(uintptr_t), -EOVERFLOW); @@ -752,12 +718,11 @@ static void _isr(netdev_t *netdev) esp_now_netdev_t *dev = (esp_now_netdev_t*)netdev; - if (dev->recv_event) { - dev->recv_event = false; - dev->netdev.event_callback(netdev, NETDEV_EVENT_RX_COMPLETE); - } - else if (dev->scan_event) { - dev->scan_event = false; + critical_enter(); + + if (dev->scan_event) { + dev->scan_event--; + critical_exit(); esp_now_scan_peers_start(); } return; diff --git a/cpu/esp_common/esp-now/esp_now_netdev.h b/cpu/esp_common/esp-now/esp_now_netdev.h index 2e0a41884bc1a539d4b401cf6c5fd7e50359bb13..5f8a2838aef42a35d9e4a331704017a1cb0efe4f 100644 --- a/cpu/esp_common/esp-now/esp_now_netdev.h +++ b/cpu/esp_common/esp-now/esp_now_netdev.h @@ -21,7 +21,6 @@ #define ESP_NOW_NETDEV_H #include "net/netdev.h" -#include "ringbuffer.h" #ifdef __cplusplus extern "C" { @@ -50,11 +49,10 @@ extern "C" { /** * @brief buffer size used for RX buffering * - * Reduce this value if your expected traffic does not include full IPv6 MTU - * sized packets. + * The buffer is use to store a ESP-NOW packet and the source MAC address. */ #ifndef ESP_NOW_BUFSIZE -#define ESP_NOW_BUFSIZE (1500) +#define ESP_NOW_BUFSIZE (ESP_NOW_MAX_SIZE_RAW + ESP_NOW_ADDR_LEN) #endif /** @@ -84,8 +82,9 @@ typedef struct uint8_t addr[ESP_NOW_ADDR_LEN]; /**< device addr (MAC address) */ - uint8_t rx_mem[ESP_NOW_BUFSIZE]; /**< memory holding incoming packages */ - ringbuffer_t rx_buf; /**< ringbuffer for incoming packages */ + uint8_t rx_len; /**< number of bytes received */ + uint8_t* rx_mac; /**< source mac of received data */ + uint8_t* rx_data; /**< received */ uint8_t tx_mem[ESP_NOW_MAX_SIZE_RAW]; /**< memory holding outgoing package */ @@ -94,10 +93,8 @@ typedef struct #endif mutex_t dev_lock; /**< device is already in use */ - mutex_t rx_lock; /**< rx_buf handling in progress */ - bool recv_event; /**< ESP-NOW frame received */ - bool scan_event; /**< ESP-NOW peers have to be scannged */ + uint8_t scan_event; /**< ESP-NOW peers have to be scannged */ } esp_now_netdev_t;