Services refactor (#17)

* Unify service arrays into a simple struct array (#14)

* Group service arrays into simple struct

* Use size_t and fixed typo

* Added some additional documentation to support new service descriptors structure, bump BBOS version to 0.2

* Minor comments & formatting

---------

Co-authored-by: nbes4 <[email protected]>

cli/node_bin.c

@@ -226,14 +226,14 @@ static void service_exec_callback(struct ush_object *self, struct ush_file_descr
                 shell_print(err_msg);
             }
             else {
-                // Iterate through service_strings array to find the matching service index
+                // Iterate through service_descriptors array to find the matching service index
                 int i;
-                for (i = 0; i < (sizeof(service_strings)/sizeof(service_strings[0])); i++) {
-                    if (strcmp(argv[2], service_strings[i]) == 0) {
-                        service_functions[i](); // call the function pointer at the same index of the matched service string
+                for(i = 0; i < service_descriptors_length; i++) {
+                    if (strcmp(argv[2], service_descriptors[i].name) == 0) {
+                        service_descriptors[i].service_func(); // call the function pointer for the matching service
                         break;
                     }
-                    if (i == ((sizeof(service_strings)/sizeof(service_strings[0]))-1)) {
+                    if (i == (service_descriptors_length - 1)) {
                         sprintf(err_msg, "%s is not an available service, try 'service list'", argv[2]);
                         shell_print(err_msg);
                         break;
@@ -276,15 +276,17 @@ static void service_exec_callback(struct ush_object *self, struct ush_file_descr
                                             "Available Services\tStatus\r\n"
                                             "------------------------------------\r\n"
                                             USH_SHELL_FONT_STYLE_RESET;
-        char *service_list_msg = pvPortMalloc(strlen(service_list_header) + (sizeof(service_strings) * (configMAX_TASK_NAME_LEN + 16)));
+        char *service_list_msg = pvPortMalloc(strlen(service_list_header) +
+                                 (service_descriptors_length *
+                                 (configMAX_TASK_NAME_LEN + 16))); // add 16 bytes per line for service state and whitespace
         TaskHandle_t service_taskhandle;
         char service_state[12];
 
         strcpy(service_list_msg, service_list_header);
 
         // interate through available services, get their states from RTOS
-        for (i = 0; i < (sizeof(service_strings)/sizeof(service_strings[0])); i++) {
-            service_taskhandle = xTaskGetHandle(service_strings[i]);
+        for (i = 0; i < service_descriptors_length; i++) {
+            service_taskhandle = xTaskGetHandle(service_descriptors[i].name);
             if (service_taskhandle == NULL) {
                 strcpy(service_state, "not started");
             }
@@ -306,9 +308,9 @@ static void service_exec_callback(struct ush_object *self, struct ush_file_descr
             }
 
             // copy current service name into table
-            strcpy(service_list_msg + strlen(service_list_msg), service_strings[i]);
+            strcpy(service_list_msg + strlen(service_list_msg), service_descriptors[i].name);
             // add an extra tab to short service names to make the table look better
-            if (strlen(service_strings[i]) < (configMAX_TASK_NAME_LEN - 8)) {
+            if (strlen(service_descriptors[i].name) < (configMAX_TASK_NAME_LEN - 8)) {
                 strcpy(service_list_msg + strlen(service_list_msg), "\t");
             }
             // copy current service state into table

services/CMakeLists.txt

@@ -1,5 +1,6 @@
 # add source files to the top-level project
 target_sources(${PROJ_NAME} PRIVATE
+    services.c
     service_queues.c
     cli_service.c
     usb_service.c

services/services.c

@@ -0,0 +1,55 @@
+/******************************************************************************
+ * @file services.c
+ *
+ * @brief Defines the service descriptors for each system service declared in
+ *        services.h. These service descriptors are used to associate a service
+ *        name string with a service function pointer, as well as indicate if
+ *        the service should be started at boot. The individual service function
+ *        implementations and FreeRTOS APIs for registering the associated tasks
+ *        are contained in the source files for the respective services. Queues
+ *        used to pass data between services are implemented in service_queues.h.
+ *
+ * @author @nbes4 (github)
+ *
+ * @date 06-18-2024
+ * 
+ * @copyright Copyright (c) 2024 @nbes4 (github)
+ *            Released under the MIT License
+ * 
+ * SPDX-License-Identifier: MIT
+ ******************************************************************************/
+
+#include "services.h"
+
+// see services.h for more information on creating services
+// note: use xstr() to convert the SERVICE_NAME_ #define from services.h to a usable string
+// (e.g. xstr(SERVICE_NAME_HEARTBEAT))
+const service_desc_t service_descriptors[] = {
+    {
+        .name = xstr(SERVICE_NAME_USB), 
+        .service_func = usb_service,
+        .startup = true
+    },
+    {
+        .name = xstr(SERVICE_NAME_CLI), 
+        .service_func = cli_service,
+        .startup = true
+    }, 
+    {
+        .name = xstr(SERVICE_NAME_STORMAN), 
+        .service_func = storman_service,
+        .startup = true
+    }, 
+    {
+        .name = xstr(SERVICE_NAME_WATCHDOG), 
+        .service_func = watchdog_service,
+        .startup = true
+    }, 
+    {
+        .name = xstr(SERVICE_NAME_HEARTBEAT), 
+        .service_func = heartbeat_service,
+        .startup = false
+    }
+};
+
+const size_t service_descriptors_length = sizeof(service_descriptors)/sizeof(service_desc_t);

services/services.h

@@ -33,12 +33,12 @@
  *            any time via 'taskmanager', the base service.
  * 
  *            If a new service is added, a SERVICE_NAME must be defined for it,
- *            the service implementation and FreeRTOS task launcher must be
+ *            and the service implementation and FreeRTOS task launcher must be
  *            created in a separate source file (see "heartbeat_service.c" for
- *            an example), its function pointer needs to be added to the
- *            'service_functions[]' array, string name added to 'service_strings[]'
- *            IN THE SAME ORDER, and then it can be added to 'startup_services[]'
- *            if it should run at boot.
+ *            an example). The service's "descriptor" then needs to be added
+ *            to the service_descriptors[] array in services.c, which associates
+ *            the service's main function pointer with its name string, and
+ *            defines whether the service should run at boot.
  * 
  *            The service schedule within FreeRTOS is determined by 3 parameters:
  *            Priority, Repeat, and Delay. Upon any given scheduler tick, the OS
@@ -49,6 +49,12 @@
  *            based on the DELAY parameter. This essentially means a task's run
  *            percentage (within a priority level) is given by REPEAT/DELAY.
  * 
+ *            Pro tip: uncomment add_definitions(-DSCHED_TEST_DELAY) in the
+ *            top-level CMakeLists.txt to burn extra cycles in each service, and
+ *            then use the 'bin/top' CLI command to show FreeRTOS task runtime
+ *            percentages to help tune the scheduler! Don't forget to comment
+ *            this back out after testing!
+ * 
  *            Note that by default, 1 OS tick is 1 ms.
  *            This can be changed in FreeRTOSConfig.h, see 'configTICK_RATE_HZ'
 *******************************************************************************/
@@ -101,6 +107,8 @@
 // FreeRTOS stack sizes for the services - "stack" in this sense is dedicated heap memory for a task.
 // local variables within a service/task use this stack space.
 // If pvPortMalloc is called within a task, it will allocate directly from shared FreeRTOS heap.
+// Use 'bin/ps' command to show a service's min stack (memory usage high water mark)
+// to determine if too much/too little has been allocated.
 #define STACK_TASKMAN   512
 #define STACK_CLI       1024
 #define STACK_USB       1024
@@ -114,7 +122,8 @@
 *************************/
 
 // below are functions for launching the services.
-// pointers to these are added to service_functions[].
+// pointers to these are added to a service's .service_func descriptor item in
+// services.c
 
 /**
 * @brief Start the taskmanager service.
@@ -203,44 +212,33 @@ BaseType_t heartbeat_service(void);
 
 
 /************************
- * Service Arrays
+ * Service Descriptors
 *************************/
 
-// function ptr array of available service functions above for launching with taskmanager.
-// note that taskmanager itself is not in this list (it is a base service).
-// the corresponding string in service_strings[] below will be used to match the service.
-// service_functions[] and service_strings[] need to be in the same order!
-typedef BaseType_t (*ServiceFunc_t)(void);
-static const ServiceFunc_t service_functions[] = {
-    cli_service,
-    usb_service,
-    storman_service,
-    watchdog_service,
-    heartbeat_service
-};
-
-// string versions of service names, used when comparing against user input.
-// use xstr() to convert the SERVICE_NAME_ #define to a usable string.
-// the corresponding index number in the service_functions[] array above will be the
-// function ptr to launch the service.
-// service_functions[] and service_strings[] need to be in the same order!
-static const char *service_strings[] = {
-    xstr(SERVICE_NAME_CLI),
-    xstr(SERVICE_NAME_USB),
-    xstr(SERVICE_NAME_STORMAN),
-    xstr(SERVICE_NAME_WATCHDOG),
-    xstr(SERVICE_NAME_HEARTBEAT)
-};
-
-// startup services - launched automatically by taskmanager at boot.
-// these strings should match with what is in service_strings[] for the intended service.
-// these can be in any order, the corresponding FreeRTOS tasked will be launched in this order.
-static const char *startup_services[] = {
-    "usb",
-    "cli",
-    "storagemanager",
-    "watchdog"
-};
+// service function pointer typedef
+typedef BaseType_t (*service_func_t)(void);
+
+// service descriptor structure to hold the service name, service function pointer and startup flag
+typedef struct service_desc_t {
+    // string version of the service name, used when comparing against user input
+    const char * const name;
+
+    //Defines wether or not this service should be automatically launched by taskmanager at boot
+    const bool startup;
+
+    //Function pointer to the service that creates the respective FreeRTOS task - declared in services.h
+    service_func_t service_func;
+} service_desc_t;
+
+// holds all the services that can be launched with taskmanager.
+// these can be in any order, the corresponding FreeRTOS tasks will be launched in this order.
+// note: taskmanager itself is not in this array (it is a base service).
+// edit services.c to add your own services!
+extern const service_desc_t service_descriptors[];
+
+// number of service descriptors in the array is used to iterate through services
+// for startup and interaction
+extern const size_t service_descriptors_length;
 
 
 #endif /* SERVICES_H */

services/taskman_service.c

@@ -74,15 +74,14 @@ static void prvTaskManagerTask(void *pvParameters)
     // launch startup services
     cli_uart_puts(timestamp());
     cli_uart_puts("Starting all bootup services...\r\n");
-    int i, j;
-    for (i = 0; i < (sizeof(startup_services)/sizeof(startup_services[0])); i++) {
-        for (j = 0; j < (sizeof(service_strings)/sizeof(service_strings[0])); j++) {
-            if (strcmp(service_strings[j], startup_services[i]) == 0) {
-                service_functions[j]();
-            }
+    int i;
+    for (i = 0; i < service_descriptors_length; i++) {
+        if(service_descriptors[i].startup) {
+            service_descriptors[i].service_func();
         }
     }
 
+
     // At this point the system is fully running. Print a message
     cli_uart_puts(timestamp());
     cli_uart_puts("All startup services launched.\r\n");

version.h

@@ -19,7 +19,7 @@
 // maintain BreadboardOS version so it can be tracked along with custom project
 #define BBOS_NAME          "breadboard-os" // do not change
 #define BBOS_VERSION_MAJOR 0               // changed only for major release update
-#define BBOS_VERSION_MINOR 1               // changed only for minor release update
+#define BBOS_VERSION_MINOR 2               // changed only for minor release update
 
 // custom project name and version is defined in top level CMakeLists.txt
 // look for PROJ_NAME and PROJ_VER