From 6fc30c76a6cbc2d3b876c0d001b9751ecfbaf260 Mon Sep 17 00:00:00 2001 From: Carlo Wood Date: Fri, 21 Oct 2016 15:54:48 +0200 Subject: [PATCH] Clean up of app.h app.h, generated from app.h_in, has unnecessary code duplication and isn't a header file (it defines globals, static functions and doesn't have a header guard, moreover, it has a 'using namespace std;'). Because of this, a real headerfile that declares the stuff defined in apps.h was missing leading to even more code duplication: scattered forward declarations in .cpp files and an often repeated type of std::map. This patch moves cmake/qurt/apps.h_in to src/platforms/apps.cpp.in (with some changes) and removes cmake/posix/apps.h_in. Then src/platforms/apps.cpp.in is split into src/platforms/apps.cpp.in and src/platforms/apps.h.in, splitting declarations from definitions. A typedef is defined for the map (apps_map_type). The main difference between cmake/posix/apps.h_in and cmake/qurt/apps.h_in was that the first defined a global 'apps', while qurt stores the apps in QShell. I opted to get rid of the global variable (which are in general evil for various reasons) and used the API of cmake/qurt/apps.h_in where a provided 'apps' map is initialized with a call to init_app_map. Thus removing the existing code duplication. --- cmake/posix/apps.h_in | 88 --------------------------- cmake/posix/px4_impl_posix.cmake | 10 +-- cmake/qurt/apps.h_in | 84 ------------------------- cmake/qurt/px4_impl_qurt.cmake | 9 ++- src/drivers/qshell/qurt/qshell.cpp | 17 ++---- src/drivers/qshell/qurt/qshell.h | 5 +- src/firmware/posix/CMakeLists.txt | 10 +-- src/firmware/qurt/CMakeLists.txt | 6 +- src/platforms/apps.cpp.in | 74 ++++++++++++++++++++++ src/platforms/apps.h.in | 27 ++++++++ src/platforms/posix/main.cpp | 12 +++- src/platforms/qurt/px4_layer/main.cpp | 10 ++- 12 files changed, 143 insertions(+), 209 deletions(-) delete mode 100644 cmake/posix/apps.h_in delete mode 100644 cmake/qurt/apps.h_in create mode 100644 src/platforms/apps.cpp.in create mode 100644 src/platforms/apps.h.in diff --git a/cmake/posix/apps.h_in b/cmake/posix/apps.h_in deleted file mode 100644 index f791bd0273..0000000000 --- a/cmake/posix/apps.h_in +++ /dev/null @@ -1,88 +0,0 @@ -/* builtin command list - automatically generated, do not edit */ -#include -#include -#include - -#include -#include -#include -#include - -using namespace std; - -extern void px4_show_devices(void); - -extern "C" { -${builtin_apps_decl_string} -static int shutdown_main(int argc, char *argv[]); -static int list_tasks_main(int argc, char *argv[]); -static int list_files_main(int argc, char *argv[]); -static int list_devices_main(int argc, char *argv[]); -static int list_topics_main(int argc, char *argv[]); -static int sleep_main(int argc, char *argv[]); -} - -static map app_map(void) -{ - static map apps; - -${builtin_apps_string} - apps["shutdown"] = shutdown_main; - apps["list_tasks"] = list_tasks_main; - apps["list_files"] = list_files_main; - apps["list_devices"] = list_devices_main; - apps["list_topics"] = list_topics_main; - apps["sleep"] = sleep_main; - - return apps; -} - -map apps = app_map(); - -static void list_builtins(void) -{ - cout << "Builtin Commands:" << endl; - for (map::iterator it=apps.begin(); it!=apps.end(); ++it) - cout << '\t' << it->first << endl; -} - -static int shutdown_main(int argc, char *argv[]) -{ - printf("Shutting down\n"); - exit(0); -} - -static int list_tasks_main(int argc, char *argv[]) -{ - px4_show_tasks(); - return 0; -} - -static int list_devices_main(int argc, char *argv[]) -{ - px4_show_devices(); - return 0; -} - -static int list_topics_main(int argc, char *argv[]) -{ - px4_show_topics(); - return 0; -} - -static int list_files_main(int argc, char *argv[]) -{ - px4_show_files(); - return 0; -} - -static int sleep_main(int argc, char *argv[]) -{ - if (argc != 2) { - cout << "Usage: sleep " << endl; - return 1; - } - sleep(atoi(argv[1])); - return 0; -} - diff --git a/cmake/posix/px4_impl_posix.cmake b/cmake/posix/px4_impl_posix.cmake index 73f1b55fef..16d5b16b72 100644 --- a/cmake/posix/px4_impl_posix.cmake +++ b/cmake/posix/px4_impl_posix.cmake @@ -66,7 +66,7 @@ list(APPEND CMAKE_MODULE_PATH ${PX4_SOURCE_DIR}/cmake/posix) # MODULE_LIST : list of modules # # Output: -# OUT : generated builtin_commands.c src +# OUT : stem of generated apps.cpp/apps.h ("apps") # # Example: # px4_posix_generate_builtin_commands( @@ -97,12 +97,14 @@ function(px4_posix_generate_builtin_commands) set(builtin_apps_string "${builtin_apps_string}\tapps[\"${MAIN}\"] = ${MAIN}_main;\n") set(builtin_apps_decl_string - "${builtin_apps_decl_string}extern int ${MAIN}_main(int argc, char *argv[]);\n") + "${builtin_apps_decl_string}int ${MAIN}_main(int argc, char *argv[]);\n") math(EXPR command_count "${command_count}+1") endif() endforeach() - configure_file(${PX4_SOURCE_DIR}/cmake/posix/apps.h_in - ${OUT}) + configure_file(${PX4_SOURCE_DIR}/src/platforms/apps.cpp.in + ${OUT}.cpp) + configure_file(${PX4_SOURCE_DIR}/src/platforms/apps.h.in + ${OUT}.h) endfunction() #============================================================================= diff --git a/cmake/qurt/apps.h_in b/cmake/qurt/apps.h_in deleted file mode 100644 index ff06f3e370..0000000000 --- a/cmake/qurt/apps.h_in +++ /dev/null @@ -1,84 +0,0 @@ -/* builtin command list - automatically generated, do not edit */ -#include -#include -#include - -#include -#include -#include -#include - -using namespace std; - -extern void px4_show_devices(void); - -extern "C" { -${builtin_apps_decl_string} -static int shutdown_main(int argc, char *argv[]); -static int list_tasks_main(int argc, char *argv[]); -static int list_files_main(int argc, char *argv[]); -static int list_devices_main(int argc, char *argv[]); -static int list_topics_main(int argc, char *argv[]); -static int sleep_main(int argc, char *argv[]); -} - - -void init_app_map(map &apps) -{ -${builtin_apps_string} - apps["shutdown"] = shutdown_main; - apps["list_tasks"] = list_tasks_main; - apps["list_files"] = list_files_main; - apps["list_devices"] = list_devices_main; - apps["list_topics"] = list_topics_main; - apps["sleep"] = sleep_main; -} - -void list_builtins(map &apps) -{ - PX4_INFO("Builtin Commands:\\n"); - for (map::iterator it=apps.begin(); it!=apps.end(); ++it) - PX4_INFO("%s : 0x%x\n", (it->first).c_str(), it->second); -} - -static int shutdown_main(int argc, char *argv[]) -{ - printf("Shutting down\\n"); - exit(0); -} - -static int list_tasks_main(int argc, char *argv[]) -{ - px4_show_tasks(); - return 0; -} - -static int list_devices_main(int argc, char *argv[]) -{ - px4_show_devices(); - return 0; -} - -static int list_topics_main(int argc, char *argv[]) -{ - px4_show_topics(); - return 0; -} -static int list_files_main(int argc, char *argv[]) -{ - px4_show_files(); - return 0; -} -static int sleep_main(int argc, char *argv[]) -{ - if (argc != 2) { - PX4_WARN( "Usage: sleep " ); - return 1; - } - - unsigned long usecs = ( (unsigned long) atol( argv[1] ) ) * 1000 * 1000; - PX4_WARN("Sleeping for %s, %ld",argv[1],usecs); - usleep( usecs ); - return 0; -} - diff --git a/cmake/qurt/px4_impl_qurt.cmake b/cmake/qurt/px4_impl_qurt.cmake index 731ecca53f..d6ff56f97d 100644 --- a/cmake/qurt/px4_impl_qurt.cmake +++ b/cmake/qurt/px4_impl_qurt.cmake @@ -67,7 +67,7 @@ list(APPEND CMAKE_MODULE_PATH ${PX4_SOURCE_DIR}/cmake/qurt) # MODULE_LIST : list of modules # # Output: -# OUT : generated builtin_commands.c src +# OUT : stem of generated apps.cpp/apps.h ("apps"). # # Example: # px4_qurt_generate_builtin_commands( @@ -91,11 +91,14 @@ function(px4_qurt_generate_builtin_commands) set(builtin_apps_string "${builtin_apps_string}\tapps[\"${MAIN}\"] = ${MAIN}_main;\n") set(builtin_apps_decl_string - "${builtin_apps_decl_string}extern int ${MAIN}_main(int argc, char *argv[]);\n") + "${builtin_apps_decl_string}int ${MAIN}_main(int argc, char *argv[]);\n") math(EXPR command_count "${command_count}+1") endif() endforeach() - configure_file(${PX4_SOURCE_DIR}/cmake/qurt/apps.h_in ${OUT}) + configure_file(${PX4_SOURCE_DIR}/src/platforms/apps.cpp.in + ${OUT}.cpp) + configure_file(${PX4_SOURCE_DIR}/src/platforms/apps.h.in + ${OUT}.h) endfunction() #============================================================================= diff --git a/src/drivers/qshell/qurt/qshell.cpp b/src/drivers/qshell/qurt/qshell.cpp index 151797df9e..6357dda127 100644 --- a/src/drivers/qshell/qurt/qshell.cpp +++ b/src/drivers/qshell/qurt/qshell.cpp @@ -41,7 +41,6 @@ #include "qshell.h" #include -#include #include #include #include @@ -63,17 +62,11 @@ #define MAX_ARGS 8 // max number of whitespace separated args after app name -extern void init_app_map(std::map &apps); -extern void list_builtins(std::map &apps); - -using std::map; -using std::string; - px4::AppState QShell::appState; QShell::QShell() { - init_app_map(apps); + init_app_map(m_apps); } int QShell::main() @@ -150,12 +143,12 @@ int QShell::run_cmd(const std::vector &appargs) std::string command = appargs[0]; if (command.compare("help") == 0) { - list_builtins(apps); + list_builtins(m_apps); return 0; } //replaces app.find with iterator code to avoid null pointer exception - for (map::iterator it = apps.begin(); it != apps.end(); ++it) { + for (apps_map_type::iterator it = m_apps.begin(); it != m_apps.end(); ++it) { if (it->first == command) { // one for command name, one for null terminator const char *arg[MAX_ARGS + 2]; @@ -176,11 +169,11 @@ int QShell::run_cmd(const std::vector &appargs) arg[i] = (char *)0; //PX4_DEBUG_PRINTF(i); - if (apps[command] == NULL) { + if (m_apps[command] == NULL) { PX4_ERR("Null function !!\n"); } else { - return apps[command](i, (char **)arg); + return m_apps[command](i, (char **)arg); } } diff --git a/src/drivers/qshell/qurt/qshell.h b/src/drivers/qshell/qurt/qshell.h index 24ddc9c867..08d86d8e41 100644 --- a/src/drivers/qshell/qurt/qshell.h +++ b/src/drivers/qshell/qurt/qshell.h @@ -41,11 +41,10 @@ #pragma once #include -#include -#include #include #include #include "uORB/topics/qshell_req.h" +#include "apps.h" class QShell { @@ -61,6 +60,6 @@ public: private: struct qshell_req_s m_qshell_req; - std::map apps; + apps_map_type m_apps; }; diff --git a/src/firmware/posix/CMakeLists.txt b/src/firmware/posix/CMakeLists.txt index 7c1f5dde2f..bca1455540 100644 --- a/src/firmware/posix/CMakeLists.txt +++ b/src/firmware/posix/CMakeLists.txt @@ -1,7 +1,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}) px4_posix_generate_builtin_commands( - OUT apps.h + OUT apps MODULE_LIST ${module_libraries}) if ("${BOARD}" STREQUAL "eagle" OR ("${BOARD}" STREQUAL "excelsior")) @@ -19,7 +19,7 @@ if ("${BOARD}" STREQUAL "eagle" OR ("${BOARD}" STREQUAL "excelsior")) SOURCES px4muorb_stub.c ${PX4_SOURCE_DIR}/src/platforms/posix/main.cpp - apps.h + apps.cpp LINK_LIBS -Wl,--start-group ${module_libraries} @@ -41,7 +41,7 @@ elseif ("${BOARD}" STREQUAL "rpi") px4_add_executable(px4 ${PX4_SOURCE_DIR}/src/platforms/posix/main.cpp - apps.h + apps.cpp ) target_link_libraries(px4 @@ -68,7 +68,7 @@ elseif ("${BOARD}" STREQUAL "bebop") px4_add_executable(px4 ${PX4_SOURCE_DIR}/src/platforms/posix/main.cpp - apps.h + apps.cpp ) if (NOT APPLE) @@ -98,7 +98,7 @@ else() px4_add_executable(px4 ${PX4_SOURCE_DIR}/src/platforms/posix/main.cpp - apps.h + apps.cpp ) if (NOT APPLE) diff --git a/src/firmware/qurt/CMakeLists.txt b/src/firmware/qurt/CMakeLists.txt index 6a23b57a12..f025a3c598 100644 --- a/src/firmware/qurt/CMakeLists.txt +++ b/src/firmware/qurt/CMakeLists.txt @@ -9,7 +9,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-missing-prototypes") px4_qurt_generate_builtin_commands( - OUT ${PX4_BINARY_DIR}/apps.h + OUT ${PX4_BINARY_DIR}/apps MODULE_LIST ${module_libraries}) FASTRPC_STUB_GEN(px4muorb.idl) @@ -23,7 +23,7 @@ if ("${QURT_ENABLE_STUBS}" STREQUAL "1") ) add_executable(px4 ${PX4_BINARY_DIR}/src/firmware/qurt/px4muorb_skel.c - ${PX4_BINARY_DIR}/apps.h) + ${PX4_BINARY_DIR}/apps.cpp) target_link_libraries(px4 -Wl,--start-group @@ -42,7 +42,7 @@ else() QURT_LIB(LIB_NAME px4 IDL_NAME px4muorb SOURCES - ${PX4_BINARY_DIR}/apps.h + ${PX4_BINARY_DIR}/apps.cpp LINK_LIBS ${module_libraries} ${target_libraries} diff --git a/src/platforms/apps.cpp.in b/src/platforms/apps.cpp.in new file mode 100644 index 0000000000..30a0875355 --- /dev/null +++ b/src/platforms/apps.cpp.in @@ -0,0 +1,74 @@ +/* definitions of builtin command list - automatically generated, do not edit */ +#include "px4_posix.h" +#include "px4_log.h" + +#include "apps.h" + +#include +#include +#include + +#include + +extern void px4_show_devices(void); + +void init_app_map(apps_map_type &apps) +{ +${builtin_apps_string} + apps["shutdown"] = shutdown_main; + apps["list_tasks"] = list_tasks_main; + apps["list_files"] = list_files_main; + apps["list_devices"] = list_devices_main; + apps["list_topics"] = list_topics_main; + apps["sleep"] = sleep_main; +} + +void list_builtins(apps_map_type &apps) +{ + PX4_INFO("Builtin Commands:\n"); + for (apps_map_type::iterator it = apps.begin(); it != apps.end(); ++it) + PX4_INFO("%s : 0x%x\n", it->first.c_str(), it->second); +} + +int shutdown_main(int argc, char *argv[]) +{ + std::cout << "Shutting down" << std::endl; + exit(0); +} + +int list_tasks_main(int argc, char *argv[]) +{ + px4_show_tasks(); + return 0; +} + +int list_devices_main(int argc, char *argv[]) +{ + px4_show_devices(); + return 0; +} + +int list_topics_main(int argc, char *argv[]) +{ + px4_show_topics(); + return 0; +} + +int list_files_main(int argc, char *argv[]) +{ + px4_show_files(); + return 0; +} + +int sleep_main(int argc, char *argv[]) +{ + if (argc != 2) { + PX4_WARN( "Usage: sleep " ); + return 1; + } + + unsigned long usecs = 1000000UL * atol(argv[1]); + std::cout << "Sleeping for " << argv[1] << " s; (" << usecs << " us)." << std::endl; + usleep(usecs); + return 0; +} diff --git a/src/platforms/apps.h.in b/src/platforms/apps.h.in new file mode 100644 index 0000000000..8e359a3bee --- /dev/null +++ b/src/platforms/apps.h.in @@ -0,0 +1,27 @@ +/* declarations of builtin command list - automatically generated, do not edit */ + +#pragma once + +#include "px4_tasks.h" // px4_main_t +#include + +extern "C" { + +${builtin_apps_decl_string} +int shutdown_main(int argc, char *argv[]); +int list_tasks_main(int argc, char *argv[]); +int list_files_main(int argc, char *argv[]); +int list_devices_main(int argc, char *argv[]); +int list_topics_main(int argc, char *argv[]); +int sleep_main(int argc, char *argv[]); + +} + +// Maps an app name to it's function. +typedef std::map apps_map_type; + +// Initialize an apps map. +void init_app_map(apps_map_type &apps); + +// List an apps map. +void list_builtins(apps_map_type &apps); diff --git a/src/platforms/posix/main.cpp b/src/platforms/posix/main.cpp index 68868a5596..2c2260a99b 100644 --- a/src/platforms/posix/main.cpp +++ b/src/platforms/posix/main.cpp @@ -48,6 +48,8 @@ #include #include "apps.h" #include "px4_middleware.h" +#include "px4_posix.h" +#include "px4_log.h" #include "DriverFramework.hpp" #include #include @@ -191,6 +193,14 @@ static void print_prompt() static void run_cmd(const vector &appargs, bool exit_on_fail, bool silently_fail = false) { + static apps_map_type apps; + static bool initialized = false; + + if (!initialized) { + init_app_map(apps); + initialized = true; + } + // command is appargs[0] string command = appargs[0]; @@ -217,7 +227,7 @@ static void run_cmd(const vector &appargs, bool exit_on_fail, bool silen } } else if (command.compare("help") == 0) { - list_builtins(); + list_builtins(apps); } else if (command.length() == 0 || command[0] == '#') { // Do nothing diff --git a/src/platforms/qurt/px4_layer/main.cpp b/src/platforms/qurt/px4_layer/main.cpp index c20f468450..dffc84e9a8 100644 --- a/src/platforms/qurt/px4_layer/main.cpp +++ b/src/platforms/qurt/px4_layer/main.cpp @@ -55,8 +55,6 @@ using namespace std; -extern void init_app_map(map &apps); -extern void list_builtins(map &apps); static px4_task_t g_dspal_task = -1; __BEGIN_DECLS @@ -70,13 +68,13 @@ void qurt_external_hook(void) { } -static void run_cmd(map &apps, const vector &appargs) +static void run_cmd(apps_map_type &apps, const vector &appargs) { // command is appargs[0] string command = appargs[0]; //replaces app.find with iterator code to avoid null pointer exception - for (map::iterator it = apps.begin(); it != apps.end(); ++it) + for (apps_map_type::iterator it = apps.begin(); it != apps.end(); ++it) if (it->first == command) { // one for command name, one for null terminator const char *arg[MAX_ARGS + 2]; @@ -117,7 +115,7 @@ void eat_whitespace(const char *&b, int &i) i = 0; } -static void process_commands(map &apps, const char *cmds) +static void process_commands(apps_map_type &apps, const char *cmds) { vector appargs; int i = 0; @@ -220,7 +218,7 @@ const char *get_commands() int dspal_entry(int argc, char *argv[]) { PX4_INFO("In dspal_entry"); - map apps; + apps_map_type apps; init_app_map(apps); DriverFramework::Framework::initialize(); px4::init_once();