Bootloader: on_mtu_get_request -- Question

[dingari]

Hi, I have a quick question regarding the DFU MTU get request.

According to the docs page, this should return the MTU used by the transport, but in reality nothing is transmitted back (the response is simply 60 07 01 and nothing more).

Looking at the implementation of the request handling, it indeed does not do anything:

static void on_mtu_get_request(nrf_dfu_request_t * p_req, nrf_dfu_response_t * p_res)
{
    NRF_LOG_DEBUG("Handle NRF_DFU_OP_MTU_GET");
    p_res->mtu.size = p_req->mtu.size;
}

Am I misunderstanding anything?

[bjorn-spockeli]

Hi Dingari, 

I agree that this looks like a bug. I would expect that the  p_res->mtu.size should be set to BLE_GATT_ATT_MTU_DEFAULT or some other define.   I have reported this internally.

Best regards

Bjørn

[dingari]

Hi Bjorn, thanks for confirming.

I think actually populating the MTU field with the currently effective MTU value would be even more beneficial.

Additionally, I'm not super familiar with the nRF bootloader library, but I think the code responsible for the actual response needs some attention (actually populating the response payload with the MTU value). This is an excerpt from nrf_dfu_ble.c, where I believe it should take place:

static void ble_dfu_req_handler_callback(nrf_dfu_response_t * p_res, void * p_context)
{
    ASSERT(p_res);
    ASSERT(p_context);

    uint8_t len = 0;
    uint8_t buffer[MAX_RESPONSE_LEN] = {0};

    if (p_res->request == NRF_DFU_OP_OBJECT_WRITE)
    {
        --m_pkt_notif_target_cnt;
        if ((m_pkt_notif_target == 0) || (m_pkt_notif_target_cnt && m_pkt_notif_target > 0))
        {
            return;
        }

        /* Reply with a CRC message and reset the packet counter. */
        m_pkt_notif_target_cnt = m_pkt_notif_target;

        p_res->request = NRF_DFU_OP_CRC_GET;
    }

    len += response_prepare(buffer, p_res->request, p_res->result);

    if (p_res->result != NRF_DFU_RES_CODE_SUCCESS)
    {
        NRF_LOG_WARNING("DFU request %d failed with error: 0x%x", p_res->request, p_res->result);

        if (p_res->result == NRF_DFU_RES_CODE_EXT_ERROR)
        {
            len += response_ext_err_payload_add(buffer, p_res->result, len);
        }

        (void) response_send(buffer, len);
        return;
    }

    switch (p_res->request)
    {
        case NRF_DFU_OP_OBJECT_CREATE:
        case NRF_DFU_OP_OBJECT_EXECUTE:
            break;

        case NRF_DFU_OP_OBJECT_SELECT:
        {
            len += response_select_obj_add(buffer,
                                           p_res->select.max_size,
                                           p_res->select.offset,
                                           p_res->select.crc);
        } break;

        case NRF_DFU_OP_OBJECT_WRITE:
        {
            len += response_crc_add(buffer, p_res->write.offset, p_res->write.crc);
        } break;

        case NRF_DFU_OP_CRC_GET:
        {
            len += response_crc_add(buffer, p_res->crc.offset, p_res->crc.crc);
        } break;

        default:
        {
            // No action.
        } break;
    }

    (void) response_send(buffer, len);
}

[bjorn-spockeli]

Will pass this on to the SDK developers.