Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bluetooth: Mesh: Fix OOB upload in Fw Distribution server #63739

Merged
merged 6 commits into from
Oct 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/zephyr/bluetooth/mesh/dfd_srv.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ struct bt_mesh_dfd_srv {
struct bt_mesh_blob_srv blob;
#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
bool is_oob;
bool is_pending_oob_check;
struct {
uint8_t uri_len;
uint8_t uri[CONFIG_BT_MESH_DFU_URI_MAXLEN];
Expand Down
57 changes: 48 additions & 9 deletions subsys/bluetooth/mesh/dfd_srv.c
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,10 @@ static int handle_apply(struct bt_mesh_model *mod, struct bt_mesh_msg_ctx *ctx,
return 0;
}

static void upload_status_rsp(struct bt_mesh_dfd_srv *srv,
struct bt_mesh_msg_ctx *ctx,
enum bt_mesh_dfd_status status)
static void upload_status_rsp_with_progress(struct bt_mesh_dfd_srv *srv,
struct bt_mesh_msg_ctx *ctx,
enum bt_mesh_dfd_status status,
uint8_t progress)
{
BT_MESH_MODEL_BUF_DEFINE(rsp, BT_MESH_DFD_OP_UPLOAD_STATUS,
DFD_UPLOAD_STATUS_MSG_MAXLEN);
Expand All @@ -361,21 +362,38 @@ static void upload_status_rsp(struct bt_mesh_dfd_srv *srv,

#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
if (srv->upload.is_oob) {
net_buf_simple_add_u8(&rsp,
srv->cb->oob_progress_get(srv, srv->upload.slot) | BIT(7));
net_buf_simple_add_u8(&rsp, progress | BIT(7));
net_buf_simple_add_mem(&rsp, srv->upload.oob.current_fwid,
srv->upload.oob.current_fwid_len);
} else
#endif
{
net_buf_simple_add_u8(&rsp, bt_mesh_blob_srv_progress(&srv->upload.blob));
net_buf_simple_add_u8(&rsp, progress);
net_buf_simple_add_mem(&rsp, srv->upload.slot->fwid,
srv->upload.slot->fwid_len);
}

bt_mesh_model_send(srv->mod, ctx, &rsp, NULL, NULL);
}

static void upload_status_rsp(struct bt_mesh_dfd_srv *srv,
struct bt_mesh_msg_ctx *ctx,
enum bt_mesh_dfd_status status)
{
uint8_t progress;

#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
if (srv->upload.is_oob) {
progress = srv->cb->oob_progress_get(srv, srv->upload.slot);
} else
#endif
{
progress = bt_mesh_blob_srv_progress(&srv->upload.blob);
}

upload_status_rsp_with_progress(srv, ctx, status, progress);
}

static int handle_upload_get(struct bt_mesh_model *mod, struct bt_mesh_msg_ctx *ctx,
struct net_buf_simple *buf)
{
Expand All @@ -400,7 +418,7 @@ static inline int set_upload_fwid(struct bt_mesh_dfd_srv *srv, struct bt_mesh_ms
case -EEXIST: /* Img with this fwid already is in list */
srv->upload.phase = BT_MESH_DFD_UPLOAD_PHASE_TRANSFER_SUCCESS;
bt_mesh_dfu_slot_release(srv->upload.slot);
upload_status_rsp(srv, ctx, BT_MESH_DFD_SUCCESS);
upload_status_rsp_with_progress(srv, ctx, BT_MESH_DFD_SUCCESS, 100);
break;
case 0:
srv->upload.phase = BT_MESH_DFD_UPLOAD_PHASE_TRANSFER_ACTIVE;
Expand Down Expand Up @@ -554,6 +572,10 @@ static int handle_upload_start_oob(struct bt_mesh_model *mod, struct bt_mesh_msg
fwid_len = buf->len;
fwid = net_buf_simple_pull_mem(buf, fwid_len);

LOG_DBG("Upload OOB Start");
LOG_HEXDUMP_DBG(uri, uri_len, "URI");
LOG_HEXDUMP_DBG(fwid, fwid_len, "FWID");

if (upload_is_busy(srv)) {
#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
if (srv->upload.is_oob &&
Expand All @@ -568,6 +590,11 @@ static int handle_upload_start_oob(struct bt_mesh_model *mod, struct bt_mesh_msg
#endif
upload_status_rsp(srv, ctx, BT_MESH_DFD_ERR_BUSY_WITH_UPLOAD);
return 0;
#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
} else if (srv->upload.is_oob && srv->upload.is_pending_oob_check) {
/* Ignore the request if we didn't confirm the previous one. */
return 0;
#endif
}

#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
Expand All @@ -584,6 +611,13 @@ static int handle_upload_start_oob(struct bt_mesh_model *mod, struct bt_mesh_msg
return 0;
}

/* This will be a no-op if the slot state isn't RESERVED, which is
* what we want.
*/
if (srv->upload.slot) {
bt_mesh_dfu_slot_release(srv->upload.slot);
}

srv->upload.is_oob = true;
srv->upload.slot = slot;
memcpy(srv->upload.oob.uri, uri, uri_len);
Expand All @@ -600,6 +634,8 @@ static int handle_upload_start_oob(struct bt_mesh_model *mod, struct bt_mesh_msg
if (status != BT_MESH_DFD_SUCCESS) {
upload_status_rsp(srv, ctx, status);
bt_mesh_dfu_slot_release(srv->upload.slot);
} else {
srv->upload.is_pending_oob_check = true;
}
#else
upload_status_rsp(srv, ctx, BT_MESH_DFD_ERR_URI_NOT_SUPPORTED);
Expand Down Expand Up @@ -743,7 +779,7 @@ const struct bt_mesh_model_op _bt_mesh_dfd_srv_op[] = {
{ BT_MESH_DFD_OP_APPLY, BT_MESH_LEN_EXACT(0), handle_apply },
{ BT_MESH_DFD_OP_UPLOAD_GET, BT_MESH_LEN_EXACT(0), handle_upload_get },
{ BT_MESH_DFD_OP_UPLOAD_START, BT_MESH_LEN_MIN(16), handle_upload_start },
{ BT_MESH_DFD_OP_UPLOAD_START_OOB, BT_MESH_LEN_EXACT(2), handle_upload_start_oob },
{ BT_MESH_DFD_OP_UPLOAD_START_OOB, BT_MESH_LEN_MIN(2), handle_upload_start_oob },
alxelax marked this conversation as resolved.
Show resolved Hide resolved
{ BT_MESH_DFD_OP_UPLOAD_CANCEL, BT_MESH_LEN_EXACT(0), handle_upload_cancel },
{ BT_MESH_DFD_OP_FW_GET, BT_MESH_LEN_MIN(0), handle_fw_get },
{ BT_MESH_DFD_OP_FW_GET_BY_INDEX, BT_MESH_LEN_EXACT(2), handle_fw_get_by_index },
Expand Down Expand Up @@ -1193,13 +1229,16 @@ int bt_mesh_dfd_srv_oob_check_complete(struct bt_mesh_dfd_srv *srv,
int err;

if (slot != srv->upload.slot || !srv->upload.is_oob ||
srv->upload.phase == BT_MESH_DFD_UPLOAD_PHASE_TRANSFER_ACTIVE) {
srv->upload.phase == BT_MESH_DFD_UPLOAD_PHASE_TRANSFER_ACTIVE ||
!srv->upload.is_pending_oob_check) {
/* This should not happen, unless the application calls the function with a
* "wrong" pointer or at a wrong time.
*/
return -EINVAL;
}

srv->upload.is_pending_oob_check = false;

if (status != BT_MESH_DFD_SUCCESS) {
bt_mesh_dfu_slot_release(srv->upload.slot);
upload_status_rsp(srv, &srv->upload.oob.ctx, status);
Expand Down
119 changes: 119 additions & 0 deletions tests/bluetooth/tester/src/btp_mesh.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,132 @@ static int dfd_srv_send(struct bt_mesh_dfd_srv *srv,
return 0;
}

#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
static struct {
uint8_t uri[CONFIG_BT_MESH_DFU_URI_MAXLEN];
uint8_t uri_len;
uint8_t fwid[CONFIG_BT_MESH_DFU_FWID_MAXLEN];
uint8_t fwid_len;
const struct bt_mesh_dfu_slot *slot;
uint8_t progress;
bool started;
} dfd_srv_oob_ctx;

static void oob_check_handler(struct k_work *work);
static K_WORK_DEFINE(oob_check, oob_check_handler);
static void oob_store_handler(struct k_work *work);
static K_WORK_DEFINE(oob_store, oob_store_handler);

static int dfd_srv_start_oob_upload(struct bt_mesh_dfd_srv *srv,
const struct bt_mesh_dfu_slot *slot,
const char *uri, uint8_t uri_len,
const uint8_t *fwid, uint16_t fwid_len)
{
LOG_DBG("Start OOB Upload");

memcpy(dfd_srv_oob_ctx.uri, uri, uri_len);
dfd_srv_oob_ctx.uri_len = uri_len;
memcpy(dfd_srv_oob_ctx.fwid, fwid, fwid_len);
dfd_srv_oob_ctx.fwid_len = fwid_len;
dfd_srv_oob_ctx.slot = slot;
dfd_srv_oob_ctx.progress = 0;
dfd_srv_oob_ctx.started = true;

k_work_submit(&oob_check);

return BT_MESH_DFD_SUCCESS;
}

static void dfd_srv_cancel_oob_upload(struct bt_mesh_dfd_srv *srv,
const struct bt_mesh_dfu_slot *slot)
{
LOG_DBG("Cancel OOB Upload");

dfd_srv_oob_ctx.started = false;
alxelax marked this conversation as resolved.
Show resolved Hide resolved
}

static uint8_t dfd_srv_oob_progress_get(struct bt_mesh_dfd_srv *srv,
const struct bt_mesh_dfu_slot *slot)
{
uint8_t progress;

if (dfd_srv_oob_ctx.started) {
progress = dfd_srv_oob_ctx.progress;

dfd_srv_oob_ctx.progress = MIN(dfd_srv_oob_ctx.progress + 25, 99);

if (dfd_srv_oob_ctx.progress == 99) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be >= or <=, or is it correct with exact 99. dfd_srv_oob_ctx.progress can be less than 99.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASFAIK The image is ready for storing if progress is exactly 99. After storing it will be 100.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, MIN will not let it to be set greater than 99.

k_work_submit(&oob_store);
}
} else {
progress = 0;
}

LOG_DBG("OOB Progress Get (%sstarted: %d %%)", dfd_srv_oob_ctx.started ? "" : "not ",
progress);
return progress;
}
#endif /* CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD */

static struct bt_mesh_dfd_srv_cb dfd_srv_cb = {
.recv = dfd_srv_recv,
.del = dfd_srv_del,
.send = dfd_srv_send,
#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
.start_oob_upload = dfd_srv_start_oob_upload,
.cancel_oob_upload = dfd_srv_cancel_oob_upload,
.oob_progress_get = dfd_srv_oob_progress_get,
#endif
};

static struct bt_mesh_dfd_srv dfd_srv = BT_MESH_DFD_SRV_INIT(&dfd_srv_cb);

#ifdef CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD
#define SUPPORTED_SCHEME "http"
alxelax marked this conversation as resolved.
Show resolved Hide resolved

static void oob_check_handler(struct k_work *work)
{
uint8_t scheme[10];
int i;
int status;
int err;

for (i = 0; i < MIN(dfd_srv_oob_ctx.uri_len, sizeof(scheme)); i++) {
if (IN_RANGE(dfd_srv_oob_ctx.uri[i], 48, 57) || /* DIGIT */
IN_RANGE(dfd_srv_oob_ctx.uri[i], 65, 90) || /* ALPHA UPPER CASE */
IN_RANGE(dfd_srv_oob_ctx.uri[i], 97, 122) || /* ALPHA LOWER CASE */
dfd_srv_oob_ctx.uri[i] == '.' ||
dfd_srv_oob_ctx.uri[i] == '+' ||
dfd_srv_oob_ctx.uri[i] == '-') {
scheme[i] = dfd_srv_oob_ctx.uri[i];
} else {
break;
}
}

if (i == dfd_srv_oob_ctx.uri_len || dfd_srv_oob_ctx.uri[i] != ':') {
status = BT_MESH_DFD_ERR_URI_MALFORMED;
} else if (i != strlen(SUPPORTED_SCHEME) ||
memcmp(scheme, SUPPORTED_SCHEME, strlen(SUPPORTED_SCHEME))) {
status = BT_MESH_DFD_ERR_URI_NOT_SUPPORTED;
} else {
status = BT_MESH_DFD_SUCCESS;
}

err = bt_mesh_dfd_srv_oob_check_complete(&dfd_srv, dfd_srv_oob_ctx.slot, status,
dfd_srv_oob_ctx.fwid, dfd_srv_oob_ctx.fwid_len);
LOG_DBG("OOB check completed (err %d)", err);
}

static void oob_store_handler(struct k_work *work)
{
int err;

err = bt_mesh_dfd_srv_oob_store_complete(&dfd_srv, dfd_srv_oob_ctx.slot, true,
10000, "metadata", 8);
LOG_DBG("OOB store completed (err %d)", err);
}
#endif /* CONFIG_BT_MESH_DFD_SRV_OOB_UPLOAD */
#endif

#if defined(CONFIG_BT_MESH_BLOB_CLI) && !defined(CONFIG_BT_MESH_DFD_SRV)
Expand Down
Loading