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/10/22 14:42:22 UTC
[GitHub] [mynewt-mcumgr] bpbradley opened a new pull request #99: Implement shell management command handlers.
bpbradley opened a new pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99
I noticed that the shell command was not supported on zephyr, but I desired this functionality for my application. I have implemented the shell_mgmt command handler so that `shell exec` is supported from the mcumgr cli. It will now return the shell command status code, and if possible it will return the shell output as a string.
I did not implement necessary implementations for mynewt os in the PR so only zephyr is supported.
If testing on zephyr, the application will required a custom KConfig option `CONFIG_MCUMGR_CMD_SHELL_MGMT=y` and the shell backend will need to be set to either serial or dummy.
To use the serial backend, configure your project with:
```CONFIG_SHELL_BACKEND_SERIAL=y```
If using the dummy backend:
```
CONFIG_SHELL_BACKEND_SERIAL=n
CONFIG_SHELL_BACKEND_DUMMY=y
```
Because of a limitation in the zephyr shell implementation, the actual shell output cannot be captured and returned to mcumgr when using the serial backend (the command will still be executed and the correct status code will be returned, but the string output won't be displayed).
Also I was unable to find contribution guidelines for the project so please let me know if I need to revise the implementation or commit details.
----------------------------------------------------------------
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 #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
utzig commented on pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#issuecomment-723447248
A bit of testing:
```
$ mcumgr --conn ttyACM0 shell exec "kernel uptime"
status=0
Uptime: 74320 ms
$ newtmgr --conn ttyACM0 shell exec "kernel cycles"
status=0
cycles: 2692533 hw cycles
$ mcumgr --conn ttyACM0 shell exec "kernel threads"
Error: NMP timeout
$ mcumgr --conn ttyACM0 shell exec "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
Error: NMP timeout
$ newtmgr --conn ttyACM0 shell exec "kernel xxx"
status=1
kernel - Kernel commands
Subcommands:
cycles :Kernel cycles.
reboot :Reboot.
stacks :List threads stack usage.
threads :List kernel threads.
uptime :Kernel uptime.
version :Kernel version.
```
For future improvements, it would be better to not output `status=0` when commands have succeeded. Sending or receiving more than a few tens of characters completely hangs the shell, including the shell in a local serial emulator; whatever thread is hanging it's almost like a DoS attack, and this should be handled and fail cleanly.
----------------------------------------------------------------
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] bpbradley commented on a change in pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
bpbradley commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r516731253
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
Review comment:
Thanks for the feedback!
Perhaps I am misunderstanding something, but I don't believe this is up to the user to configure since it is a defined command id which is part of the shell management group.
I reviewed the protocol description [here](https://github.com/apache/mynewt-mcumgr/blob/master/protocol.md) and understood from it that the `shell exec` function in the mcumgr-cli would generate a frame header with `.nh_group = 9` and `.nh_id = 0`. I then setup the mapping for the actual implementation accordingly. If that isn't how it works I am not sure how I would map the function to the cli command without those being pre-defined and agreed upon by both the client and the manager.
I made the other options configurable because those do have repercussions if the os is setup differently than assumptions I would make. For instance, if I just assumed a max line length of 300, but the user configured zephyr with a max line length of 100 (or 400), things might not behave as expected.
----------------------------------------------------------------
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 a change in pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r516688985
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
+
+/**
+ * @brief Registers the shell management command handler group.
+ */
+void
+shell_mgmt_register_group(void);
Review comment:
Why not make it configurable like `SHELL_MGMT_MAX_LINE_LEN`?
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
Review comment:
Why not make it configurable like `SHELL_MGMT_MAX_LINE_LEN`?
----------------------------------------------------------------
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] nvlsianpu commented on pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
nvlsianpu commented on pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#issuecomment-722546793
@utzig ^^
Can be tested with zephyr here https://github.com/zephyrproject-rtos/zephyr/pull/29452
----------------------------------------------------------------
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 #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r519177951
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
+
+/**
+ * @brief Registers the shell management command handler group.
+ */
+void
+shell_mgmt_register_group(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* H_SHELL_MGMT_ */
+
Review comment:
This was not fixed but it's OK.
----------------------------------------------------------------
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] bpbradley commented on a change in pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
bpbradley commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r518992246
##########
File path: cmd/CMakeLists.txt
##########
@@ -19,3 +19,5 @@ add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_FS_MGMT fs_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_IMG_MGMT img_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_OS_MGMT os_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_STAT_MGMT stat_mgmt)
+add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_SHELL_MGMT shell_mgmt)
+
Review comment:
Done
----------------------------------------------------------------
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] bpbradley commented on pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
bpbradley commented on pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#issuecomment-723276464
@utzig I believe I have addressed your comments. Sorry about all those formatting errors, they should be resolved now. 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] de-nordic commented on a change in pull request #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
de-nordic commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r517149240
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
Review comment:
OK.
----------------------------------------------------------------
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 #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
utzig commented on a change in pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99#discussion_r518682415
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
+
+/**
+ * @brief Registers the shell management command handler group.
+ */
+void
Review comment:
There are trailing spaces on both lines above, please remove them.
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt.h
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_
+#define H_SHELL_MGMT_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Command IDs for shell management group.
+ */
+#define SHELL_MGMT_ID_EXEC 0
+
+/**
+ * @brief Registers the shell management command handler group.
+ */
+void
+shell_mgmt_register_group(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* H_SHELL_MGMT_ */
+
Review comment:
Extra empty line should be removed.
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt_impl.h
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @file
+ * @brief Declares implementation-specific functions required by shell
+ * management. The default stubs can be overridden with functions that
+ * are compatible with the host OS.
+ */
+
+#ifndef H_SHELL_MGMT_IMPL_
+#define H_SHELL_MGMT_IMPL_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Execute `line` as a shell command
+ *
+ * @param line : shell command to be executed
+ * @return int : 0 on success, -errno otherwire
+ */
+int
+shell_mgmt_impl_exec(const char *line);
+
+/**
+ * @brief Capture the output of the shell
+ *
+ * @return const char* : shell output. This is not the return code, it is
+ * the string output of the shell command if it exists. If the shell provided no
+ * output, this will be an empty string
+ */
+const char *
+shell_mgmt_impl_get_output();
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
+
Review comment:
Remove the empty line.
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt_impl.h
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @file
+ * @brief Declares implementation-specific functions required by shell
+ * management. The default stubs can be overridden with functions that
+ * are compatible with the host OS.
+ */
+
+#ifndef H_SHELL_MGMT_IMPL_
+#define H_SHELL_MGMT_IMPL_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Execute `line` as a shell command
+ *
Review comment:
Remove the trailing space.
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt_impl.h
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * @file
+ * @brief Declares implementation-specific functions required by shell
+ * management. The default stubs can be overridden with functions that
+ * are compatible with the host OS.
+ */
+
+#ifndef H_SHELL_MGMT_IMPL_
+#define H_SHELL_MGMT_IMPL_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief Execute `line` as a shell command
+ *
+ * @param line : shell command to be executed
+ * @return int : 0 on success, -errno otherwire
+ */
+int
+shell_mgmt_impl_exec(const char *line);
+
+/**
+ * @brief Capture the output of the shell
+ *
Review comment:
Remove the trailing space.
##########
File path: cmd/shell_mgmt/include/shell_mgmt/shell_mgmt_config.h
##########
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef H_SHELL_MGMT_CONFIG_
+#define H_SHELL_MGMT_CONFIG_
+
+#if defined MYNEWT
+
+#include "syscfg/syscfg.h"
+
+#define SHELL_MGMT_MAX_LINE_LEN MYNEWT_VAL(SHELL_BRIDGE_MAX_IN_LEN)
+#define SHELL_MGMT_MAX_ARGC MYNEWT_VAL(SHELL_CMD_ARGC_MAX)
+
+#elif defined __ZEPHYR__
+
+#define SHELL_MGMT_MAX_LINE_LEN CONFIG_SHELL_CMD_BUFF_SIZE
+#define SHELL_MGMT_MAX_ARGC CONFIG_SHELL_ARGC_MAX
+
+#else
+
+/* No direct support for this OS. The application needs to define the above
+ * settings itself.
+ */
+
+#endif
+
+#endif
+
Review comment:
Remove the empty line.
##########
File path: cmd/CMakeLists.txt
##########
@@ -19,3 +19,5 @@ add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_FS_MGMT fs_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_IMG_MGMT img_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_OS_MGMT os_mgmt)
add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_STAT_MGMT stat_mgmt)
+add_subdirectory_ifdef(CONFIG_MCUMGR_CMD_SHELL_MGMT shell_mgmt)
+
Review comment:
Please remove the extra line here.
##########
File path: cmd/shell_mgmt/src/shell_mgmt.c
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include "mgmt/mgmt.h"
+#include "cborattr/cborattr.h"
+#include "shell_mgmt/shell_mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+#include "shell_mgmt/shell_mgmt_config.h"
+
+static mgmt_handler_fn shell_mgmt_exec;
+
+static struct mgmt_handler shell_mgmt_handlers[] = {
+ [SHELL_MGMT_ID_EXEC] = { NULL, shell_mgmt_exec },
+};
+
+#define SHELL_MGMT_HANDLER_CNT \
+ sizeof shell_mgmt_handlers / sizeof shell_mgmt_handlers[0]
+
+static struct mgmt_group shell_mgmt_group = {
+ .mg_handlers = shell_mgmt_handlers,
+ .mg_handlers_count = SHELL_MGMT_HANDLER_CNT,
+ .mg_group_id = MGMT_GROUP_ID_SHELL,
+};
+
+/**
+ * Command handler: shell exec
+ */
+static int
+shell_mgmt_exec(struct mgmt_ctxt *cb)
+{
+ static char line[SHELL_MGMT_MAX_LINE_LEN + 1] = {0};
+ CborEncoder str_encoder;
+ CborError err;
+ int rc;
+ char *argv[SHELL_MGMT_MAX_ARGC];
+ int argc;
+
+
+ const struct cbor_attr_t attrs[] = {
+ {
+ .attribute = "argv",
+ .type = CborAttrArrayType,
+ .addr.array = {
+ .element_type = CborAttrTextStringType,
+ .arr.strings.ptrs = argv,
+ .arr.strings.store = line,
+ .arr.strings.storelen = sizeof line,
+ .count = &argc,
+ .maxlen = sizeof argv / sizeof argv[0],
+ },
+ },
+ { 0 },
+ };
+
+ err = cbor_read_object(&cb->it, attrs);
+ if (err != 0) {
+ return MGMT_ERR_EINVAL;
+ }
+
+ /* Key="o"; value=<command-output> */
+ err |= cbor_encode_text_stringz(&cb->encoder, "o");
+ err |= cbor_encoder_create_indef_text_string(&cb->encoder, &str_encoder);
+
+ rc = shell_mgmt_impl_exec(line);
+
+ err |= cbor_encode_text_stringz(&str_encoder,
Review comment:
Remove trailing space.
##########
File path: cmd/shell_mgmt/src/stubs.c
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License") you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * These stubs get linked in when there is no equivalent OS-specific
+ * implementation.
+ */
+
+#include "mgmt/mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+
+int __attribute__((weak))
+shell_mgmt_impl_exec(const char *line)
+{
+ return MGMT_ERR_ENOTSUP;
+}
+
+const char * __attribute__((weak))
+shell_mgmt_impl_get_output()
+{
+ return "";
+}
+
Review comment:
Please remove the empty line.
##########
File path: cmd/shell_mgmt/port/zephyr/src/zephyr_shell_mgmt.c
##########
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <sys/util.h>
+#include <shell/shell.h>
+#include <mgmt/mgmt.h>
+#include <shell_mgmt/shell_mgmt.h>
+#include <shell/shell_dummy.h>
+
+int
+shell_mgmt_impl_exec(const char *line)
+{
+ const struct shell * shell = shell_backend_dummy_get_ptr();
+ shell_backend_dummy_clear_output(shell);
+ return shell_execute_cmd(shell, line);
+}
+
+const char *
Review comment:
Remove the trailing white space.
##########
File path: cmd/shell_mgmt/src/shell_mgmt.c
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include "mgmt/mgmt.h"
+#include "cborattr/cborattr.h"
+#include "shell_mgmt/shell_mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+#include "shell_mgmt/shell_mgmt_config.h"
+
+static mgmt_handler_fn shell_mgmt_exec;
+
+static struct mgmt_handler shell_mgmt_handlers[] = {
+ [SHELL_MGMT_ID_EXEC] = { NULL, shell_mgmt_exec },
+};
+
+#define SHELL_MGMT_HANDLER_CNT \
+ sizeof shell_mgmt_handlers / sizeof shell_mgmt_handlers[0]
+
+static struct mgmt_group shell_mgmt_group = {
+ .mg_handlers = shell_mgmt_handlers,
+ .mg_handlers_count = SHELL_MGMT_HANDLER_CNT,
+ .mg_group_id = MGMT_GROUP_ID_SHELL,
+};
+
+/**
+ * Command handler: shell exec
+ */
+static int
+shell_mgmt_exec(struct mgmt_ctxt *cb)
+{
+ static char line[SHELL_MGMT_MAX_LINE_LEN + 1] = {0};
+ CborEncoder str_encoder;
+ CborError err;
+ int rc;
+ char *argv[SHELL_MGMT_MAX_ARGC];
+ int argc;
+
+
+ const struct cbor_attr_t attrs[] = {
+ {
+ .attribute = "argv",
+ .type = CborAttrArrayType,
+ .addr.array = {
+ .element_type = CborAttrTextStringType,
+ .arr.strings.ptrs = argv,
+ .arr.strings.store = line,
+ .arr.strings.storelen = sizeof line,
+ .count = &argc,
+ .maxlen = sizeof argv / sizeof argv[0],
+ },
+ },
+ { 0 },
+ };
+
+ err = cbor_read_object(&cb->it, attrs);
+ if (err != 0) {
+ return MGMT_ERR_EINVAL;
+ }
+
+ /* Key="o"; value=<command-output> */
+ err |= cbor_encode_text_stringz(&cb->encoder, "o");
+ err |= cbor_encoder_create_indef_text_string(&cb->encoder, &str_encoder);
+
+ rc = shell_mgmt_impl_exec(line);
+
+ err |= cbor_encode_text_stringz(&str_encoder,
+ shell_mgmt_impl_get_output());
+
+ err |= cbor_encoder_close_container(&cb->encoder, &str_encoder);
+
+ /* Key="rc"; value=<status> */
+ err |= cbor_encode_text_stringz(&cb->encoder, "rc");
+ err |= cbor_encode_int(&cb->encoder, rc);
+
+ if (err != 0) {
+ return MGMT_ERR_ENOMEM;
+ }
+
+ return 0;
+}
+
+void
+shell_mgmt_register_group(void)
+{
+ mgmt_register_group(&shell_mgmt_group);
+}
+
Review comment:
Remove empty line
##########
File path: cmd/shell_mgmt/src/shell_mgmt.c
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include "mgmt/mgmt.h"
+#include "cborattr/cborattr.h"
+#include "shell_mgmt/shell_mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+#include "shell_mgmt/shell_mgmt_config.h"
+
+static mgmt_handler_fn shell_mgmt_exec;
+
+static struct mgmt_handler shell_mgmt_handlers[] = {
+ [SHELL_MGMT_ID_EXEC] = { NULL, shell_mgmt_exec },
+};
+
+#define SHELL_MGMT_HANDLER_CNT \
+ sizeof shell_mgmt_handlers / sizeof shell_mgmt_handlers[0]
+
+static struct mgmt_group shell_mgmt_group = {
+ .mg_handlers = shell_mgmt_handlers,
+ .mg_handlers_count = SHELL_MGMT_HANDLER_CNT,
+ .mg_group_id = MGMT_GROUP_ID_SHELL,
+};
+
+/**
+ * Command handler: shell exec
+ */
+static int
+shell_mgmt_exec(struct mgmt_ctxt *cb)
+{
+ static char line[SHELL_MGMT_MAX_LINE_LEN + 1] = {0};
+ CborEncoder str_encoder;
+ CborError err;
+ int rc;
+ char *argv[SHELL_MGMT_MAX_ARGC];
+ int argc;
+
+
+ const struct cbor_attr_t attrs[] = {
+ {
+ .attribute = "argv",
+ .type = CborAttrArrayType,
+ .addr.array = {
+ .element_type = CborAttrTextStringType,
+ .arr.strings.ptrs = argv,
+ .arr.strings.store = line,
+ .arr.strings.storelen = sizeof line,
+ .count = &argc,
+ .maxlen = sizeof argv / sizeof argv[0],
+ },
+ },
+ { 0 },
+ };
+
Review comment:
Remove trailing spaces.
##########
File path: cmd/shell_mgmt/src/shell_mgmt.c
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include "mgmt/mgmt.h"
+#include "cborattr/cborattr.h"
+#include "shell_mgmt/shell_mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+#include "shell_mgmt/shell_mgmt_config.h"
+
+static mgmt_handler_fn shell_mgmt_exec;
+
+static struct mgmt_handler shell_mgmt_handlers[] = {
+ [SHELL_MGMT_ID_EXEC] = { NULL, shell_mgmt_exec },
+};
+
+#define SHELL_MGMT_HANDLER_CNT \
+ sizeof shell_mgmt_handlers / sizeof shell_mgmt_handlers[0]
+
+static struct mgmt_group shell_mgmt_group = {
+ .mg_handlers = shell_mgmt_handlers,
+ .mg_handlers_count = SHELL_MGMT_HANDLER_CNT,
+ .mg_group_id = MGMT_GROUP_ID_SHELL,
+};
+
+/**
+ * Command handler: shell exec
+ */
+static int
+shell_mgmt_exec(struct mgmt_ctxt *cb)
+{
+ static char line[SHELL_MGMT_MAX_LINE_LEN + 1] = {0};
+ CborEncoder str_encoder;
+ CborError err;
+ int rc;
+ char *argv[SHELL_MGMT_MAX_ARGC];
+ int argc;
+
+
Review comment:
Remove trailing spaces, or better just remove the extra line.
##########
File path: cmd/shell_mgmt/src/shell_mgmt.c
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include "mgmt/mgmt.h"
+#include "cborattr/cborattr.h"
+#include "shell_mgmt/shell_mgmt.h"
+#include "shell_mgmt/shell_mgmt_impl.h"
+#include "shell_mgmt/shell_mgmt_config.h"
+
+static mgmt_handler_fn shell_mgmt_exec;
+
+static struct mgmt_handler shell_mgmt_handlers[] = {
+ [SHELL_MGMT_ID_EXEC] = { NULL, shell_mgmt_exec },
+};
+
+#define SHELL_MGMT_HANDLER_CNT \
+ sizeof shell_mgmt_handlers / sizeof shell_mgmt_handlers[0]
+
+static struct mgmt_group shell_mgmt_group = {
+ .mg_handlers = shell_mgmt_handlers,
+ .mg_handlers_count = SHELL_MGMT_HANDLER_CNT,
+ .mg_group_id = MGMT_GROUP_ID_SHELL,
+};
+
+/**
+ * Command handler: shell exec
+ */
+static int
+shell_mgmt_exec(struct mgmt_ctxt *cb)
+{
+ static char line[SHELL_MGMT_MAX_LINE_LEN + 1] = {0};
+ CborEncoder str_encoder;
+ CborError err;
+ int rc;
+ char *argv[SHELL_MGMT_MAX_ARGC];
+ int argc;
+
+
+ const struct cbor_attr_t attrs[] = {
+ {
+ .attribute = "argv",
+ .type = CborAttrArrayType,
+ .addr.array = {
+ .element_type = CborAttrTextStringType,
+ .arr.strings.ptrs = argv,
+ .arr.strings.store = line,
+ .arr.strings.storelen = sizeof line,
+ .count = &argc,
+ .maxlen = sizeof argv / sizeof argv[0],
+ },
+ },
+ { 0 },
+ };
+
+ err = cbor_read_object(&cb->it, attrs);
+ if (err != 0) {
+ return MGMT_ERR_EINVAL;
+ }
+
+ /* Key="o"; value=<command-output> */
+ err |= cbor_encode_text_stringz(&cb->encoder, "o");
+ err |= cbor_encoder_create_indef_text_string(&cb->encoder, &str_encoder);
+
+ rc = shell_mgmt_impl_exec(line);
+
Review comment:
Remove trailing spaces.
----------------------------------------------------------------
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 #99: Implement shell management command handlers.
Posted by GitBox <gi...@apache.org>.
utzig merged pull request #99:
URL: https://github.com/apache/mynewt-mcumgr/pull/99
----------------------------------------------------------------
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