espressif / esp-now

A connectionless Wi-Fi communication protocol
Apache License 2.0
527 stars 93 forks source link

Bug with ESP-NOW init config struct (AEGHB-648) #119

Closed ChenYuWuAi closed 2 months ago

ChenYuWuAi commented 4 months ago

I met problems when sending packet with ack enabled: I received ESP_WIFI_TIMEOUT every time when I called espnow_send function. After looking back into the library, I found bug when declaring the espnow_config struct.

I declared the config struct like

// espnow cfg
espnow_config_t espnow_config = {
    .pmk = "ESP_NOW",
    .forward_enable = 1,
    .forward_switch_channel = 0,
    .sec_enable = 0,
    .reserved1 = 0,
    .qsize = 32,
    .send_retry_num = 10,
    .send_max_timeout = 3000,
    .receive_enable = {
        .ack = 1,
        .forward = 1,
        .group = 1,
        .provisoning = 0,
        .control_bind = 0,
        .control_data = 0,
        .ota_status = 0,
        .ota_data = 0,
        .debug_log = 0,
        .debug_command = 0,
        .data = 0,
        .sec_status = 0,
        .sec = 0,
        .sec_data = 0,
        .reserved2 = 0,
    },
};

In the library, it init ack queues using a struct called g_recv_handle. However g_recv_handle[0].enable is always false because in the code:

uint32_t *enable = (uint32_t *)&config->receive_enable;
    for (int i = 0; i < ESPNOW_DATA_TYPE_MAX; ++i) {
        g_recv_handle[i].enable = (*enable) & BIT(i);
    }

The config struct isn't aligned to 1. So using:

/**
 * @brief Initialize the configuration of espnow
 */
typedef struct {
    const uint8_t pmk[16];                  /**< Primary master key */
    bool forward_enable             : 1;    /**< Forward when packets are received */
    bool forward_switch_channel     : 1;    /**< Forward data packet with exchange channel */
    bool sec_enable                 : 1;    /**< Encrypt ESP-NOW data payload when send and decrypt when receive */
    uint8_t reserved1               : 5;    /**< Reserved */
    uint8_t qsize;                          /**< Size of packet buffer queue */
    uint8_t send_retry_num;                 /**< Number of retransmissions */
    uint32_t send_max_timeout;              /**< Maximum timeout */
    struct {
        bool ack                    : 1;    /**< Enable or disable ACK */
        bool forward                : 1;    /**< Enable or disable forword */
        bool group                  : 1;    /**< Enable or disable group */
        bool provisoning            : 1;    /**< Enable or disable provisoning */
        bool control_bind           : 1;    /**< Enable or disable control bind */
        bool control_data           : 1;    /**< Enable or disable control data */
        bool ota_status             : 1;    /**< Enable or disable OTA status */
        bool ota_data               : 1;    /**< Enable or disable OTA data */
        bool debug_log              : 1;    /**< Enable or disable debug LOG */
        bool debug_command          : 1;    /**< Enable or disable debug command */
        bool data                   : 1;    /**< Enable or disable data */
        bool sec_status             : 1;    /**< Enable or disable security status */
        bool sec                    : 1;    /**< Enable or disable security */
        bool sec_data               : 1;    /**< Enable or disable security data */
        uint32_t reserved2          : 18;   /**< Reserved */
    } receive_enable;            /**< Set 1 to enable receiving the corresponding ESP-NOW data type */
} ESPNOW_PACKED_STRUCT espnow_config_t;

which is in espnow.h

I solved the problem.

lhespress commented 3 months ago

@ChenYuWuAi Could you provide the code which can reproduce the issue? I modified the get-started code at the attachment esp_now_init.zip as you declared, but can't reproduce g_recv_handle[0].enable is always false.

The g_recv_handle[i].enable value as follows:

I (614) espnow: espnow_init 1059 0, 1
I (614) espnow: espnow_init 1059 1, 1
I (624) espnow: espnow_init 1059 2, 1
I (624) espnow: espnow_init 1059 3, 0
I (634) espnow: espnow_init 1059 4, 0
I (634) espnow: espnow_init 1059 5, 0
I (634) espnow: espnow_init 1059 6, 0
I (644) espnow: espnow_init 1059 7, 0
I (644) espnow: espnow_init 1059 8, 0
I (654) espnow: espnow_init 1059 9, 0
I (654) espnow: espnow_init 1059 10, 0
I (664) espnow: espnow_init 1059 11, 0
I (664) espnow: espnow_init 1059 12, 0
I (664) espnow: espnow_init 1059 13, 0
I (674) espnow: espnow_init 1059 14, 0
lhespress commented 2 months ago

@ChenYuWuAi Closing this issue since there has been no update on this. Please feel free to reopen if required.