You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/05/22 15:21:10 UTC
[GitHub] [mynewt-mcumgr] lgl88911 opened a new pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
lgl88911 opened a new pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81
This patch add two new features:
- show task name
- show task stack usage
Signed-off-by: Frank Li <lg...@163.com>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] lgl88911 commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
lgl88911 commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r434648523
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -56,19 +56,36 @@ int
os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
{
const struct k_thread *thread;
+#if defined(CONFIG_INIT_STACKS) && defined(CONFIG_THREAD_STACK_INFO)
+ size_t unused;
+#endif
thread = zephyr_os_mgmt_task_at(idx);
if (thread == NULL) {
return MGMT_ERR_ENOENT;
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, OS_MGMT_TASK_NAME_LEN-1);
+ out_info->oti_name[OS_MGMT_TASK_NAME_LEN - 1] = '\0';
+#else
ll_to_s(thread->base.prio, sizeof out_info->oti_name, out_info->oti_name);
+#endif
+
out_info->oti_prio = thread->base.prio;
out_info->oti_taskid = idx;
out_info->oti_state = thread->base.thread_state;
#ifdef CONFIG_THREAD_STACK_INFO
out_info->oti_stksize = thread->stack_info.size / 4;
+#ifdef CONFIG_INIT_STACKS
+ if(k_thread_stack_space_get(thread, &unused)==0){
+ out_info->oti_stkusage = (thread->stack_info.size - unused)/4;
+ }else{
Review comment:
Thanks for your review, Modified
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#issuecomment-634061713
Can you point me to where you are reviewing Zephyr west.yml (https://github.com/zephyrproject-rtos/zephyr/pulls) changes for this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] lgl88911 commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
lgl88911 commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r430396654
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -63,12 +64,21 @@ os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, sizeof out_info->oti_name);
Review comment:
Good suggestion，Thanks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] lgl88911 commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
lgl88911 commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r429545719
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -63,12 +64,21 @@ os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, sizeof out_info->oti_name);
Review comment:
Thanks for your review.
Indeed oti_name maybe overflow, I modify it, Do you think it is appropriate to modify this way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] de-nordic commented on pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
de-nordic commented on pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#issuecomment-638632117
@utzig Thanks for merging it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig merged pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
utzig merged pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r430022552
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -63,12 +64,21 @@ os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, sizeof out_info->oti_name);
Review comment:
That works, but you could just add the NULL after finishing the `strncpy`:
```
out_info->oti_name[OS_MGMT_TASK_NAME_LEN - 1] = '\0';
```
It would avoid doing an extra write of the whole the buffer (which `strncpy` also does!).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r434151755
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -56,19 +56,36 @@ int
os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
{
const struct k_thread *thread;
+#if defined(CONFIG_INIT_STACKS) && defined(CONFIG_THREAD_STACK_INFO)
+ size_t unused;
+#endif
thread = zephyr_os_mgmt_task_at(idx);
if (thread == NULL) {
return MGMT_ERR_ENOENT;
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, OS_MGMT_TASK_NAME_LEN-1);
+ out_info->oti_name[OS_MGMT_TASK_NAME_LEN - 1] = '\0';
+#else
ll_to_s(thread->base.prio, sizeof out_info->oti_name, out_info->oti_name);
+#endif
+
out_info->oti_prio = thread->base.prio;
out_info->oti_taskid = idx;
out_info->oti_state = thread->base.thread_state;
#ifdef CONFIG_THREAD_STACK_INFO
out_info->oti_stksize = thread->stack_info.size / 4;
+#ifdef CONFIG_INIT_STACKS
+ if(k_thread_stack_space_get(thread, &unused)==0){
+ out_info->oti_stkusage = (thread->stack_info.size - unused)/4;
+ }else{
Review comment:
Could you fix the coding style with the missing "space" characters? eg
```
if (k_thread_stack_space_get(thread, &unused) == 0) {
out_info->oti_stkusage = (thread->stack_info.size - unused) / 4;
} else {
```
Otherwise looks good to me, ,I will merge once fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] lgl88911 commented on pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
lgl88911 commented on pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#issuecomment-635394966
> Got informed that documentation is here: https://docs.zephyrproject.org/latest/guides/modules.html#changes-to-existing-modules so I am hiding my previous comment that describes the process.
Thank you for your guidance, I am not sure if this is correct, please help check this pr
https://github.com/zephyrproject-rtos/zephyr/pull/25711
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig commented on pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
utzig commented on pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#issuecomment-634891760
> Can you point me to where you are reviewing Zephyr west.yml (https://github.com/zephyrproject-rtos/zephyr/pulls) changes for this?
@lgl88911 Do you mind to comment?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [mynewt-mcumgr] utzig commented on a change in pull request #81: cmd/os_mgmt/port/zephyr: zephyr port os enhancement
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #81:
URL: https://github.com/apache/mynewt-mcumgr/pull/81#discussion_r429317064
##########
File path: cmd/os_mgmt/port/zephyr/src/zephyr_os_mgmt.c
##########
@@ -63,12 +64,21 @@ os_mgmt_impl_task_info(int idx, struct os_mgmt_task_info *out_info)
}
*out_info = (struct os_mgmt_task_info){ 0 };
+
+#ifdef CONFIG_THREAD_NAME
+ strncpy(out_info->oti_name, thread->name, sizeof out_info->oti_name);
Review comment:
What if Zephyr's `CONFIG_THREAD_MAX_NAME_LEN` is configured with a value larger than `OS_MGMT_TASK_NAME_LEN`, wouldn't this end up with a missing NULL terminator?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org