You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2017/03/02 16:17:34 UTC
[1/2] incubator-mynewt-core git commit: MYNEWT-647 Changes to NMP
over OIC scheme
Repository: incubator-mynewt-core
Updated Branches:
refs/heads/develop a4df93c10 -> 382847055
MYNEWT-647 Changes to NMP over OIC scheme
Update NMP handlers to account for new scheme. Now the top-level
handler creates and closes the response root map rather than each
handler.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/38284705
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/38284705
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/38284705
Branch: refs/heads/develop
Commit: 3828470558992547a6243df028af3142937cec85
Parents: 860d2d2
Author: Christopher Collins <cc...@apache.org>
Authored: Wed Mar 1 17:48:55 2017 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Thu Mar 2 08:15:05 2017 -0800
----------------------------------------------------------------------
fs/fs/src/fs_nmgr.c | 63 +++++---------
mgmt/imgmgr/src/imgmgr.c | 38 +++------
mgmt/imgmgr/src/imgmgr_coredump.c | 70 ++++++++-------
mgmt/imgmgr/src/imgmgr_state.c | 41 ++++-----
mgmt/newtmgr/nmgr_os/src/newtmgr_os.c | 132 ++++++++++++++---------------
mgmt/newtmgr/src/newtmgr.c | 54 ++++++++++--
sys/config/src/config_nmgr.c | 8 +-
sys/log/full/src/log_nmgr.c | 81 ++++++++----------
sys/stats/full/src/stats_nmgr.c | 51 +++++------
test/crash_test/src/crash_nmgr.c | 23 +++--
test/runtest/src/runtest_nmgr.c | 21 ++---
11 files changed, 274 insertions(+), 308 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/fs/fs/src/fs_nmgr.c
----------------------------------------------------------------------
diff --git a/fs/fs/src/fs_nmgr.c b/fs/fs/src/fs_nmgr.c
index d04d768..bd87697 100644
--- a/fs/fs/src/fs_nmgr.c
+++ b/fs/fs/src/fs_nmgr.c
@@ -90,19 +90,15 @@ fs_nmgr_file_download(struct mgmt_cbuf *cb)
uint32_t out_len;
struct fs_file *file;
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp;
rc = cbor_read_object(&cb->it, dload_attr);
if (rc || off == UINT_MAX) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
rc = fs_open(tmp_str, FS_ACCESS_READ, &file);
if (rc || !file) {
- rc = MGMT_ERR_ENOMEM;
- goto err;
+ return MGMT_ERR_ENOMEM;
}
rc = fs_seek(file, off);
@@ -116,34 +112,30 @@ fs_nmgr_file_download(struct mgmt_cbuf *cb)
goto err_close;
}
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
-
- g_err |= cbor_encode_text_stringz(&rsp, "off");
- g_err |= cbor_encode_uint(&rsp, off);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "off");
+ g_err |= cbor_encode_uint(&cb->encoder, off);
- g_err |= cbor_encode_text_stringz(&rsp, "data");
- g_err |= cbor_encode_byte_string(&rsp, img_data, out_len);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "data");
+ g_err |= cbor_encode_byte_string(&cb->encoder, img_data, out_len);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
if (off == 0) {
rc = fs_filelen(file, &out_len);
- g_err |= cbor_encode_text_stringz(&rsp, "len");
- g_err |= cbor_encode_uint(&rsp, out_len);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "len");
+ g_err |= cbor_encode_uint(&cb->encoder, out_len);
}
- g_err |= cbor_encoder_close_container(penc, &rsp);
fs_close(file);
if (g_err) {
- return MGMT_ERR_ENOMEM;
+ return MGMT_ERR_ENOMEM;
}
+
return 0;
err_close:
fs_close(file);
-err:
- mgmt_cbuf_setoerr(cb, rc);
- return 0;
+ return rc;
}
static int
@@ -183,14 +175,11 @@ fs_nmgr_file_upload(struct mgmt_cbuf *cb)
[4] = { 0 },
};
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp;
int rc;
rc = cbor_read_object(&cb->it, off_attr);
if (rc || off == UINT_MAX) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (off == 0) {
@@ -201,8 +190,7 @@ fs_nmgr_file_upload(struct mgmt_cbuf *cb)
fs_nmgr_state.upload.size = size;
if (!strlen(file_name)) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (fs_nmgr_state.upload.file) {
fs_close(fs_nmgr_state.upload.file);
@@ -211,8 +199,7 @@ fs_nmgr_file_upload(struct mgmt_cbuf *cb)
rc = fs_open(file_name, FS_ACCESS_WRITE | FS_ACCESS_TRUNCATE,
&fs_nmgr_state.upload.file);
if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
} else if (off != fs_nmgr_state.upload.off) {
/*
@@ -223,8 +210,7 @@ fs_nmgr_file_upload(struct mgmt_cbuf *cb)
}
if (!fs_nmgr_state.upload.file) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (img_len) {
rc = fs_write(fs_nmgr_state.upload.file, img_data, img_len);
@@ -239,13 +225,12 @@ fs_nmgr_file_upload(struct mgmt_cbuf *cb)
fs_nmgr_state.upload.file = NULL;
}
}
+
out:
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "off");
- g_err |= cbor_encode_uint(&rsp, fs_nmgr_state.upload.off);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "off");
+ g_err |= cbor_encode_uint(&cb->encoder, fs_nmgr_state.upload.off);
if (g_err) {
return MGMT_ERR_ENOMEM;
}
@@ -254,9 +239,7 @@ out:
err_close:
fs_close(fs_nmgr_state.upload.file);
fs_nmgr_state.upload.file = NULL;
-err:
- mgmt_cbuf_setoerr(cb, rc);
- return 0;
+ return rc;
}
int
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/mgmt/imgmgr/src/imgmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/imgmgr/src/imgmgr.c b/mgmt/imgmgr/src/imgmgr.c
index 32744e2..7b4431e 100644
--- a/mgmt/imgmgr/src/imgmgr.c
+++ b/mgmt/imgmgr/src/imgmgr.c
@@ -257,14 +257,11 @@ imgr_upload(struct mgmt_cbuf *cb)
int best;
int rc;
int i;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp;
CborError g_err = CborNoError;
rc = cbor_read_object(&cb->it, off_attr);
if (rc || off == UINT_MAX) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (off == 0) {
@@ -272,13 +269,11 @@ imgr_upload(struct mgmt_cbuf *cb)
/*
* Image header is the first thing in the image.
*/
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
hdr = (struct image_header *)img_data;
if (hdr->ih_magic != IMAGE_MAGIC) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
/*
@@ -318,12 +313,10 @@ imgr_upload(struct mgmt_cbuf *cb)
}
rc = flash_area_open(area_id, &imgr_state.upload.fa);
if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (IMAGE_SIZE(hdr) > imgr_state.upload.fa->fa_size) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
/*
* XXX only erase if needed.
@@ -334,8 +327,7 @@ imgr_upload(struct mgmt_cbuf *cb)
/*
* No slot where to upload!
*/
- rc = MGMT_ERR_ENOMEM;
- goto err;
+ return MGMT_ERR_ENOMEM;
}
} else if (off != imgr_state.upload.off) {
/*
@@ -346,8 +338,7 @@ imgr_upload(struct mgmt_cbuf *cb)
}
if (!imgr_state.upload.fa) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (data_len) {
rc = flash_area_write(imgr_state.upload.fa, imgr_state.upload.off,
@@ -363,13 +354,12 @@ imgr_upload(struct mgmt_cbuf *cb)
imgr_state.upload.fa = NULL;
}
}
+
out:
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "off");
- g_err |= cbor_encode_int(&rsp, imgr_state.upload.off);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "off");
+ g_err |= cbor_encode_int(&cb->encoder, imgr_state.upload.off);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -378,9 +368,7 @@ out:
err_close:
flash_area_close(imgr_state.upload.fa);
imgr_state.upload.fa = NULL;
-err:
- mgmt_cbuf_setoerr(cb, rc);
- return 0;
+ return rc;
}
void
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/mgmt/imgmgr/src/imgmgr_coredump.c
----------------------------------------------------------------------
diff --git a/mgmt/imgmgr/src/imgmgr_coredump.c b/mgmt/imgmgr/src/imgmgr_coredump.c
index df3e321..48058c9 100644
--- a/mgmt/imgmgr/src/imgmgr_coredump.c
+++ b/mgmt/imgmgr/src/imgmgr_coredump.c
@@ -40,20 +40,24 @@ imgr_core_list(struct mgmt_cbuf *cb)
int rc;
rc = flash_area_open(MYNEWT_VAL(COREDUMP_FLASH_AREA), &fa);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
- } else {
- rc = flash_area_read(fa, 0, &hdr, sizeof(hdr));
- if (rc != 0) {
- rc = MGMT_ERR_EINVAL;
- } else if (hdr.ch_magic != COREDUMP_MAGIC) {
- rc = MGMT_ERR_ENOENT;
- } else {
- rc = 0;
- }
+ if (rc != 0) {
+ return MGMT_ERR_EUNKNOWN;
+ }
+
+ rc = flash_area_read(fa, 0, &hdr, sizeof(hdr));
+ if (rc != 0) {
+ return MGMT_ERR_EINVAL;
+ }
+
+ if (hdr.ch_magic != COREDUMP_MAGIC) {
+ return MGMT_ERR_ENOENT;
+ }
+
+ rc = mgmt_cbuf_setoerr(cb, 0);
+ if (rc != 0) {
+ return rc;
}
- mgmt_cbuf_setoerr(cb, rc);
return 0;
}
@@ -75,21 +79,17 @@ imgr_core_load(struct mgmt_cbuf *cb)
uint8_t data[IMGMGR_NMGR_MAX_MSG];
struct coredump_header *hdr;
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp;
hdr = (struct coredump_header *)data;
rc = cbor_read_object(&cb->it, dload_attr);
if (rc || off == UINT_MAX) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
rc = flash_area_open(MYNEWT_VAL(COREDUMP_FLASH_AREA), &fa);
if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
rc = flash_area_read(fa, 0, hdr, sizeof(*hdr));
@@ -115,14 +115,12 @@ imgr_core_load(struct mgmt_cbuf *cb)
goto err_close;
}
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "off");
- g_err |= cbor_encode_int(&rsp, off);
- g_err |= cbor_encode_text_stringz(&rsp, "data");
- g_err |= cbor_encode_byte_string(&rsp, data, sz);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "off");
+ g_err |= cbor_encode_int(&cb->encoder, off);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "data");
+ g_err |= cbor_encode_byte_string(&cb->encoder, data, sz);
flash_area_close(fa);
if (g_err) {
@@ -132,8 +130,6 @@ imgr_core_load(struct mgmt_cbuf *cb)
err_close:
flash_area_close(fa);
-err:
- mgmt_cbuf_setoerr(cb, rc);
return rc;
}
@@ -148,24 +144,26 @@ imgr_core_erase(struct mgmt_cbuf *cb)
int rc;
rc = flash_area_open(MYNEWT_VAL(COREDUMP_FLASH_AREA), &fa);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ if (rc != 0) {
+ return MGMT_ERR_EINVAL;
}
rc = flash_area_read(fa, 0, &hdr, sizeof(hdr));
if (rc == 0 &&
(hdr.ch_magic == COREDUMP_MAGIC || hdr.ch_magic == 0xffffffff)) {
rc = flash_area_erase(fa, 0, fa->fa_size);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
+ if (rc != 0) {
+ return MGMT_ERR_EINVAL;
}
}
- rc = 0;
flash_area_close(fa);
-err:
- mgmt_cbuf_setoerr(cb, rc);
+
+ rc = mgmt_cbuf_setoerr(cb, rc);
+ if (rc != 0) {
+ return rc;
+ }
+
return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/mgmt/imgmgr/src/imgmgr_state.c
----------------------------------------------------------------------
diff --git a/mgmt/imgmgr/src/imgmgr_state.c b/mgmt/imgmgr/src/imgmgr_state.c
index ee7d76c..610b57f 100644
--- a/mgmt/imgmgr/src/imgmgr_state.c
+++ b/mgmt/imgmgr/src/imgmgr_state.c
@@ -240,15 +240,15 @@ imgmgr_state_read(struct mgmt_cbuf *cb)
int split_status;
uint8_t state_flags;
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, images, image;
+ CborEncoder images;
+ CborEncoder image;
any_non_bootable = 0;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "images");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "images");
- g_err |= cbor_encoder_create_array(&rsp, &images, CborIndefiniteLength);
+ g_err |= cbor_encoder_create_array(&cb->encoder, &images,
+ CborIndefiniteLength);
for (i = 0; i < 2; i++) {
rc = imgr_read_info(i, &ver, hash, &flags);
if (rc != 0) {
@@ -261,7 +261,8 @@ imgmgr_state_read(struct mgmt_cbuf *cb)
state_flags = imgmgr_state_flags(i);
- g_err |= cbor_encoder_create_map(&images, &image, CborIndefiniteLength);
+ g_err |= cbor_encoder_create_map(&images, &image,
+ CborIndefiniteLength);
g_err |= cbor_encode_text_stringz(&image, "slot");
g_err |= cbor_encode_int(&image, i);
@@ -294,7 +295,7 @@ imgmgr_state_read(struct mgmt_cbuf *cb)
g_err |= cbor_encoder_close_container(&images, &image);
}
- g_err |= cbor_encoder_close_container(&rsp, &images);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &images);
if (any_non_bootable) {
split_status = split_check_status();
@@ -302,10 +303,8 @@ imgmgr_state_read(struct mgmt_cbuf *cb)
split_status = SPLIT_STATUS_INVALID;
}
- g_err |= cbor_encode_text_stringz(&rsp, "splitStatus");
- g_err |= cbor_encode_int(&rsp, split_status);
-
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "splitStatus");
+ g_err |= cbor_encode_int(&cb->encoder, split_status);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -341,45 +340,37 @@ imgmgr_state_write(struct mgmt_cbuf *cb)
rc = cbor_read_object(&cb->it, write_attr);
if (rc != 0) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
/* Validate arguments. */
if ((hash_len == 0) && !confirm) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
if (hash_len != 0) {
slot = imgr_find_by_hash(hash, NULL);
if (slot < 0) {
- rc = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
rc = imgmgr_state_set_pending(slot, confirm);
if (rc != 0) {
- goto err;
+ return rc;
}
} else {
/* Confirm current setup. */
rc = imgmgr_state_confirm();
if (rc != 0) {
- goto err;
+ return rc;
}
}
/* Send the current image state in the response. */
rc = imgmgr_state_read(cb);
if (rc != 0) {
- goto err;
+ return rc;
}
return 0;
-
-err:
- mgmt_cbuf_setoerr(cb, rc);
-
- return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/mgmt/newtmgr/nmgr_os/src/newtmgr_os.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/nmgr_os/src/newtmgr_os.c b/mgmt/newtmgr/nmgr_os/src/newtmgr_os.c
index c3e38d6..2a75d47 100644
--- a/mgmt/newtmgr/nmgr_os/src/newtmgr_os.c
+++ b/mgmt/newtmgr/nmgr_os/src/newtmgr_os.c
@@ -84,9 +84,8 @@ static int
nmgr_def_echo(struct mgmt_cbuf *cb)
{
char echo_buf[128] = {'\0'};
- CborEncoder *penc = &cb->encoder;
CborError g_err = CborNoError;
- CborEncoder rsp;
+
struct cbor_attr_t attrs[2] = {
[0] = {
.attribute = "d",
@@ -100,11 +99,9 @@ nmgr_def_echo(struct mgmt_cbuf *cb)
}
};
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "r");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "r");
g_err |= cbor_read_object(&cb->it, attrs);
- g_err |= cbor_encode_text_string(&rsp, echo_buf, strlen(echo_buf));
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encode_text_string(&cb->encoder, echo_buf, strlen(echo_buf));
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -145,19 +142,18 @@ nmgr_def_taskstat_read(struct mgmt_cbuf *cb)
{
struct os_task *prev_task;
struct os_task_info oti;
-
CborError g_err = CborNoError;
- CborEncoder rsp, tasks, task;
+ CborEncoder tasks;
+ CborEncoder task;
- g_err |= cbor_encoder_create_map(&cb->encoder, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "tasks");
- g_err |= cbor_encoder_create_map(&rsp, &tasks, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "tasks");
+ g_err |= cbor_encoder_create_map(&cb->encoder, &tasks,
+ CborIndefiniteLength);
prev_task = NULL;
while (1) {
-
prev_task = os_task_info_get_next(prev_task, &oti);
if (prev_task == NULL) {
break;
@@ -165,28 +161,27 @@ nmgr_def_taskstat_read(struct mgmt_cbuf *cb)
g_err |= cbor_encode_text_stringz(&tasks, oti.oti_name);
g_err |= cbor_encoder_create_map(&tasks, &task, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "prio");
- g_err |= cbor_encode_uint(&rsp, oti.oti_prio);
- g_err |= cbor_encode_text_stringz(&rsp, "tid");
- g_err |= cbor_encode_uint(&rsp, oti.oti_taskid);
- g_err |= cbor_encode_text_stringz(&rsp, "state");
- g_err |= cbor_encode_uint(&rsp, oti.oti_state);
- g_err |= cbor_encode_text_stringz(&rsp, "stkuse");
- g_err |= cbor_encode_uint(&rsp, oti.oti_stkusage);
- g_err |= cbor_encode_text_stringz(&rsp, "stksiz");
- g_err |= cbor_encode_uint(&rsp, oti.oti_stksize);
- g_err |= cbor_encode_text_stringz(&rsp, "cswcnt");
- g_err |= cbor_encode_uint(&rsp, oti.oti_cswcnt);
- g_err |= cbor_encode_text_stringz(&rsp, "runtime");
- g_err |= cbor_encode_uint(&rsp, oti.oti_runtime);
- g_err |= cbor_encode_text_stringz(&rsp, "last_checkin");
- g_err |= cbor_encode_uint(&rsp, oti.oti_last_checkin);
- g_err |= cbor_encode_text_stringz(&rsp, "next_checkin");
- g_err |= cbor_encode_uint(&rsp, oti.oti_next_checkin);
+ g_err |= cbor_encode_text_stringz(&task, "prio");
+ g_err |= cbor_encode_uint(&task, oti.oti_prio);
+ g_err |= cbor_encode_text_stringz(&task, "tid");
+ g_err |= cbor_encode_uint(&task, oti.oti_taskid);
+ g_err |= cbor_encode_text_stringz(&task, "state");
+ g_err |= cbor_encode_uint(&task, oti.oti_state);
+ g_err |= cbor_encode_text_stringz(&task, "stkuse");
+ g_err |= cbor_encode_uint(&task, oti.oti_stkusage);
+ g_err |= cbor_encode_text_stringz(&task, "stksiz");
+ g_err |= cbor_encode_uint(&task, oti.oti_stksize);
+ g_err |= cbor_encode_text_stringz(&task, "cswcnt");
+ g_err |= cbor_encode_uint(&task, oti.oti_cswcnt);
+ g_err |= cbor_encode_text_stringz(&task, "runtime");
+ g_err |= cbor_encode_uint(&task, oti.oti_runtime);
+ g_err |= cbor_encode_text_stringz(&task, "last_checkin");
+ g_err |= cbor_encode_uint(&task, oti.oti_last_checkin);
+ g_err |= cbor_encode_text_stringz(&task, "next_checkin");
+ g_err |= cbor_encode_uint(&task, oti.oti_next_checkin);
g_err |= cbor_encoder_close_container(&tasks, &task);
}
- g_err |= cbor_encoder_close_container(&rsp, &tasks);
- g_err |= cbor_encoder_close_container(&cb->encoder, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &tasks);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -200,13 +195,14 @@ nmgr_def_mpstat_read(struct mgmt_cbuf *cb)
struct os_mempool *prev_mp;
struct os_mempool_info omi;
CborError g_err = CborNoError;
- CborEncoder rsp, pools, pool;
+ CborEncoder pools;
+ CborEncoder pool;
- g_err |= cbor_encoder_create_map(&cb->encoder, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "mpools");
- g_err |= cbor_encoder_create_map(&rsp, &pools, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "mpools");
+ g_err |= cbor_encoder_create_map(&cb->encoder, &pools,
+ CborIndefiniteLength);
prev_mp = NULL;
while (1) {
@@ -217,19 +213,18 @@ nmgr_def_mpstat_read(struct mgmt_cbuf *cb)
g_err |= cbor_encode_text_stringz(&pools, omi.omi_name);
g_err |= cbor_encoder_create_map(&pools, &pool, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "blksiz");
- g_err |= cbor_encode_uint(&rsp, omi.omi_block_size);
- g_err |= cbor_encode_text_stringz(&rsp, "nblks");
- g_err |= cbor_encode_uint(&rsp, omi.omi_num_blocks);
- g_err |= cbor_encode_text_stringz(&rsp, "nfree");
- g_err |= cbor_encode_uint(&rsp, omi.omi_num_free);
- g_err |= cbor_encode_text_stringz(&rsp, "min");
- g_err |= cbor_encode_uint(&rsp, omi.omi_min_free);
+ g_err |= cbor_encode_text_stringz(&pool, "blksiz");
+ g_err |= cbor_encode_uint(&pool, omi.omi_block_size);
+ g_err |= cbor_encode_text_stringz(&pool, "nblks");
+ g_err |= cbor_encode_uint(&pool, omi.omi_num_blocks);
+ g_err |= cbor_encode_text_stringz(&pool, "nfree");
+ g_err |= cbor_encode_uint(&pool, omi.omi_num_free);
+ g_err |= cbor_encode_text_stringz(&pool, "min");
+ g_err |= cbor_encode_uint(&pool, omi.omi_min_free);
g_err |= cbor_encoder_close_container(&pools, &pool);
}
- g_err |= cbor_encoder_close_container(&rsp, &pools);
- g_err |= cbor_encoder_close_container(&cb->encoder, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &pools);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -245,11 +240,9 @@ nmgr_datetime_get(struct mgmt_cbuf *cb)
char buf[DATETIME_BUFSIZE];
int rc;
CborError g_err = CborNoError;
- CborEncoder rsp;
- g_err |= cbor_encoder_create_map(&cb->encoder, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
/* Display the current datetime */
rc = os_gettimeofday(&tv, &tz);
@@ -259,9 +252,8 @@ nmgr_datetime_get(struct mgmt_cbuf *cb)
rc = MGMT_ERR_EINVAL;
goto err;
}
- g_err |= cbor_encode_text_stringz(&rsp, "datetime");
- g_err |= cbor_encode_text_stringz(&rsp, buf);
- g_err |= cbor_encoder_close_container(&cb->encoder, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "datetime");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, buf);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -295,8 +287,7 @@ nmgr_datetime_set(struct mgmt_cbuf *cb)
rc = cbor_read_object(&cb->it, datetime_write_attr);
if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto out;
+ return MGMT_ERR_EINVAL;
}
/* Set the current datetime */
@@ -304,18 +295,18 @@ nmgr_datetime_set(struct mgmt_cbuf *cb)
if (!rc) {
rc = os_settimeofday(&tv, &tz);
if (rc) {
- rc = MGMT_ERR_EINVAL;
- goto out;
+ return MGMT_ERR_EINVAL;
}
} else {
- rc = MGMT_ERR_EINVAL;
- goto out;
+ return MGMT_ERR_EINVAL;
}
- rc = 0;
-out:
- mgmt_cbuf_setoerr(cb, rc);
- return rc;
+ rc = mgmt_cbuf_setoerr(cb, 0);
+ if (rc != 0) {
+ return rc;
+ }
+
+ return 0;
}
static void
@@ -327,6 +318,8 @@ nmgr_reset_tmo(struct os_event *ev)
static int
nmgr_reset(struct mgmt_cbuf *cb)
{
+ int rc;
+
os_callout_init(&nmgr_reset_callout, mgmt_evq_get(), nmgr_reset_tmo, NULL);
#if MYNEWT_VAL(LOG_SOFT_RESET)
@@ -334,7 +327,10 @@ nmgr_reset(struct mgmt_cbuf *cb)
#endif
os_callout_reset(&nmgr_reset_callout, OS_TICKS_PER_SEC / 4);
- mgmt_cbuf_setoerr(cb, OS_OK);
+ rc = mgmt_cbuf_setoerr(cb, 0);
+ if (rc != 0) {
+ return rc;
+ }
return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/mgmt/newtmgr/src/newtmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/src/newtmgr.c b/mgmt/newtmgr/src/newtmgr.c
index 5d37021..043cef6 100644
--- a/mgmt/newtmgr/src/newtmgr.c
+++ b/mgmt/newtmgr/src/newtmgr.c
@@ -95,20 +95,36 @@ nmgr_init_rsp(struct os_mbuf *m, struct nmgr_hdr *src)
static void
nmgr_send_err_rsp(struct nmgr_transport *nt, struct os_mbuf *m,
- struct nmgr_hdr *hdr, int rc)
+ struct nmgr_hdr *hdr, int status)
{
+ struct CborEncoder map;
+ int rc;
+
hdr = nmgr_init_rsp(m, hdr);
if (!hdr) {
os_mbuf_free_chain(m);
return;
}
- mgmt_cbuf_setoerr(&nmgr_task_cbuf.n_b, rc);
- hdr->nh_len +=
- cbor_encode_bytes_written(&nmgr_task_cbuf.n_b.encoder);
+ rc = cbor_encoder_create_map(&nmgr_task_cbuf.n_b.encoder, &map,
+ CborIndefiniteLength);
+ if (rc != 0) {
+ return;
+ }
+
+ rc = mgmt_cbuf_setoerr(&nmgr_task_cbuf.n_b, status);
+ if (rc != 0) {
+ return;
+ }
+
+ rc = cbor_encoder_close_container(&nmgr_task_cbuf.n_b.encoder, &map);
+ if (rc != 0) {
+ return;
+ }
+
+ hdr->nh_len =
+ htons(cbor_encode_bytes_written(&nmgr_task_cbuf.n_b.encoder));
- hdr->nh_len = htons(hdr->nh_len);
- hdr->nh_flags = 0;
nt->nt_output(nt, nmgr_task_cbuf.n_out_m);
}
@@ -172,6 +188,7 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
{
struct os_mbuf *rsp;
const struct mgmt_handler *handler;
+ CborEncoder payload_enc;
struct nmgr_hdr *rsp_hdr;
struct nmgr_hdr hdr;
int off;
@@ -194,7 +211,7 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
mtu = nt->nt_get_mtu(req);
- /* Copy the request packet header into the response. */
+ /* Copy the request user header into the response. */
memcpy(OS_MBUF_USRHDR(rsp), OS_MBUF_USRHDR(req), OS_MBUF_USRHDR_LEN(req));
off = 0;
@@ -228,6 +245,16 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
cbor_parser_init(&nmgr_task_cbuf.reader.r, 0,
&nmgr_task_cbuf.n_b.parser, &nmgr_task_cbuf.n_b.it);
+ /* Begin response payload. Response fields are inserted into the root
+ * map as key value pairs.
+ */
+ rc = cbor_encoder_create_map(&nmgr_task_cbuf.n_b.encoder, &payload_enc,
+ CborIndefiniteLength);
+ if (rc != 0) {
+ rc = MGMT_ERR_ENOMEM;
+ goto err;
+ }
+
if (hdr.nh_op == NMGR_OP_READ) {
if (handler->mh_read) {
rc = handler->mh_read(&nmgr_task_cbuf.n_b);
@@ -243,8 +270,15 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
} else {
rc = MGMT_ERR_EINVAL;
}
+ if (rc != 0) {
+ goto err;
+ }
+ /* End response payload. */
+ rc = cbor_encoder_close_container(&nmgr_task_cbuf.n_b.encoder,
+ &payload_enc);
if (rc != 0) {
+ rc = MGMT_ERR_ENOMEM;
goto err;
}
@@ -270,11 +304,15 @@ nmgr_handle_req(struct nmgr_transport *nt, struct os_mbuf *req)
os_mbuf_free_chain(rsp);
os_mbuf_free_chain(req);
return;
+
err:
- OS_MBUF_PKTHDR(rsp)->omp_len = rsp->om_len = 0;
+ /* Clear partially written response. */
+ os_mbuf_adj(rsp, OS_MBUF_PKTLEN(rsp));
+
nmgr_send_err_rsp(nt, rsp, &hdr, rc);
os_mbuf_free_chain(req);
return;
+
err_norsp:
os_mbuf_free_chain(rsp);
os_mbuf_free_chain(req);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/sys/config/src/config_nmgr.c
----------------------------------------------------------------------
diff --git a/sys/config/src/config_nmgr.c b/sys/config/src/config_nmgr.c
index 24c7080..0b42bc6 100644
--- a/sys/config/src/config_nmgr.c
+++ b/sys/config/src/config_nmgr.c
@@ -49,7 +49,6 @@ conf_nmgr_read(struct mgmt_cbuf *cb)
char val_str[CONF_MAX_VAL_LEN];
char *val;
CborError g_err = CborNoError;
- CborEncoder rsp;
const struct cbor_attr_t attr[2] = {
[0] = {
@@ -73,10 +72,9 @@ conf_nmgr_read(struct mgmt_cbuf *cb)
return MGMT_ERR_EINVAL;
}
- g_err |= cbor_encoder_create_map(&cb->encoder, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "val");
- g_err |= cbor_encode_text_stringz(&rsp, val);
- g_err |= cbor_encoder_close_container(&cb->encoder, &rsp);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "val");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, val);
+
if (g_err) {
return MGMT_ERR_ENOMEM;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/sys/log/full/src/log_nmgr.c
----------------------------------------------------------------------
diff --git a/sys/log/full/src/log_nmgr.c b/sys/log/full/src/log_nmgr.c
index 6d51cb6..00beeba 100644
--- a/sys/log/full/src/log_nmgr.c
+++ b/sys/log/full/src/log_nmgr.c
@@ -250,8 +250,7 @@ log_nmgr_read(struct mgmt_cbuf *cb)
int64_t ts;
uint64_t index;
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, logs;
+ CborEncoder logs;
const struct cbor_attr_t attr[4] = {
[0] = {
@@ -281,10 +280,9 @@ log_nmgr_read(struct mgmt_cbuf *cb)
}
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "logs");
-
- g_err |= cbor_encoder_create_array(&rsp, &logs, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "logs");
+ g_err |= cbor_encoder_create_array(&cb->encoder, &logs,
+ CborIndefiniteLength);
name_len = strlen(name);
log = NULL;
@@ -321,10 +319,9 @@ log_nmgr_read(struct mgmt_cbuf *cb)
}
err:
- g_err |= cbor_encoder_close_container(&rsp, &logs);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, rc);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &logs);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, rc);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -344,15 +341,14 @@ log_nmgr_module_list(struct mgmt_cbuf *cb)
int module;
char *str;
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, modules;
+ CborEncoder modules;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "module_map");
- g_err |= cbor_encoder_create_map(&rsp, &modules, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "module_map");
+ g_err |= cbor_encoder_create_map(&cb->encoder, &modules,
+ CborIndefiniteLength);
module = LOG_MODULE_DEFAULT;
while (module < LOG_MODULE_MAX) {
@@ -367,8 +363,7 @@ log_nmgr_module_list(struct mgmt_cbuf *cb)
module++;
}
- g_err |= cbor_encoder_close_container(&rsp, &modules);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &modules);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -385,16 +380,15 @@ static int
log_nmgr_logs_list(struct mgmt_cbuf *cb)
{
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, log_list;
+ CborEncoder log_list;
struct log *log;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "log_list");
- g_err |= cbor_encoder_create_array(&rsp, &log_list, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "log_list");
+ g_err |= cbor_encoder_create_array(&cb->encoder, &log_list,
+ CborIndefiniteLength);
log = NULL;
while (1) {
@@ -410,8 +404,7 @@ log_nmgr_logs_list(struct mgmt_cbuf *cb)
g_err |= cbor_encode_text_stringz(&log_list, log->l_name);
}
- g_err |= cbor_encoder_close_container(&rsp, &log_list);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &log_list);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -428,17 +421,16 @@ static int
log_nmgr_level_list(struct mgmt_cbuf *cb)
{
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, level_map;
+ CborEncoder level_map;
int level;
char *str;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "level_map");
- g_err |= cbor_encoder_create_map(&rsp, &level_map, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "level_map");
+ g_err |= cbor_encoder_create_map(&cb->encoder, &level_map,
+ CborIndefiniteLength);
level = LOG_LEVEL_DEBUG;
while (level < LOG_LEVEL_MAX) {
@@ -453,8 +445,7 @@ log_nmgr_level_list(struct mgmt_cbuf *cb)
level++;
}
- g_err |= cbor_encoder_close_container(&rsp, &level_map);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &level_map);
if (g_err) {
return MGMT_ERR_ENOMEM;
@@ -470,9 +461,6 @@ log_nmgr_level_list(struct mgmt_cbuf *cb)
static int
log_nmgr_clear(struct mgmt_cbuf *cb)
{
- CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp;
struct log *log;
int rc;
@@ -489,19 +477,16 @@ log_nmgr_clear(struct mgmt_cbuf *cb)
rc = log_flush(log);
if (rc) {
- goto err;
+ return rc;
}
}
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encoder_close_container(penc, &rsp);
- if (g_err) {
- return MGMT_ERR_ENOMEM;
+ rc = mgmt_cbuf_setoerr(cb, 0);
+ if (rc != 0) {
+ return rc;
}
+
return 0;
-err:
- mgmt_cbuf_setoerr(cb, rc);
- return (rc);
}
/**
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/sys/stats/full/src/stats_nmgr.c
----------------------------------------------------------------------
diff --git a/sys/stats/full/src/stats_nmgr.c b/sys/stats/full/src/stats_nmgr.c
index 5df6e7c..0b28b25 100644
--- a/sys/stats/full/src/stats_nmgr.c
+++ b/sys/stats/full/src/stats_nmgr.c
@@ -95,46 +95,39 @@ stats_nmgr_read(struct mgmt_cbuf *cb)
{ NULL },
};
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, stats;
+ CborEncoder stats;
g_err = cbor_read_object(&cb->it, attrs);
if (g_err != 0) {
- g_err = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
hdr = stats_group_find(stats_name);
if (!hdr) {
- g_err = MGMT_ERR_EINVAL;
- goto err;
+ return MGMT_ERR_EINVAL;
}
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "name");
- g_err |= cbor_encode_text_stringz(&rsp, stats_name);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "name");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, stats_name);
- g_err |= cbor_encode_text_stringz(&rsp, "group");
- g_err |= cbor_encode_text_string(&rsp, "sys", sizeof("sys")-1);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "group");
+ g_err |= cbor_encode_text_string(&cb->encoder, "sys", sizeof("sys")-1);
- g_err |= cbor_encode_text_stringz(&rsp, "fields");
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "fields");
- g_err |= cbor_encoder_create_map(&rsp, &stats, CborIndefiniteLength);
+ g_err |= cbor_encoder_create_map(&cb->encoder, &stats,
+ CborIndefiniteLength);
stats_walk(hdr, stats_nmgr_walk_func, &stats);
- g_err |= cbor_encoder_close_container(&rsp, &stats);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &stats);
if (g_err) {
return MGMT_ERR_ENOMEM;
}
- return (0);
-err:
- mgmt_cbuf_setoerr(cb, g_err);
return (0);
}
@@ -143,17 +136,15 @@ static int
stats_nmgr_list(struct mgmt_cbuf *cb)
{
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, stats;
-
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "stat_list");
- g_err |= cbor_encoder_create_array(&rsp, &stats, CborIndefiniteLength);
+ CborEncoder stats;
+
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "stat_list");
+ g_err |= cbor_encoder_create_array(&cb->encoder, &stats,
+ CborIndefiniteLength);
stats_group_walk(stats_nmgr_encode_name, &stats);
- g_err |= cbor_encoder_close_container(&rsp, &stats);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &stats);
if (g_err) {
return MGMT_ERR_ENOMEM;
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/test/crash_test/src/crash_nmgr.c
----------------------------------------------------------------------
diff --git a/test/crash_test/src/crash_nmgr.c b/test/crash_test/src/crash_nmgr.c
index 7ffd969..b1b8eaa 100644
--- a/test/crash_test/src/crash_nmgr.c
+++ b/test/crash_test/src/crash_nmgr.c
@@ -33,7 +33,7 @@
static int crash_test_nmgr_write(struct mgmt_cbuf *);
static const struct mgmt_handler crash_test_nmgr_handler[] = {
- [0] = { crash_test_nmgr_write, crash_test_nmgr_write }
+ [0] = { NULL, crash_test_nmgr_write }
};
struct mgmt_group crash_test_nmgr_group = {
@@ -60,15 +60,20 @@ crash_test_nmgr_write(struct mgmt_cbuf *cb)
int rc;
rc = cbor_read_object(&cb->it, attr);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
- } else {
- rc = crash_device(tmp_str);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
- }
+ if (rc != 0) {
+ return MGMT_ERR_EINVAL;
+ }
+
+ rc = crash_device(tmp_str);
+ if (rc != 0) {
+ return MGMT_ERR_EINVAL;
}
- mgmt_cbuf_setoerr(cb, rc);
+
+ rc = mgmt_cbuf_setoerr(cb, 0);
+ if (rc != 0) {
+ return rc;
+ }
+
return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/38284705/test/runtest/src/runtest_nmgr.c
----------------------------------------------------------------------
diff --git a/test/runtest/src/runtest_nmgr.c b/test/runtest/src/runtest_nmgr.c
index 3c02ee3..ce282ce 100644
--- a/test/runtest/src/runtest_nmgr.c
+++ b/test/runtest/src/runtest_nmgr.c
@@ -105,11 +105,6 @@ run_nmgr_test(struct mgmt_cbuf *cb)
os_eventq_put(run_evq_get(), &run_test_event);
- if (rc) {
- rc = MGMT_ERR_EINVAL;
- }
-
- mgmt_cbuf_setoerr(cb, rc);
return 0;
}
@@ -120,23 +115,21 @@ static int
run_nmgr_list(struct mgmt_cbuf *cb)
{
CborError g_err = CborNoError;
- CborEncoder *penc = &cb->encoder;
- CborEncoder rsp, run_list;
+ CborEncoder run_list;
struct ts_suite *ts;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, MGMT_ERR_EOK);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ g_err |= cbor_encode_int(&cb->encoder, MGMT_ERR_EOK);
- g_err |= cbor_encode_text_stringz(&rsp, "run_list");
- g_err |= cbor_encoder_create_array(&rsp, &run_list, CborIndefiniteLength);
+ g_err |= cbor_encode_text_stringz(&cb->encoder, "run_list");
+ g_err |= cbor_encoder_create_array(&cb->encoder, &run_list,
+ CborIndefiniteLength);
SLIST_FOREACH(ts, &g_ts_suites, ts_next) {
g_err |= cbor_encode_text_stringz(&run_list, ts->ts_name);
}
- g_err |= cbor_encoder_close_container(&rsp, &run_list);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ g_err |= cbor_encoder_close_container(&cb->encoder, &run_list);
if (g_err) {
return MGMT_ERR_ENOMEM;
Re: [2/2] incubator-mynewt-core git commit: MYNEWT-647 Changes to NMP over OIC scheme
Posted by Vipul Rahane <vi...@runtime.io>.
+1, I like the simplification and the idea of NMP header being in one place. I had a few doubts which I clarified with Chris.
Regards,
Vipul Rahane
> On Mar 2, 2017, at 8:17 AM, ccollins@apache.org wrote:
>
> MYNEWT-647 Changes to NMP over OIC scheme
>
> This commit addresses the following three issues with NMP over OIC (OMP):
>
> 1. Parts of NMP header missing from OMP responses. This causes problems
> when multiplexing OMP among many target devices.
>
> 2. Parts of the NMP header are scattered in a few different places.
> This works, but it makes it difficult to debug packet traces.
>
> 3. NMP errors get lost in translation. Instead of an NMP message with
> an error code, the OMP server sends back a generic "Bad Request" OIC
> message.
>
> ***** Current Scheme
>
> *** Requests
>
> The NMP op is indicated by the OIC op:
> NMP read <=> OIC get
> NMP write <=> OIC put
>
> The NMP group and ID are indicated as CoAP URI Query options:
>
> gr=<group>
> id=<id>
>
> The remaining NMP header fields, seq and flags, are not present.
>
> The NMP payload is the entire CoAP request body. This is identical to the body
> of a plain NMP request.
>
> *** Responses
>
> The NMP header is not present. The NMP op is indicated in the payload (see
> below), but other header fields cannot be inferred.
>
> Payload consists of a single CBOR key-value pair. For read responses, the key
> is "r"; for write responses, the key is "w". The value is a second CBOR map
> containing the actual NMP response fields.
>
> Errors encountered during processing of NMP requests are reported via OIC error
> responses (bad request, internal server error).
>
> ***** Proposed Scheme
>
> *** Requests
>
> * The OIC op is always the same: put.
>
> * No URI Query CoAP options.
>
> * The NMP header is included in the payload as a key-value pair (key="_h").
> This pair is in the root map of the request and is a sibling of the other
> request fields. The value of this pair is the big-endian eight-byte NMP
> header with a length field of 0.
>
> *** Responses
>
> * As with requests, the NMP header is included in the payload as a
> key-value pair (key="_h").
>
> * No "r" or "w" field. The response fields are inserted into the root map
> as a sibling of the "_h" pair.
>
> * Errors encountered during processing of NMP requests are reported
> identically to other NMP responses (embedded NMP response).
>
> *** Notes
>
> * Keys that start with an underscore are reserved to the OIC manager
> protocol (e.g., "_h"). NMP requests and responses must not name any of
> their fields with a leading underscore.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
> Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/860d2d26
> Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/860d2d26
> Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/860d2d26
>
> Branch: refs/heads/develop
> Commit: 860d2d266110918b4a6f923f1e18c9032d13c07d
> Parents: a4df93c
> Author: Christopher Collins <cc...@apache.org>
> Authored: Wed Mar 1 17:41:21 2017 -0800
> Committer: Christopher Collins <cc...@apache.org>
> Committed: Thu Mar 2 08:15:05 2017 -0800
>
> ----------------------------------------------------------------------
> mgmt/mgmt/include/mgmt/mgmt.h | 24 ++-
> mgmt/mgmt/src/mgmt.c | 22 ++-
> mgmt/newtmgr/include/newtmgr/newtmgr.h | 21 ---
> mgmt/oicmgr/src/oicmgr.c | 238 ++++++++++++++++++----------
> 4 files changed, 192 insertions(+), 113 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/include/mgmt/mgmt.h
> ----------------------------------------------------------------------
> diff --git a/mgmt/mgmt/include/mgmt/mgmt.h b/mgmt/mgmt/include/mgmt/mgmt.h
> index 9b79723..3c402f2 100644
> --- a/mgmt/mgmt/include/mgmt/mgmt.h
> +++ b/mgmt/mgmt/include/mgmt/mgmt.h
> @@ -36,6 +36,10 @@ extern "C" {
> #define STR(x) #x
> #endif
>
> +#define NMGR_OP_READ (0)
> +#define NMGR_OP_READ_RSP (1)
> +#define NMGR_OP_WRITE (2)
> +#define NMGR_OP_WRITE_RSP (3)
>
> /* First 64 groups are reserved for system level newtmgr commands.
> * Per-user commands are then defined after group 64.
> @@ -63,6 +67,24 @@ extern "C" {
> #define MGMT_ERR_EBADSTATE (6) /* Current state disallows command. */
> #define MGMT_ERR_EPERUSER (256)
>
> +#define NMGR_HDR_SIZE (8)
> +
> +struct nmgr_hdr {
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> + uint8_t nh_op:3; /* NMGR_OP_XXX */
> + uint8_t _res1:5;
> +#endif
> +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> + uint8_t _res1:5;
> + uint8_t nh_op:3; /* NMGR_OP_XXX */
> +#endif
> + uint8_t nh_flags; /* XXX reserved for future flags */
> + uint16_t nh_len; /* length of the payload */
> + uint16_t nh_group; /* NMGR_GROUP_XXX */
> + uint8_t nh_seq; /* sequence number */
> + uint8_t nh_id; /* message ID within group */
> +};
> +
> struct mgmt_cbuf;
>
> typedef int (*mgmt_handler_func_t)(struct mgmt_cbuf *);
> @@ -85,7 +107,7 @@ struct mgmt_group {
> sizeof(struct mgmt_handler));
>
> int mgmt_group_register(struct mgmt_group *group);
> -void mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
> +int mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
> const struct mgmt_handler *mgmt_find_handler(uint16_t group_id,
> uint16_t handler_id);
>
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/src/mgmt.c
> ----------------------------------------------------------------------
> diff --git a/mgmt/mgmt/src/mgmt.c b/mgmt/mgmt/src/mgmt.c
> index 4ac232a..c1a2a1e 100644
> --- a/mgmt/mgmt/src/mgmt.c
> +++ b/mgmt/mgmt/src/mgmt.c
> @@ -137,14 +137,20 @@ err:
> return (NULL);
> }
>
> -void
> +int
> mgmt_cbuf_setoerr(struct mgmt_cbuf *cb, int errcode)
> {
> - CborEncoder *penc = &cb->encoder;
> - CborError g_err = CborNoError;
> - CborEncoder rsp;
> - g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
> - g_err |= cbor_encode_text_stringz(&rsp, "rc");
> - g_err |= cbor_encode_int(&rsp, errcode);
> - g_err |= cbor_encoder_close_container(penc, &rsp);
> + int rc;
> +
> + rc = cbor_encode_text_stringz(&cb->encoder, "rc");
> + if (rc != 0) {
> + return rc;
> + }
> +
> + rc = cbor_encode_int(&cb->encoder, errcode);
> + if (rc != 0) {
> + return rc;
> + }
> +
> + return 0;
> }
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/newtmgr/include/newtmgr/newtmgr.h
> ----------------------------------------------------------------------
> diff --git a/mgmt/newtmgr/include/newtmgr/newtmgr.h b/mgmt/newtmgr/include/newtmgr/newtmgr.h
> index 50deb15..1cf314d 100644
> --- a/mgmt/newtmgr/include/newtmgr/newtmgr.h
> +++ b/mgmt/newtmgr/include/newtmgr/newtmgr.h
> @@ -29,27 +29,6 @@
> extern "C" {
> #endif
>
> -#define NMGR_OP_READ (0)
> -#define NMGR_OP_READ_RSP (1)
> -#define NMGR_OP_WRITE (2)
> -#define NMGR_OP_WRITE_RSP (3)
> -
> -struct nmgr_hdr {
> -#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> - uint8_t nh_op:3; /* NMGR_OP_XXX */
> - uint8_t _res1:5;
> -#endif
> -#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> - uint8_t _res1:5;
> - uint8_t nh_op:3; /* NMGR_OP_XXX */
> -#endif
> - uint8_t nh_flags; /* XXX reserved for future flags */
> - uint16_t nh_len; /* length of the payload */
> - uint16_t nh_group; /* NMGR_GROUP_XXX */
> - uint8_t nh_seq; /* sequence number */
> - uint8_t nh_id; /* message ID within group */
> -};
> -
> struct nmgr_transport;
>
> /**
>
> http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/oicmgr/src/oicmgr.c
> ----------------------------------------------------------------------
> diff --git a/mgmt/oicmgr/src/oicmgr.c b/mgmt/oicmgr/src/oicmgr.c
> index d2110ef..d7d3591 100644
> --- a/mgmt/oicmgr/src/oicmgr.c
> +++ b/mgmt/oicmgr/src/oicmgr.c
> @@ -22,6 +22,7 @@
>
> #include <os/os.h>
> #include <os/endian.h>
> +#include <defs/error.h>
>
> #include <assert.h>
> #include <string.h>
> @@ -52,9 +53,6 @@ static struct omgr_state omgr_state = {
> .os_event.ev_cb = omgr_event_start,
> };
>
> -static void omgr_oic_get(oc_request_t *request, oc_interface_mask_t interface);
> -static void omgr_oic_put(oc_request_t *request, oc_interface_mask_t interface);
> -
> struct os_eventq *
> mgmt_evq_get(void)
> {
> @@ -67,124 +65,199 @@ mgmt_evq_set(struct os_eventq *evq)
> os_eventq_designate(&omgr_state.os_evq, evq, &omgr_state.os_event);
> }
>
> -static const struct mgmt_handler *
> -omgr_find_handler(const char *q, int qlen)
> +static int
> +omgr_oic_read_hdr(struct CborValue *cv, struct nmgr_hdr *out_hdr)
> {
> - char id_str[8];
> - int grp = -1;
> - int id = -1;
> - char *str;
> - char *eptr;
> - int slen;
> -
> - slen = oc_ri_get_query_value(q, qlen, "gr", &str);
> - if (slen > 0 && slen < sizeof(id_str) - 1) {
> - memcpy(id_str, str, slen);
> - id_str[slen] = '\0';
> - grp = strtoul(id_str, &eptr, 0);
> - if (*eptr != '\0') {
> - return NULL;
> - }
> + size_t hlen;
> + int rc;
> +
> + struct cbor_attr_t attrs[] = {
> + [0] = {
> + .attribute = "_h",
> + .type = CborAttrByteStringType,
> + .addr.bytestring.data = (void *)out_hdr,
> + .addr.bytestring.len = &hlen,
> + .nodefault = 1,
> + .len = sizeof *out_hdr,
> + },
> + [1] = { 0 }
> + };
> +
> + rc = cbor_read_object(cv, attrs);
> + if (rc != 0 || hlen != sizeof *out_hdr) {
> + return MGMT_ERR_EINVAL;
> }
> - slen = oc_ri_get_query_value(q, qlen, "id", &str);
> - if (slen > 0 && slen < sizeof(id_str) - 1) {
> - memcpy(id_str, str, slen);
> - id_str[slen] = '\0';
> - id = strtoul(id_str, &eptr, 0);
> - if (*eptr != '\0') {
> - return NULL;
> - }
> +
> + out_hdr->nh_len = ntohs(out_hdr->nh_len);
> + out_hdr->nh_group = ntohs(out_hdr->nh_group);
> +
> + return 0;
> +}
> +
> +static int
> +omgr_encode_nmp_hdr(struct CborEncoder *enc, struct nmgr_hdr hdr)
> +{
> + int rc;
> +
> + rc = cbor_encode_text_string(enc, "_h", 2);
> + if (rc != 0) {
> + return MGMT_ERR_ENOMEM;
> + }
> +
> + hdr.nh_len = htons(hdr.nh_len);
> + hdr.nh_group = htons(hdr.nh_group);
> +
> + /* Encode the NMP header in the response. */
> + rc = cbor_encode_byte_string(enc, (void *)&hdr, sizeof hdr);
> + if (rc != 0) {
> + return MGMT_ERR_ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int
> +omgr_send_err_rsp(struct CborEncoder *enc, const struct nmgr_hdr *hdr,
> + int nmp_status)
> +{
> + int rc;
> +
> + rc = omgr_encode_nmp_hdr(enc, *hdr);
> + if (rc != 0) {
> + return rc;
> + }
> +
> + rc = cbor_encode_text_stringz(enc, "rc");
> + if (rc != 0) {
> + return MGMT_ERR_ENOMEM;
> + }
> +
> + rc = cbor_encode_int(enc, nmp_status);
> + if (rc != 0) {
> + return MGMT_ERR_ENOMEM;
> }
> - return mgmt_find_handler(grp, id);
> +
> + return 0;
> }
>
> static void
> -omgr_oic_op(oc_request_t *req, oc_interface_mask_t mask, int isset)
> +omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
> {
> struct omgr_state *o = &omgr_state;
> const struct mgmt_handler *handler;
> uint16_t data_off;
> struct os_mbuf *m;
> int rc = 0;
> - extern CborEncoder g_encoder;
> + struct nmgr_hdr hdr;
> + int rsp_hdr_filled = 0;
>
> - if (!req->query_len) {
> - goto bad_req;
> - }
> -
> - handler = omgr_find_handler(req->query, req->query_len);
> - if (!handler) {
> - goto bad_req;
> - }
> + coap_get_payload(req->packet, &m, &data_off);
>
> - rc = coap_get_payload(req->packet, &m, &data_off);
> cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
> -
> cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
> &o->os_cbuf.ob_mj.it);
>
> - /* start generating the CBOR OUTPUT */
> - /* this is worth a quick note. We are encoding CBOR within CBOR, so we
> - * need to use the same encoder as ocf stack. We are using their global
> - * encoder g_encoder which they intialized before this function is called.
> - * But we can't call their macros here as it won't use the right mape
> - * encoder (ob_mj) */
> + rc = omgr_oic_read_hdr(&o->os_cbuf.ob_mj.it, &hdr);
> + if (rc != 0) {
> + rc = MGMT_ERR_EINVAL;
> + goto done;
> + }
> +
> + /* Convert request header to response header to be sent. */
> + switch (hdr.nh_op) {
> + case NMGR_OP_READ:
> + hdr.nh_op = NMGR_OP_READ_RSP;
> + break;
> +
> + case NMGR_OP_WRITE:
> + hdr.nh_op = NMGR_OP_WRITE_RSP;
> + break;
> +
> + default:
> + goto done;
> + }
> + rsp_hdr_filled = 1;
> +
> + /* Begin root map in response. */
> cbor_encoder_create_map(&g_encoder, &o->os_cbuf.ob_mj.encoder,
> CborIndefiniteLength);
>
> + handler = mgmt_find_handler(hdr.nh_group, hdr.nh_id);
> + if (handler == NULL) {
> + rc = MGMT_ERR_ENOENT;
> + goto done;
> + }
> +
> + cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
> + cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
> + &o->os_cbuf.ob_mj.it);
> +
> switch (mask) {
> case OC_IF_BASELINE:
> oc_process_baseline_interface(req->resource);
> + /* Fallthrough */
> +
> case OC_IF_RW:
> - if (!isset) {
> - cbor_encode_text_string(&root_map, "r", 1);
> - if (handler->mh_read) {
> - rc = handler->mh_read(&o->os_cbuf.ob_mj);
> + switch (hdr.nh_op) {
> + case NMGR_OP_READ_RSP:
> + if (handler->mh_read == NULL) {
> + rc = MGMT_ERR_ENOENT;
> } else {
> - goto bad_req;
> + rc = handler->mh_read(&o->os_cbuf.ob_mj);
> }
> - } else {
> - cbor_encode_text_string(&root_map, "w", 1);
> - if (handler->mh_write) {
> - rc = handler->mh_write(&o->os_cbuf.ob_mj);
> + break;
> +
> + case NMGR_OP_WRITE_RSP:
> + if (handler->mh_write == NULL) {
> + rc = MGMT_ERR_ENOENT;
> } else {
> - goto bad_req;
> + rc = handler->mh_write(&o->os_cbuf.ob_mj);
> }
> + break;
> +
> + default:
> + rc = MGMT_ERR_EINVAL;
> + break;
> }
> - if (rc) {
> - goto bad_req;
> + if (rc != 0) {
> + goto done;
> + }
> +
> + /* Encode the NMP header in the response. */
> + rc = omgr_encode_nmp_hdr(&o->os_cbuf.ob_mj.encoder, hdr);
> + if (rc != 0) {
> + rc = MGMT_ERR_ENOMEM;
> + goto done;
> }
> break;
> +
> default:
> break;
> }
>
> - cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
> - oc_send_response(req, OC_STATUS_OK);
> + rc = 0;
>
> - return;
> -bad_req:
> - /*
> - * XXXX might send partially constructed response as payload
> - */
> - if (rc == MGMT_ERR_ENOMEM) {
> - rc = OC_STATUS_INTERNAL_SERVER_ERROR;
> - } else {
> - rc = OC_STATUS_BAD_REQUEST;
> - }
> - oc_send_response(req, rc);
> -}
> +done:
> + if (rc != 0) {
> + if (rsp_hdr_filled) {
> + rc = omgr_send_err_rsp(&g_encoder, &hdr, rc);
> + }
> + switch (rc) {
> + case 0:
> + break;
>
> -static void
> -omgr_oic_get(oc_request_t *req, oc_interface_mask_t mask)
> -{
> - omgr_oic_op(req, mask, 0);
> -}
> + case MGMT_ERR_ENOMEM:
> + rc = OC_STATUS_INTERNAL_SERVER_ERROR;
> + break;
>
> -static void
> -omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
> -{
> - omgr_oic_op(req, mask, 1);
> + default:
> + rc = OC_STATUS_BAD_REQUEST;
> + break;
> + }
> + }
> +
> + cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
> + oc_send_response(req, rc);
> }
>
> static void
> @@ -204,7 +277,6 @@ omgr_event_start(struct os_event *ev)
> oc_resource_bind_resource_interface(res, mode);
> oc_resource_set_default_interface(res, mode);
> oc_resource_set_discoverable(res);
> - oc_resource_set_request_handler(res, OC_GET, omgr_oic_get);
> oc_resource_set_request_handler(res, OC_PUT, omgr_oic_put);
> oc_add_resource(res);
> }
>
[2/2] incubator-mynewt-core git commit: MYNEWT-647 Changes to NMP
over OIC scheme
Posted by cc...@apache.org.
MYNEWT-647 Changes to NMP over OIC scheme
This commit addresses the following three issues with NMP over OIC (OMP):
1. Parts of NMP header missing from OMP responses. This causes problems
when multiplexing OMP among many target devices.
2. Parts of the NMP header are scattered in a few different places.
This works, but it makes it difficult to debug packet traces.
3. NMP errors get lost in translation. Instead of an NMP message with
an error code, the OMP server sends back a generic "Bad Request" OIC
message.
***** Current Scheme
*** Requests
The NMP op is indicated by the OIC op:
NMP read <=> OIC get
NMP write <=> OIC put
The NMP group and ID are indicated as CoAP URI Query options:
gr=<group>
id=<id>
The remaining NMP header fields, seq and flags, are not present.
The NMP payload is the entire CoAP request body. This is identical to the body
of a plain NMP request.
*** Responses
The NMP header is not present. The NMP op is indicated in the payload (see
below), but other header fields cannot be inferred.
Payload consists of a single CBOR key-value pair. For read responses, the key
is "r"; for write responses, the key is "w". The value is a second CBOR map
containing the actual NMP response fields.
Errors encountered during processing of NMP requests are reported via OIC error
responses (bad request, internal server error).
***** Proposed Scheme
*** Requests
* The OIC op is always the same: put.
* No URI Query CoAP options.
* The NMP header is included in the payload as a key-value pair (key="_h").
This pair is in the root map of the request and is a sibling of the other
request fields. The value of this pair is the big-endian eight-byte NMP
header with a length field of 0.
*** Responses
* As with requests, the NMP header is included in the payload as a
key-value pair (key="_h").
* No "r" or "w" field. The response fields are inserted into the root map
as a sibling of the "_h" pair.
* Errors encountered during processing of NMP requests are reported
identically to other NMP responses (embedded NMP response).
*** Notes
* Keys that start with an underscore are reserved to the OIC manager
protocol (e.g., "_h"). NMP requests and responses must not name any of
their fields with a leading underscore.
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/860d2d26
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/860d2d26
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/860d2d26
Branch: refs/heads/develop
Commit: 860d2d266110918b4a6f923f1e18c9032d13c07d
Parents: a4df93c
Author: Christopher Collins <cc...@apache.org>
Authored: Wed Mar 1 17:41:21 2017 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Thu Mar 2 08:15:05 2017 -0800
----------------------------------------------------------------------
mgmt/mgmt/include/mgmt/mgmt.h | 24 ++-
mgmt/mgmt/src/mgmt.c | 22 ++-
mgmt/newtmgr/include/newtmgr/newtmgr.h | 21 ---
mgmt/oicmgr/src/oicmgr.c | 238 ++++++++++++++++++----------
4 files changed, 192 insertions(+), 113 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/include/mgmt/mgmt.h
----------------------------------------------------------------------
diff --git a/mgmt/mgmt/include/mgmt/mgmt.h b/mgmt/mgmt/include/mgmt/mgmt.h
index 9b79723..3c402f2 100644
--- a/mgmt/mgmt/include/mgmt/mgmt.h
+++ b/mgmt/mgmt/include/mgmt/mgmt.h
@@ -36,6 +36,10 @@ extern "C" {
#define STR(x) #x
#endif
+#define NMGR_OP_READ (0)
+#define NMGR_OP_READ_RSP (1)
+#define NMGR_OP_WRITE (2)
+#define NMGR_OP_WRITE_RSP (3)
/* First 64 groups are reserved for system level newtmgr commands.
* Per-user commands are then defined after group 64.
@@ -63,6 +67,24 @@ extern "C" {
#define MGMT_ERR_EBADSTATE (6) /* Current state disallows command. */
#define MGMT_ERR_EPERUSER (256)
+#define NMGR_HDR_SIZE (8)
+
+struct nmgr_hdr {
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ uint8_t nh_op:3; /* NMGR_OP_XXX */
+ uint8_t _res1:5;
+#endif
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+ uint8_t _res1:5;
+ uint8_t nh_op:3; /* NMGR_OP_XXX */
+#endif
+ uint8_t nh_flags; /* XXX reserved for future flags */
+ uint16_t nh_len; /* length of the payload */
+ uint16_t nh_group; /* NMGR_GROUP_XXX */
+ uint8_t nh_seq; /* sequence number */
+ uint8_t nh_id; /* message ID within group */
+};
+
struct mgmt_cbuf;
typedef int (*mgmt_handler_func_t)(struct mgmt_cbuf *);
@@ -85,7 +107,7 @@ struct mgmt_group {
sizeof(struct mgmt_handler));
int mgmt_group_register(struct mgmt_group *group);
-void mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
+int mgmt_cbuf_setoerr(struct mgmt_cbuf *njb, int errcode);
const struct mgmt_handler *mgmt_find_handler(uint16_t group_id,
uint16_t handler_id);
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/mgmt/src/mgmt.c
----------------------------------------------------------------------
diff --git a/mgmt/mgmt/src/mgmt.c b/mgmt/mgmt/src/mgmt.c
index 4ac232a..c1a2a1e 100644
--- a/mgmt/mgmt/src/mgmt.c
+++ b/mgmt/mgmt/src/mgmt.c
@@ -137,14 +137,20 @@ err:
return (NULL);
}
-void
+int
mgmt_cbuf_setoerr(struct mgmt_cbuf *cb, int errcode)
{
- CborEncoder *penc = &cb->encoder;
- CborError g_err = CborNoError;
- CborEncoder rsp;
- g_err |= cbor_encoder_create_map(penc, &rsp, CborIndefiniteLength);
- g_err |= cbor_encode_text_stringz(&rsp, "rc");
- g_err |= cbor_encode_int(&rsp, errcode);
- g_err |= cbor_encoder_close_container(penc, &rsp);
+ int rc;
+
+ rc = cbor_encode_text_stringz(&cb->encoder, "rc");
+ if (rc != 0) {
+ return rc;
+ }
+
+ rc = cbor_encode_int(&cb->encoder, errcode);
+ if (rc != 0) {
+ return rc;
+ }
+
+ return 0;
}
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/newtmgr/include/newtmgr/newtmgr.h
----------------------------------------------------------------------
diff --git a/mgmt/newtmgr/include/newtmgr/newtmgr.h b/mgmt/newtmgr/include/newtmgr/newtmgr.h
index 50deb15..1cf314d 100644
--- a/mgmt/newtmgr/include/newtmgr/newtmgr.h
+++ b/mgmt/newtmgr/include/newtmgr/newtmgr.h
@@ -29,27 +29,6 @@
extern "C" {
#endif
-#define NMGR_OP_READ (0)
-#define NMGR_OP_READ_RSP (1)
-#define NMGR_OP_WRITE (2)
-#define NMGR_OP_WRITE_RSP (3)
-
-struct nmgr_hdr {
-#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
- uint8_t nh_op:3; /* NMGR_OP_XXX */
- uint8_t _res1:5;
-#endif
-#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
- uint8_t _res1:5;
- uint8_t nh_op:3; /* NMGR_OP_XXX */
-#endif
- uint8_t nh_flags; /* XXX reserved for future flags */
- uint16_t nh_len; /* length of the payload */
- uint16_t nh_group; /* NMGR_GROUP_XXX */
- uint8_t nh_seq; /* sequence number */
- uint8_t nh_id; /* message ID within group */
-};
-
struct nmgr_transport;
/**
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/860d2d26/mgmt/oicmgr/src/oicmgr.c
----------------------------------------------------------------------
diff --git a/mgmt/oicmgr/src/oicmgr.c b/mgmt/oicmgr/src/oicmgr.c
index d2110ef..d7d3591 100644
--- a/mgmt/oicmgr/src/oicmgr.c
+++ b/mgmt/oicmgr/src/oicmgr.c
@@ -22,6 +22,7 @@
#include <os/os.h>
#include <os/endian.h>
+#include <defs/error.h>
#include <assert.h>
#include <string.h>
@@ -52,9 +53,6 @@ static struct omgr_state omgr_state = {
.os_event.ev_cb = omgr_event_start,
};
-static void omgr_oic_get(oc_request_t *request, oc_interface_mask_t interface);
-static void omgr_oic_put(oc_request_t *request, oc_interface_mask_t interface);
-
struct os_eventq *
mgmt_evq_get(void)
{
@@ -67,124 +65,199 @@ mgmt_evq_set(struct os_eventq *evq)
os_eventq_designate(&omgr_state.os_evq, evq, &omgr_state.os_event);
}
-static const struct mgmt_handler *
-omgr_find_handler(const char *q, int qlen)
+static int
+omgr_oic_read_hdr(struct CborValue *cv, struct nmgr_hdr *out_hdr)
{
- char id_str[8];
- int grp = -1;
- int id = -1;
- char *str;
- char *eptr;
- int slen;
-
- slen = oc_ri_get_query_value(q, qlen, "gr", &str);
- if (slen > 0 && slen < sizeof(id_str) - 1) {
- memcpy(id_str, str, slen);
- id_str[slen] = '\0';
- grp = strtoul(id_str, &eptr, 0);
- if (*eptr != '\0') {
- return NULL;
- }
+ size_t hlen;
+ int rc;
+
+ struct cbor_attr_t attrs[] = {
+ [0] = {
+ .attribute = "_h",
+ .type = CborAttrByteStringType,
+ .addr.bytestring.data = (void *)out_hdr,
+ .addr.bytestring.len = &hlen,
+ .nodefault = 1,
+ .len = sizeof *out_hdr,
+ },
+ [1] = { 0 }
+ };
+
+ rc = cbor_read_object(cv, attrs);
+ if (rc != 0 || hlen != sizeof *out_hdr) {
+ return MGMT_ERR_EINVAL;
}
- slen = oc_ri_get_query_value(q, qlen, "id", &str);
- if (slen > 0 && slen < sizeof(id_str) - 1) {
- memcpy(id_str, str, slen);
- id_str[slen] = '\0';
- id = strtoul(id_str, &eptr, 0);
- if (*eptr != '\0') {
- return NULL;
- }
+
+ out_hdr->nh_len = ntohs(out_hdr->nh_len);
+ out_hdr->nh_group = ntohs(out_hdr->nh_group);
+
+ return 0;
+}
+
+static int
+omgr_encode_nmp_hdr(struct CborEncoder *enc, struct nmgr_hdr hdr)
+{
+ int rc;
+
+ rc = cbor_encode_text_string(enc, "_h", 2);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ hdr.nh_len = htons(hdr.nh_len);
+ hdr.nh_group = htons(hdr.nh_group);
+
+ /* Encode the NMP header in the response. */
+ rc = cbor_encode_byte_string(enc, (void *)&hdr, sizeof hdr);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ return 0;
+}
+
+static int
+omgr_send_err_rsp(struct CborEncoder *enc, const struct nmgr_hdr *hdr,
+ int nmp_status)
+{
+ int rc;
+
+ rc = omgr_encode_nmp_hdr(enc, *hdr);
+ if (rc != 0) {
+ return rc;
+ }
+
+ rc = cbor_encode_text_stringz(enc, "rc");
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ rc = cbor_encode_int(enc, nmp_status);
+ if (rc != 0) {
+ return MGMT_ERR_ENOMEM;
}
- return mgmt_find_handler(grp, id);
+
+ return 0;
}
static void
-omgr_oic_op(oc_request_t *req, oc_interface_mask_t mask, int isset)
+omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
{
struct omgr_state *o = &omgr_state;
const struct mgmt_handler *handler;
uint16_t data_off;
struct os_mbuf *m;
int rc = 0;
- extern CborEncoder g_encoder;
+ struct nmgr_hdr hdr;
+ int rsp_hdr_filled = 0;
- if (!req->query_len) {
- goto bad_req;
- }
-
- handler = omgr_find_handler(req->query, req->query_len);
- if (!handler) {
- goto bad_req;
- }
+ coap_get_payload(req->packet, &m, &data_off);
- rc = coap_get_payload(req->packet, &m, &data_off);
cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
-
cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
&o->os_cbuf.ob_mj.it);
- /* start generating the CBOR OUTPUT */
- /* this is worth a quick note. We are encoding CBOR within CBOR, so we
- * need to use the same encoder as ocf stack. We are using their global
- * encoder g_encoder which they intialized before this function is called.
- * But we can't call their macros here as it won't use the right mape
- * encoder (ob_mj) */
+ rc = omgr_oic_read_hdr(&o->os_cbuf.ob_mj.it, &hdr);
+ if (rc != 0) {
+ rc = MGMT_ERR_EINVAL;
+ goto done;
+ }
+
+ /* Convert request header to response header to be sent. */
+ switch (hdr.nh_op) {
+ case NMGR_OP_READ:
+ hdr.nh_op = NMGR_OP_READ_RSP;
+ break;
+
+ case NMGR_OP_WRITE:
+ hdr.nh_op = NMGR_OP_WRITE_RSP;
+ break;
+
+ default:
+ goto done;
+ }
+ rsp_hdr_filled = 1;
+
+ /* Begin root map in response. */
cbor_encoder_create_map(&g_encoder, &o->os_cbuf.ob_mj.encoder,
CborIndefiniteLength);
+ handler = mgmt_find_handler(hdr.nh_group, hdr.nh_id);
+ if (handler == NULL) {
+ rc = MGMT_ERR_ENOENT;
+ goto done;
+ }
+
+ cbor_mbuf_reader_init(&o->os_cbuf.ob_reader, m, data_off);
+ cbor_parser_init(&o->os_cbuf.ob_reader.r, 0, &o->os_cbuf.ob_mj.parser,
+ &o->os_cbuf.ob_mj.it);
+
switch (mask) {
case OC_IF_BASELINE:
oc_process_baseline_interface(req->resource);
+ /* Fallthrough */
+
case OC_IF_RW:
- if (!isset) {
- cbor_encode_text_string(&root_map, "r", 1);
- if (handler->mh_read) {
- rc = handler->mh_read(&o->os_cbuf.ob_mj);
+ switch (hdr.nh_op) {
+ case NMGR_OP_READ_RSP:
+ if (handler->mh_read == NULL) {
+ rc = MGMT_ERR_ENOENT;
} else {
- goto bad_req;
+ rc = handler->mh_read(&o->os_cbuf.ob_mj);
}
- } else {
- cbor_encode_text_string(&root_map, "w", 1);
- if (handler->mh_write) {
- rc = handler->mh_write(&o->os_cbuf.ob_mj);
+ break;
+
+ case NMGR_OP_WRITE_RSP:
+ if (handler->mh_write == NULL) {
+ rc = MGMT_ERR_ENOENT;
} else {
- goto bad_req;
+ rc = handler->mh_write(&o->os_cbuf.ob_mj);
}
+ break;
+
+ default:
+ rc = MGMT_ERR_EINVAL;
+ break;
}
- if (rc) {
- goto bad_req;
+ if (rc != 0) {
+ goto done;
+ }
+
+ /* Encode the NMP header in the response. */
+ rc = omgr_encode_nmp_hdr(&o->os_cbuf.ob_mj.encoder, hdr);
+ if (rc != 0) {
+ rc = MGMT_ERR_ENOMEM;
+ goto done;
}
break;
+
default:
break;
}
- cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
- oc_send_response(req, OC_STATUS_OK);
+ rc = 0;
- return;
-bad_req:
- /*
- * XXXX might send partially constructed response as payload
- */
- if (rc == MGMT_ERR_ENOMEM) {
- rc = OC_STATUS_INTERNAL_SERVER_ERROR;
- } else {
- rc = OC_STATUS_BAD_REQUEST;
- }
- oc_send_response(req, rc);
-}
+done:
+ if (rc != 0) {
+ if (rsp_hdr_filled) {
+ rc = omgr_send_err_rsp(&g_encoder, &hdr, rc);
+ }
+ switch (rc) {
+ case 0:
+ break;
-static void
-omgr_oic_get(oc_request_t *req, oc_interface_mask_t mask)
-{
- omgr_oic_op(req, mask, 0);
-}
+ case MGMT_ERR_ENOMEM:
+ rc = OC_STATUS_INTERNAL_SERVER_ERROR;
+ break;
-static void
-omgr_oic_put(oc_request_t *req, oc_interface_mask_t mask)
-{
- omgr_oic_op(req, mask, 1);
+ default:
+ rc = OC_STATUS_BAD_REQUEST;
+ break;
+ }
+ }
+
+ cbor_encoder_close_container(&g_encoder, &o->os_cbuf.ob_mj.encoder);
+ oc_send_response(req, rc);
}
static void
@@ -204,7 +277,6 @@ omgr_event_start(struct os_event *ev)
oc_resource_bind_resource_interface(res, mode);
oc_resource_set_default_interface(res, mode);
oc_resource_set_discoverable(res);
- oc_resource_set_request_handler(res, OC_GET, omgr_oic_get);
oc_resource_set_request_handler(res, OC_PUT, omgr_oic_put);
oc_add_resource(res);
}