From 570858d87c016841610ef71bfe2a9184c1f319b9 Mon Sep 17 00:00:00 2001 From: Simon Wilks Date: Tue, 23 Apr 2013 22:41:44 +0200 Subject: [PATCH] General cleanup based on the pull request from muggenhor --- apps/hott_telemetry/hott_telemetry_main.c | 117 +++++++++------------- apps/hott_telemetry/messages.c | 17 ++-- apps/hott_telemetry/messages.h | 20 ++-- 3 files changed, 68 insertions(+), 86 deletions(-) diff --git a/apps/hott_telemetry/hott_telemetry_main.c b/apps/hott_telemetry/hott_telemetry_main.c index 31c9247aa5..3e147d8292 100644 --- a/apps/hott_telemetry/hott_telemetry_main.c +++ b/apps/hott_telemetry/hott_telemetry_main.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include "messages.h" @@ -66,56 +67,44 @@ static int thread_should_exit = false; /**< Deamon exit flag */ static int thread_running = false; /**< Deamon status flag */ static int deamon_task; /**< Handle of deamon task / thread */ -static char *daemon_name = "hott_telemetry"; -static char *commandline_usage = "usage: hott_telemetry start|status|stop [-d ]"; +static const char daemon_name[] = "hott_telemetry"; +static const char commandline_usage[] = "usage: hott_telemetry start|status|stop [-d ]"; - -/* A little console messaging experiment - console helper macro */ -#define FATAL_MSG(_msg) fprintf(stderr, "[%s] %s\n", daemon_name, _msg); exit(1); -#define ERROR_MSG(_msg) fprintf(stderr, "[%s] %s\n", daemon_name, _msg); -#define INFO_MSG(_msg) printf("[%s] %s\n", daemon_name, _msg); /** * Deamon management function. */ __EXPORT int hott_telemetry_main(int argc, char *argv[]); /** - * Mainloop of deamon. + * Mainloop of daemon. */ int hott_telemetry_thread_main(int argc, char *argv[]); -static int read_data(int uart, int *id); -static int send_data(int uart, uint8_t *buffer, int size); +static int recv_req_id(int uart, uint8_t *id); +static int send_data(int uart, uint8_t *buffer, size_t size); -static int open_uart(const char *device, struct termios *uart_config_original) +static int +open_uart(const char *device, struct termios *uart_config_original) { /* baud rate */ - int speed = B19200; - int uart; + static const speed_t speed = B19200; /* open uart */ - uart = open(device, O_RDWR | O_NOCTTY); + const int uart = open(device, O_RDWR | O_NOCTTY); if (uart < 0) { - char msg[80]; - sprintf(msg, "Error opening port: %s\n", device); - FATAL_MSG(msg); + err(1, "Error opening port: %s", device); } - - /* Try to set baud rate */ - struct termios uart_config; + + /* Back up the original uart configuration to restore it after exit */ int termios_state; - - /* Back up the original uart configuration to restore it after exit */ - char msg[80]; - if ((termios_state = tcgetattr(uart, uart_config_original)) < 0) { - sprintf(msg, "Error getting baudrate / termios config for %s: %d\n", device, termios_state); close(uart); - FATAL_MSG(msg); + err(1, "Error getting baudrate / termios config for %s: %d", device, termios_state); } /* Fill the struct for the new configuration */ + struct termios uart_config; tcgetattr(uart, &uart_config); /* Clear ONLCR flag (which appends a CR for every LF) */ @@ -123,16 +112,14 @@ static int open_uart(const char *device, struct termios *uart_config_original) /* Set baud rate */ if (cfsetispeed(&uart_config, speed) < 0 || cfsetospeed(&uart_config, speed) < 0) { - sprintf(msg, "Error setting baudrate / termios config for %s: %d (cfsetispeed, cfsetospeed)\n", - device, termios_state); close(uart); - FATAL_MSG(msg); + err(1, "Error setting baudrate / termios config for %s: %d (cfsetispeed, cfsetospeed)", + device, termios_state); } if ((termios_state = tcsetattr(uart, TCSANOW, &uart_config)) < 0) { - sprintf(msg, "Error setting baudrate / termios config for %s (tcsetattr)\n", device); close(uart); - FATAL_MSG(msg); + err(1, "Error setting baudrate / termios config for %s (tcsetattr)", device); } /* Activate single wire mode */ @@ -141,18 +128,19 @@ static int open_uart(const char *device, struct termios *uart_config_original) return uart; } -int read_data(int uart, int *id) +int +recv_req_id(int uart, uint8_t *id) { - const int timeout = 1000; + static const int timeout_ms = 1000; // TODO make it a define struct pollfd fds[] = { { .fd = uart, .events = POLLIN } }; - char mode; + uint8_t mode; - if (poll(fds, 1, timeout) > 0) { + if (poll(fds, 1, timeout_ms) > 0) { /* Get the mode: binary or text */ - read(uart, &mode, 1); + read(uart, &mode, sizeof(mode)); /* Read the device ID being polled */ - read(uart, id, 1); + read(uart, id, sizeof(*id)); /* if we have a binary mode request */ if (mode != BINARY_MODE_REQUEST_ID) { @@ -160,20 +148,21 @@ int read_data(int uart, int *id) } } else { - ERROR_MSG("UART timeout on TX/RX port"); + warnx("UART timeout on TX/RX port"); return ERROR; } return OK; } -int send_data(int uart, uint8_t *buffer, int size) +int +send_data(int uart, uint8_t *buffer, size_t size) { usleep(POST_READ_DELAY_IN_USECS); uint16_t checksum = 0; - for (int i = 0; i < size; i++) { + for (size_t i = 0; i < size; i++) { if (i == size - 1) { /* Set the checksum: the first uint8_t is taken as the checksum. */ buffer[i] = checksum & 0xff; @@ -182,7 +171,7 @@ int send_data(int uart, uint8_t *buffer, int size) checksum += buffer[i]; } - write(uart, &buffer[i], 1); + write(uart, &buffer[i], sizeof(buffer[i])); /* Sleep before sending the next byte. */ usleep(POST_WRITE_DELAY_IN_USECS); @@ -196,13 +185,14 @@ int send_data(int uart, uint8_t *buffer, int size) return OK; } -int hott_telemetry_thread_main(int argc, char *argv[]) +int +hott_telemetry_thread_main(int argc, char *argv[]) { - INFO_MSG("starting"); + warnx("starting"); thread_running = true; - char *device = "/dev/ttyS1"; /**< Default telemetry port: USART2 */ + const char *device = "/dev/ttyS1"; /**< Default telemetry port: USART2 */ /* read commandline arguments */ for (int i = 0; i < argc && argv[i]; i++) { @@ -212,31 +202,28 @@ int hott_telemetry_thread_main(int argc, char *argv[]) } else { thread_running = false; - ERROR_MSG("missing parameter to -d"); - ERROR_MSG(commandline_usage); - exit(1); + errx(1, "missing parameter to -d\n%s", commandline_usage); } } } /* enable UART, writes potentially an empty buffer, but multiplexing is disabled */ struct termios uart_config_original; - int uart = open_uart(device, &uart_config_original); + const int uart = open_uart(device, &uart_config_original); if (uart < 0) { - ERROR_MSG("Failed opening HoTT UART, exiting."); + errx(1, "Failed opening HoTT UART, exiting."); thread_running = false; - exit(ERROR); } messages_init(); uint8_t buffer[MESSAGE_BUFFER_SIZE]; - int size = 0; - int id = 0; + size_t size = 0; + uint8_t id = 0; while (!thread_should_exit) { - if (read_data(uart, &id) == OK) { + if (recv_req_id(uart, &id) == OK) { switch (id) { case ELECTRIC_AIR_MODULE: build_eam_response(buffer, &size); @@ -250,7 +237,7 @@ int hott_telemetry_thread_main(int argc, char *argv[]) } } - INFO_MSG("exiting"); + warnx("exiting"); close(uart); @@ -262,23 +249,22 @@ int hott_telemetry_thread_main(int argc, char *argv[]) /** * Process command line arguments and tart the daemon. */ -int hott_telemetry_main(int argc, char *argv[]) +int +hott_telemetry_main(int argc, char *argv[]) { if (argc < 1) { - ERROR_MSG("missing command"); - ERROR_MSG(commandline_usage); - exit(1); + errx(1, "missing command\n%s", commandline_usage); } if (!strcmp(argv[1], "start")) { if (thread_running) { - INFO_MSG("deamon already running"); + warnx("deamon already running"); exit(0); } thread_should_exit = false; - deamon_task = task_spawn("hott_telemetry", + deamon_task = task_spawn(daemon_name, SCHED_DEFAULT, SCHED_PRIORITY_MAX - 40, 2048, @@ -294,19 +280,14 @@ int hott_telemetry_main(int argc, char *argv[]) if (!strcmp(argv[1], "status")) { if (thread_running) { - INFO_MSG("daemon is running"); + warnx("daemon is running"); } else { - INFO_MSG("daemon not started"); + warnx("daemon not started"); } exit(0); } - ERROR_MSG("unrecognized command"); - ERROR_MSG(commandline_usage); - exit(1); + errx(1, "unrecognized command\n%s", commandline_usage); } - - - diff --git a/apps/hott_telemetry/messages.c b/apps/hott_telemetry/messages.c index 8bfb997737..c1988cd054 100644 --- a/apps/hott_telemetry/messages.c +++ b/apps/hott_telemetry/messages.c @@ -48,27 +48,26 @@ static int battery_sub = -1; static int sensor_sub = -1; -void messages_init(void) +void +messages_init(void) { battery_sub = orb_subscribe(ORB_ID(battery_status)); sensor_sub = orb_subscribe(ORB_ID(sensor_combined)); } -void build_eam_response(uint8_t *buffer, int *size) +void +build_eam_response(uint8_t *buffer, size_t *size) { /* get a local copy of the current sensor values */ - struct sensor_combined_s raw; - memset(&raw, 0, sizeof(raw)); + struct sensor_combined_s raw = { 0 }; orb_copy(ORB_ID(sensor_combined), sensor_sub, &raw); /* get a local copy of the battery data */ - struct battery_status_s battery; - memset(&battery, 0, sizeof(battery)); + struct battery_status_s battery = { 0 }; orb_copy(ORB_ID(battery_status), battery_sub, &battery); - struct eam_module_msg msg; + struct eam_module_msg msg = { 0 }; *size = sizeof(msg); - memset(&msg, 0, *size); msg.start = START_BYTE; msg.eam_sensor_id = ELECTRIC_AIR_MODULE; @@ -84,4 +83,4 @@ void build_eam_response(uint8_t *buffer, int *size) msg.stop = STOP_BYTE; memcpy(buffer, &msg, *size); -} \ No newline at end of file +} diff --git a/apps/hott_telemetry/messages.h b/apps/hott_telemetry/messages.h index 44001e04f0..9ca205f818 100644 --- a/apps/hott_telemetry/messages.h +++ b/apps/hott_telemetry/messages.h @@ -43,11 +43,6 @@ #include -/* The buffer size used to store messages. This must be at least as big as the number of - * fields in the largest message struct. - */ -#define MESSAGE_BUFFER_SIZE 50 - /* The HoTT receiver demands a minimum 5ms period of silence after delivering its request. * Note that the value specified here is lower than 5000 (5ms) as time is lost constucting * the message after the read which takes some milliseconds. @@ -71,12 +66,12 @@ /* The Electric Air Module message. */ struct eam_module_msg { uint8_t start; /**< Start byte */ - uint8_t eam_sensor_id; /**< EAM sensor ID */ + uint8_t eam_sensor_id; /**< EAM sensor */ uint8_t warning; - uint8_t sensor_id; /**< Sensor ID, why different? */ + uint8_t sensor_id; /**< Sensor ID, why different? */ uint8_t alarm_inverse1; uint8_t alarm_inverse2; - uint8_t cell1_L; /**< Lipo cell voltages. Not supported. */ + uint8_t cell1_L; /**< Lipo cell voltages. Not supported. */ uint8_t cell2_L; uint8_t cell3_L; uint8_t cell4_L; @@ -117,7 +112,14 @@ struct eam_module_msg { uint8_t checksum; /**< Lower 8-bits of all bytes summed. */ }; +/** + * The maximum buffer size required to store a HoTT message. + */ +#define MESSAGE_BUFFER_SIZE sizeof(union { \ + struct eam_module_msg eam; \ +}) + void messages_init(void); -void build_eam_response(uint8_t *buffer, int *size); +void build_eam_response(uint8_t *buffer, size_t *size); #endif /* MESSAGES_H_ */