This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

cli_agent_ep_handler_ping When to free zb_bufid_t

Setup: Segger Embedded Studio v3.52, nRF5_SDK_for_Thread_and_Zigbee_v4, nRF52840 Custom Board

Hello,

I based my code off of your Zigbee cli_agent_router example. In the routine cli_agent_ep_handler_ping(zb_bufid_t bufid), the buffer "bufid" is released at certain points, but not everywhere I think it should be released. I've attached the routine below. 

/**@brief The Handler to 'intercept' every frame coming to the endpoint
 *
 * @param bufid    Reference to a ZBOSS buffer
 */
static zb_uint8_t cli_agent_ep_handler_ping(zb_bufid_t bufid)
{
    zb_zcl_parsed_hdr_t * p_cmd_info = ZB_BUF_GET_PARAM(bufid, zb_zcl_parsed_hdr_t);
    zb_uint32_t time_diff;

    if (p_cmd_info->cluster_id != PING_CUSTOM_CLUSTER ||
        p_cmd_info->profile_id != ZB_AF_HA_PROFILE_ID)
    {
        return ZB_FALSE;
    }

    NRF_LOG_INST_DEBUG(m_log.p_log, "New ping frame received, bufid: %d", bufid);
    ping_req_indicate(bufid);

    if (p_cmd_info->cmd_id == PING_ECHO_REPLY)
    {
        zb_uint16_t remote_short_addr = 0x0000;

        /* We have our ping reply */
        ping_request_t * p_request = find_request_by_sn(p_cmd_info->seq_number);
        if (p_request == NULL)
        {
            return ZB_FALSE;
        }

        if (p_request->remote_addr_mode == ZB_APS_ADDR_MODE_16_ENDP_PRESENT)
        {
            remote_short_addr = p_request->remote_addr.addr_short;
        }
        else
        {
            remote_short_addr = zb_address_short_by_ieee(p_request->remote_addr.addr_long);
        }

        if (p_cmd_info->addr_data.common_data.source.addr_type == ZB_ZCL_ADDR_TYPE_SHORT)
        {
            if (remote_short_addr != p_cmd_info->addr_data.common_data.source.u.short_addr)
            {
                return ZB_FALSE;
            }
        }
        else
        {
            return ZB_FALSE;
        }

        /* Catch the timers value */
        time_diff = get_request_duration(p_request);

        /* Cancel the ongoing alarm which was to erase the row... */
        zb_ret_t zb_err_code = ZB_SCHEDULE_APP_ALARM_CANCEL(invalidate_row_cb,
                                                            get_request_row(p_request));
        ZB_ERROR_CHECK(zb_err_code);

        /* Call callback function in order to indicate echo response reception. */
        if (p_request->p_cb)
        {
            p_request->p_cb(PING_EVT_ECHO_RECEIVED, time_diff, p_request);
        }

        /* ...and erase it manually */
        if (zb_err_code == RET_OK)
        {
            zb_ping_release_request(p_request);
        }

    }
    else if ((p_cmd_info->cmd_id == PING_ECHO_REQUEST) ||
             (p_cmd_info->cmd_id == PING_ECHO_NO_ACK_REQUEST))
    {
        zb_uint8_t     len = zb_buf_len(bufid);
        ping_reply_t * p_reply = ping_aquire_reply();

        if (p_reply == NULL)
        {
            NRF_LOG_INST_WARNING(m_log.p_log, "Cannot obtain new row for incoming ping request");
            return ZB_FALSE;
        }

        /* Save the Ping Reply information in the table and schedule a sending function */
        p_reply->count = len;
        p_reply->ping_seq = p_cmd_info->seq_number;

        if (p_cmd_info->cmd_id == PING_ECHO_REQUEST)
        {
            NRF_LOG_INST_DEBUG(m_log.p_log,
                               "PING echo request with APS ACK received");
            p_reply->send_ack = 1;
        }
        else
        {
            NRF_LOG_INST_DEBUG(m_log.p_log,
                               "PING echo request without APS ACK received");
            p_reply->send_ack = 0;
        }

        if (p_cmd_info->addr_data.common_data.source.addr_type == ZB_ZCL_ADDR_TYPE_SHORT)
        {
            p_reply->remote_short_addr = p_cmd_info->addr_data.common_data.source.u.short_addr;
        }
        else
        {
            NRF_LOG_INST_WARNING(m_log.p_log, "Drop ping request due to incorrect address type");
            ping_release_reply(p_reply);
            zb_buf_free(bufid);
            return ZB_TRUE;
        }

        /* Send the Ping Reply, if not possible then invalidate the row */
        ping_reply_send(p_reply);
    }
    else if (p_cmd_info->cmd_id == PING_NO_ECHO_REQUEST)
    {
        NRF_LOG_INST_DEBUG(m_log.p_log,
                          "PING request without ECHO received");
    }
    else
    {
        NRF_LOG_INST_WARNING(m_log.p_log,
                             "Unsupported Ping message received, cmd_id %d\r\n",
                             p_cmd_info->cmd_id);
    }

    zb_buf_free(bufid);
    return ZB_TRUE;
}

Can you explain why zb_buf_free(bufid) is not called after:

ping_request_t * p_request = find_request_by_sn(p_cmd_info->seq_number);
if (p_request == NULL)
{
    // I think zb_buf_free(bufid) should be called here
    return ZB_FALSE;
}

if (p_request->remote_addr_mode == ZB_APS_ADDR_MODE_16_ENDP_PRESENT)
{
    remote_short_addr = p_request->remote_addr.addr_short;
}
else
{
    remote_short_addr = zb_address_short_by_ieee(p_request->remote_addr.addr_long);
}

if (p_cmd_info->addr_data.common_data.source.addr_type == ZB_ZCL_ADDR_TYPE_SHORT)
{
    if (remote_short_addr != p_cmd_info->addr_data.common_data.source.u.short_addr)
    {
        // I think zb_buf_free(bufid) should be called here
        return ZB_FALSE;
    }
}
else
{
    // I think zb_buf_free(bufid) should be called here
    return ZB_FALSE;
}

Thanks!

John

Parents
  • Hello John,

    I checked with the Zigbee team. They said that if the endpoint handler returns ZB_FALSE, then the zigbee stack will process the frame, and then free the buffer, so you can't free it in this handler. Only if you return ZB_TRUE, you will need to manually free the buffer from the handler.

    So in this case, since it will return ZB_FALSE, the stack needs the buffer for further processing, and it will later be freed by the stack itself.

    Best regards,

    Edvin

Reply
  • Hello John,

    I checked with the Zigbee team. They said that if the endpoint handler returns ZB_FALSE, then the zigbee stack will process the frame, and then free the buffer, so you can't free it in this handler. Only if you return ZB_TRUE, you will need to manually free the buffer from the handler.

    So in this case, since it will return ZB_FALSE, the stack needs the buffer for further processing, and it will later be freed by the stack itself.

    Best regards,

    Edvin

Children
Related