Repository: incubator-mynewt-core
Updated Branches:
  refs/heads/develop 37864b722 -> 8fbd28852


minor cleanup of console to CODING_STANDARDS, document shell, make
stats_name_map into a const, to locate statistics into .text


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/8fbd2885
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/8fbd2885
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/8fbd2885

Branch: refs/heads/develop
Commit: 8fbd288523432f48966a2bc217aa2b75444c4637
Parents: 37864b7
Author: Sterling Hughes <sterl...@apache.org>
Authored: Tue Oct 11 12:18:44 2016 +0200
Committer: Sterling Hughes <sterl...@apache.org>
Committed: Tue Oct 11 12:18:44 2016 +0200

----------------------------------------------------------------------
 sys/console/full/src/prompt.c   |   4 +-
 sys/shell/src/shell.c           |  24 +++---
 sys/stats/include/stats/stats.h |  20 ++---
 sys/stats/src/stats.c           | 137 ++++++++++++++++++++++++++++++++++-
 4 files changed, 160 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8fbd2885/sys/console/full/src/prompt.c
----------------------------------------------------------------------
diff --git a/sys/console/full/src/prompt.c b/sys/console/full/src/prompt.c
index 33e37c0..f1b8075 100644
--- a/sys/console/full/src/prompt.c
+++ b/sys/console/full/src/prompt.c
@@ -35,7 +35,9 @@ console_set_prompt(char p)
     console_prompt[1] = p;
 }
 
