Default MWIFI mesh event handler hides mesh event information to application level event handlers

raulMrello commented 5 years ago

Environment

Development Kit: [none]
Kit version (for WroverKit/PicoKit/DevKitC): []
Core (if using chip or module): [ESP-WROOM32]
MDF version (git rev-parse --short HEAD to get the commit id.): 0c64172
Development Env: [Make|Eclipse]
Operating System: [Windows]
Power Supply: [USB]

Problem Description

Default mwifi event handler static void esp_mesh_event_cb(mesh_event_t event) hides important information about the event when it forwards to application level event handlers:

At mwifi.c line 128 the event handler forwards the event to upper handlers at application level, as shown below:

mdf_event_loop_send(event.id, NULL);

As you can see the second argument is NULL, so event information is hidden to those upper handlers at app level.

Expected Behavior

Event information should be forwarded, replacing:

mdf_event_loop_send(event.id, NULL);

With the event information:

mdf_event_loop_send(event.id, &event.info);

Steps to reproduce

Let's follow this example https://github.com/espressif/esp-mdf/blob/master/examples/function_demo/mwifi/main/mwifi_example.c
At line 336 starts the application level event handler static mdf_err_t event_loop_cb(mdf_event_loop_t event, void *ctx){...

Now we add a switch case to check child nodes disconnections, for example at line 357 we add this code:

   case MDF_EVENT_MWIFI_CHILD_DISCONNECTED:
        MDF_LOGI("Child is disconnected");
        break;

But as event information is hidden we cannot handle which child (mac address) has been disconnected.

With the proposed modification, we can get mac address of the disconnected child through this piece of code:

   case MDF_EVENT_MWIFI_CHILD_DISCONNECTED:
        mesh_event_info_t* event_info = (mesh_event_info_t*)ctx;            
        MDF_LOGI("Disconnected Child with mac: " MACSTR, MAC2STR(event_info->child_disconnected.mac));
        break;

Related stuff

As event information defined in file esp_mesh.h includes different event information:

/**
 * @brief Mesh event information
 */
typedef union {
    mesh_event_channel_switch_t channel_switch;            /**< channel switch */
    mesh_event_child_connected_t child_connected;          /**< child connected */
    mesh_event_child_disconnected_t child_disconnected;    /**< child disconnected */
    mesh_event_routing_table_change_t routing_table;       /**< routing table change */
    mesh_event_connected_t connected;                      /**< parent connected */
    mesh_event_disconnected_t disconnected;                /**< parent disconnected */
    mesh_event_no_parent_found_t no_parent;                /**< no parent found */
    mesh_event_layer_change_t layer_change;                /**< layer change */
    mesh_event_toDS_state_t toDS_state;                    /**< toDS state, devices shall check this state firstly before trying to send packets to
                                                                external IP network. This state indicates right now whether the root is capable of sending
                                                                packets out. If not, devices had better to wait until this state changes to be
                                                                MESH_TODS_REACHABLE. */
    mesh_event_vote_started_t vote_started;                /**< vote started */
    mesh_event_root_got_ip_t got_ip;                       /**< root obtains IP address */
    mesh_event_root_address_t root_addr;                   /**< root address */
    mesh_event_root_switch_req_t switch_req;               /**< root switch request */
    mesh_event_root_conflict_t root_conflict;              /**< other powerful root */
    mesh_event_root_fixed_t root_fixed;                    /**< fixed root */
    mesh_event_scan_done_t scan_done;                      /**< scan done */
    mesh_event_network_state_t network_state;              /**< network state, such as whether current mesh network has a root. */
    mesh_event_find_network_t find_network;                /**< network found that can join */
    mesh_event_router_switch_t router_switch;              /**< new router information */
} mesh_event_info_t;

... this will allow to have higher control of the handled mesh event at application level.

zhanzhaocheng commented 5 years ago

There is no good way to return the ESP-MESH event parameters to the upper level at the current level.
- The restrictions are: - Processing directly in the event will affect the normal operation of the ESP-MESH. - A dynamic variable of the parameter of the event, which will be released immediately after the event is returned.

Scenario: (replace mdf_event_loop_send(event.id, NULL); as follows) - mdf_event_loop_send(event.id, &event.info); The space of event has been released when event.info is used, causing memory access exceptions. - Use static variable caching c Stiatic mesh_event_t s_event_info = {0x0}; Memcpy(&s_event_info, &event.info, sizeof(mesh_event_t)); Mdf_event_loop_send(event.id, &event.info);` When using this method, when multiple ESP-MESH events are received at the same time, the parameters of the event will be modified. - Call the upper callback function directly c Mdf_event_loop(event.id, &event.info); With this method, the use of the upper callback function must be very careful. If there is a blocking operation or a large stack is needed, it will affect the normal operation of ESP-MESH. - Add a list of events to buffer Since the event parameters of ESP-MESH are relatively large, the buffer queue will only occupy a large amount of memory. This scheme is pending, and this scheme may be used afterwards. If you have a better method, please give us suggestions.

The application layer does not have to use the parameters of the event. - The parameters of the event can basically be obtained through other APIs. If you receive the level change, you can get it directly through esp_mesh_get_layer(); - Many of the parameters of the event are applications that serve the scheme of manual networking. No need to care.
About MDF_EVENT_MWIFI_CHILD_DISCONNECTED: The event of ESP-MESH can only get the information of the direct child node. If there is a device under the child node, you will not know its disconnection. If you want to get all the offline devices, you need to store the devices connected to the current device. When there is a child node disconnection event, you can get the number of connections under the device again to compare with the previous ones to determine which devices are disconnected. Using this method will consume memory (number of devices × 6 Bytes), if you think this solution is feasible, I will provide you with an example of use or directly into the corresponding interface in mwifi.c

Jigar3690 commented 3 years ago

Hello, I just came across the same issue. Where I need to access the important details of the mesh at application level. which is not really accessible unless I put my code inside the library code.

For example: If I like to access the network state at application level callback function to know whether it's a rootless mesh or not I can't really do it.

        case MESH_EVENT_NETWORK_STATE:
            g_rootless_flag = event.info.network_state.is_rootless;
           if (g_rootless_flag) {
                    // My code here
          }

Or even if we can have enough get functions to access those details would be good. As for now there are lot of important details of the mesh that we can't really access at application level.

Please reply. Thank you.

EspHuifeng commented 3 years ago

Hi， @Jigar3690 . mdf_event_loop can already pass the underlying event information. see https://github.com/espressif/esp-mdf/blob/1c76187faaf16d40a574fc7c0c98122e93dee746/components/mwifi/mwifi.c#L288

mmrein commented 2 years ago

3. About MDF_EVENT_MWIFI_CHILD_DISCONNECTED: The event of ESP-MESH can only get the information of the direct child node. If there is a device under the child node, you will not know its disconnection. If you want to get all the offline devices, you need to store the devices connected to the current device. When there is a child node disconnection event, you can get the number of connections under the device again to compare with the previous ones to determine which devices are disconnected. Using this method will consume memory (number of devices × 6 Bytes), if you think this solution is feasible, I will provide you with an example of use or directly into the corresponding interface in mwifi.c

Hello, I'm interested in such example. I'm using PSRAM so memory should not be an issue.

Not sure how much has changed since then or if I simply missed some API that can provide information about currently connected / disconnected nodes.

espressif / esp-mdf