-void console_no_prompt(void) {
+void
+console_no_prompt(void)
+{
     do_prompt = 0;
 }
 

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8fbd2885/sys/shell/src/shell.c
----------------------------------------------------------------------
diff --git a/sys/shell/src/shell.c b/sys/shell/src/shell.c
index e2844ab..4e3266e 100644
--- a/sys/shell/src/shell.c
+++ b/sys/shell/src/shell.c
@@ -47,27 +47,27 @@ static int shell_echo_cmd(int argc, char **argv);
 static int shell_help_cmd(int argc, char **argv);
 int shell_prompt_cmd(int argc, char **argv);
 
-static struct shell_cmd g_shell_echo_cmd = {
+static const struct shell_cmd g_shell_echo_cmd = {
     .sc_cmd = "echo",
     .sc_cmd_func = shell_echo_cmd
 };
-static struct shell_cmd g_shell_help_cmd = {
+static const struct shell_cmd g_shell_help_cmd = {
     .sc_cmd = "?",
     .sc_cmd_func = shell_help_cmd
 };
-static struct shell_cmd g_shell_prompt_cmd = {
+static const struct shell_cmd g_shell_prompt_cmd = {
    .sc_cmd = "prompt",
    .sc_cmd_func = shell_prompt_cmd
 };
-static struct shell_cmd g_shell_os_tasks_display_cmd = {
+static const struct shell_cmd g_shell_os_tasks_display_cmd = {
     .sc_cmd = "tasks",
     .sc_cmd_func = shell_os_tasks_display_cmd
 };
-static struct shell_cmd g_shell_os_mpool_display_cmd = {
+static const struct shell_cmd g_shell_os_mpool_display_cmd = {
     .sc_cmd = "mempools",
     .sc_cmd_func = shell_os_mpool_display_cmd
 };
-static struct shell_cmd g_shell_os_date_cmd = {
+static const struct shell_cmd g_shell_os_date_cmd = {
     .sc_cmd = "date",
     .sc_cmd_func = shell_os_date_cmd
 };
@@ -549,22 +549,22 @@ shell_init(void)
     rc = os_mutex_init(&g_shell_cmd_list_lock);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_echo_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) &g_shell_echo_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_help_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) &g_shell_help_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_prompt_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) &g_shell_prompt_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_os_tasks_display_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) 
&g_shell_os_tasks_display_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_os_mpool_display_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) 
&g_shell_os_mpool_display_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
-    rc = shell_cmd_register(&g_shell_os_date_cmd);
+    rc = shell_cmd_register((struct shell_cmd *) &g_shell_os_date_cmd);
     SYSINIT_PANIC_ASSERT(rc == 0);
 
     os_eventq_init(&shell_evq);

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8fbd2885/sys/stats/include/stats/stats.h
----------------------------------------------------------------------
diff --git a/sys/stats/include/stats/stats.h b/sys/stats/include/stats/stats.h
index bb047b4..95fe45f 100644
--- a/sys/stats/include/stats/stats.h
+++ b/sys/stats/include/stats/stats.h
@@ -6,7 +6,7 @@
  * 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,
@@ -16,8 +16,8 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-#ifndef __UTIL_STATS_H__ 
-#define __UTIL_STATS_H__ 
+#ifndef __UTIL_STATS_H__
+#define __UTIL_STATS_H__
 
 #include <stdint.h>
 #include "syscfg/syscfg.h"
@@ -30,7 +30,7 @@ extern "C" {
 struct stats_name_map {
     uint16_t snm_off;
     char *snm_name;
-};
+} __attribute__((packed));
 
 struct stats_hdr {
     char *s_name;
@@ -38,7 +38,7 @@ struct stats_hdr {
     uint8_t s_cnt;
     uint16_t s_pad1;
 #if MYNEWT_VAL(STATS_NAMES)
-    struct stats_name_map *s_map;
+    const struct stats_name_map *s_map;
     int s_map_cnt;
 #endif
     STAILQ_ENTRY(stats_hdr) s_next;
@@ -82,7 +82,7 @@ STATS_SECT_DECL(__name) {                   \
 #define STATS_NAME_MAP_NAME(__sectname) g_stats_map_ ## __sectname
 
 #define STATS_NAME_START(__sectname)                                        \
-struct stats_name_map STATS_NAME_MAP_NAME(__sectname)[] = {
+const struct stats_name_map STATS_NAME_MAP_NAME(__sectname)[] = {
 
 #define STATS_NAME(__sectname, __entry)                                     \
     { offsetof(STATS_SECT_DECL(__sectname), STATS_SECT_VAR(__entry)),       \
@@ -105,14 +105,14 @@ struct stats_name_map STATS_NAME_MAP_NAME(__sectname)[] = 
{
 #endif /* MYNEWT_VAL(STATS_NAME) */
 
 void stats_module_init(void);
-int stats_init(struct stats_hdr *shdr, uint8_t size, uint8_t cnt, 
-    struct stats_name_map *map, uint8_t map_cnt);
+int stats_init(struct stats_hdr *shdr, uint8_t size, uint8_t cnt,
+    const struct stats_name_map *map, uint8_t map_cnt);
 int stats_register(char *name, struct stats_hdr *shdr);
 int stats_init_and_reg(struct stats_hdr *shdr, uint8_t size, uint8_t cnt,
                        struct stats_name_map *map, uint8_t map_cnt,
                        char *name);
 
-typedef int (*stats_walk_func_t)(struct stats_hdr *, void *, char *, 
+typedef int (*stats_walk_func_t)(struct stats_hdr *, void *, char *,
         uint16_t);
 int stats_walk(struct stats_hdr *, stats_walk_func_t, void *);
 
@@ -124,7 +124,7 @@ struct stats_hdr *stats_group_find(char *name);
 /* Private */
 #if MYNEWT_VAL(STATS_NEWTMGR)
 int stats_nmgr_register_group(void);
-#endif 
+#endif
 #if MYNEWT_VAL(STATS_CLI)
 int stats_shell_register(void);
 #endif

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/8fbd2885/sys/stats/src/stats.c
----------------------------------------------------------------------
diff --git a/sys/stats/src/stats.c b/sys/stats/src/stats.c
index 5c4ab55..1e4b44e 100644
--- a/sys/stats/src/stats.c
+++ b/sys/stats/src/stats.c
@@ -26,6 +26,48 @@
 #include "os/os.h"
 #include "stats/stats.h"
 
+/**
+ * Declare an example statistics section, which is fittingly, the number
+ * statistics registered in the system.  There are two, largely duplicated,
+ * statistics sections here, in order to provide the optional ability to
+ * name statistics.
+ *
+ * STATS_SECT_START/END actually declare the statistics structure definition,
+ * STATS_SECT_DECL() creates the structure declaration so you can declare
+ * these statistics as a global structure, and STATS_NAME_START/END are how
+ * you name the statistics themselves.
+ *
+ * Statistics entries can be declared as any of the following values, however,
+ * all statistics in a given structure must be of the same size, and they are
+ * all unsigned.
+ *
+ * - STATS_SECT_ENTRY(): default statistic entry, 32-bits.  This
+ *   is the good stuff. Two factors to consider:
+ *       - With 16-bit numbers, rollovers happen, frequently.  Whether its
+ *       testing a pathological condition, or just a long time since you've
+ *       collected a statistic: it really sucks to not have a crucial piece
+ *       of information.
+ *       - 64-bit numbers are wonderful things.  However, the gods did not see
+ *       fit to bless us with unlimited memory.  64-bit statistics are useful
+ *       when you want to store non-statistics in a statistics entry (i.e. 
time),
+ *       because its convenient...
+ *
+ * - STATS_SECT_ENTRY16(): 16-bits.  Smaller statistics if you need to fit into
+ *   specific RAM or code size numbers.
+ *
+ * - STATS_SECT_ENTRY32(): 32-bits, if you want to force it.
+ *
+ * - STATS_SECT_ENTRY64(): 64-bits.  Useful for storing chunks of data.
+ *
+ * Following the statics entry declaration is the statistic names declaration.
+ * This is compiled out when STATS_NAME_ENABLE is set to 0.  This declaration
+ * is const, and therefore can be located in .text, not .data.
+ *
+ * In cases where the system configuration variable STATS_NAME_ENABLE is set
+ * to 1, the statistics names are stored and returned to both the console
+ * and management APIs.  Whereas, when STATS_NAME_ENABLE = 0, these statistics
+ * are numbered, s0, s1, etc.
+ */
 STATS_SECT_START(stats)
     STATS_SECT_ENTRY(num_registered)
 STATS_SECT_END
@@ -39,6 +81,22 @@ STATS_NAME_END(stats)
 STAILQ_HEAD(, stats_hdr) g_stats_registry =
     STAILQ_HEAD_INITIALIZER(g_stats_registry);
 
+
+/**
+ * Walk a specific statistic entry, and call walk_func with arg for
+ * each field within that entry.
+ *
+ * Walk func takes the following parameters:
+ *
+ * - The header of the statistics section (stats_hdr)
+ * - The user supplied argument
+ * - The name of the statistic (if STATS_NAME_ENABLE = 0, this is
+ *   ("s%d", n), where n is the number of the statistic in the structure.
+ * - A pointer to the current entry.
+ *
+ * @return 0 on success, the return code of the walk_func on abort.
+ *
+ */
 int
 stats_walk(struct stats_hdr *hdr, stats_walk_func_t walk_func, void *arg)
 {
@@ -63,6 +121,11 @@ stats_walk(struct stats_hdr *hdr, stats_walk_func_t 
walk_func, void *arg)
          */
         name = NULL;
 #if MYNEWT_VAL(STATS_NAMES)
+        /* The stats name map contains two elements, an offset into the
+         * statistics entry structure, and the name corresponding with that
+         * offset.  This annotation allows for naming only certain statistics,
+         * and doesn't enforce ordering restrictions on the stats name map.
+         */
         for (i = 0; i < hdr->s_map_cnt; ++i) {
             if (hdr->s_map[i].snm_off == cur) {
                 name = hdr->s_map[i].snm_name;
@@ -70,6 +133,10 @@ stats_walk(struct stats_hdr *hdr, stats_walk_func_t 
walk_func, void *arg)
             }
         }
 #endif
+        /* Do this check irrespective of whether MYNEWT_VALUE(STATS_NAMES)
+         * is set.  Users may only partially name elements in the statistics
+         * structure.
+         */
         if (name == NULL) {
             ent_n = (cur - sizeof(*hdr)) / hdr->s_size;
             len = snprintf(name_buf, sizeof(name_buf), "s%d", ent_n);
@@ -82,6 +149,9 @@ stats_walk(struct stats_hdr *hdr, stats_walk_func_t 
walk_func, void *arg)
             goto err;
         }
 
+        /* Statistics are variable sized, move forward either 16, 32 or 64
+         * bits in the structure.
+         */
         cur += hdr->s_size;
     }
 
@@ -90,7 +160,13 @@ err:
     return (rc);
 }
 
-
+/**
+ * Initialize the stastics module.  Called before any of the statistics get
+ * registered to initialize global structures, and register the default
+ * statistics "stat."
+ *
+ * ASSERT's if it fails, as something is likely h0rked system wide.
+ */
 void
 stats_module_init(void)
 {
@@ -117,9 +193,24 @@ stats_module_init(void)
     SYSINIT_PANIC_ASSERT(rc == 0);
 }
 
+
+/**
+ * Initialize a statistics structure, pointed to by hdr.
+ *
+ * @param hdr The header of the statistics structure, contains things
+ *            like statistic section name, size of statistics entries,
+ *            number of statistics, etc.
+ * @param size The size of the individual statistics elements, either
+ *             2 (16-bits), 4 (32-bits) or 8 (64-bits).
+ * @param cnt The number of elements in the statistics structure
+ * @param map The mapping of statistics name to statistic entry
+ * @param map_cnt The number of items in the statistics map
+ *
+ * @return 0 on success, non-zero error code on failure.
+ */
 int
 stats_init(struct stats_hdr *shdr, uint8_t size, uint8_t cnt,
-        struct stats_name_map *map, uint8_t map_cnt)
+        const struct stats_name_map *map, uint8_t map_cnt)
 {
     memset((uint8_t *) shdr+sizeof(*shdr), 0, size * cnt);
 
@@ -133,6 +224,18 @@ stats_init(struct stats_hdr *shdr, uint8_t size, uint8_t 
cnt,
     return (0);
 }
 
+/**
+ * Walk the group of registered statistics and call walk_func() for
+ * each element in the list.  This function _DOES NOT_ lock the statistics
+ * list, and assumes that the list is not being changed by another task.
+ * (assumption: all statistics are registered prior to OS start.)
+ *
+ * @param walk_func The walk function to call, with a statistics header
+ *                  and arg.
+ * @param arg The argument to call the walk function with.
+ *
+ * @return 0 on success, non-zero error code on failure
+ */
 int
 stats_group_walk(stats_group_walk_func_t walk_func, void *arg)
 {
@@ -150,6 +253,14 @@ err:
     return (rc);
 }
 
+/**
+ * Find a statistics structure by name, this is not thread-safe.
+ * (assumption: all statistics are registered prior ot OS start.)
+ *
+ * @param name The statistic structure name to find
+ *
+ * @return statistic structure if found, NULL if not found.
+ */
 struct stats_hdr *
 stats_group_find(char *name)
 {
@@ -165,6 +276,17 @@ stats_group_find(char *name)
     return (cur);
 }
 
+/**
+ * Register the statistics pointed to by shdr, with the name of "name."
+ *
+ * @param name The name of the statistic to register.  This name is guaranteed
+ *             unique in the statistics map.  If already exists, this function
+ *             will return an error.
+ * @param shdr The statistics header to register into the statistic map under
+ *             name.
+ *
+ * @return 0 on success, non-zero error code on failure.
+ */
 int
 stats_register(char *name, struct stats_hdr *shdr)
 {
@@ -194,6 +316,17 @@ err:
 
 /**
  * Initializes and registers the specified statistics section.
+ *
+ * @param shdr The statistics header to register
+ * @param size The entry size of the statistics to register either 2 (16-bit),
+ *             4 (32-bit) or 8 (64-bit).
+ * @param cnt  The number of statistics entries in the statistics structure.
+ * @param map  The map of statistics entry to statistics name, only used when
+ *             MYNEWT_VAL(STATS_NAME) = 1.
+ * @param map_cnt The number of elements in the statistics name map.
+ * @param name The name of the statistics element to register with the system.
+ *
+ * @return 0 on success, non-zero error code on failure.
  */
 int
 stats_init_and_reg(struct stats_hdr *shdr, uint8_t size, uint8_t cnt,

Reply via email to