From 172f8fe44b9cf3adafdf998966d0b8cf69b4fc62 Mon Sep 17 00:00:00 2001 From: Tom van Dijk <5869151+tomvand@users.noreply.github.com> Date: Thu, 10 Dec 2020 09:41:41 +0100 Subject: [PATCH] SPI SD Logger updates (#2605) * modify spi sd logger for arbitrary downlink devices * Update sglogger_download to pprzlink v2 * Fix logger_sd_spi_direct enum settings * Change commented printf to DEBUG_PRINT * settings_xml_parse.py: print errors and warnings to stderr Co-authored-by: Tom van Dijk --- conf/modules/logger_sd_spi_direct.xml | 28 +++++--- .../modules/loggers/sdlogger_spi_direct.c | 17 ++--- sw/lib/python/settings_xml_parse.py | 15 ++-- sw/logalizer/sdlogger_download.c | 69 +++++++++++++------ 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/conf/modules/logger_sd_spi_direct.xml b/conf/modules/logger_sd_spi_direct.xml index dff1c5bbc2..eb60cbdc73 100644 --- a/conf/modules/logger_sd_spi_direct.xml +++ b/conf/modules/logger_sd_spi_direct.xml @@ -5,27 +5,34 @@ Direct SPI SD Logger that saves pprzlog messages to SD Card. -This module logs data directly to an SD Card which is connected on a SPI port. For now, it is only possible to use a radio switch to start and stop logging. +Supported SD cards: +- SDv2 cards. Old SDv1 cards are not supported. +- Both SDSC and SDHC/SDXC are supported. +Some SD cards need to be zeroed out before use: https://askubuntu.com/a/511476 + +This module logs data directly to an SD Card which is connected on a SPI port. +Logging can be started/stopped by setting sdlogger_spi.do_log from the GCS +or by defining SDLOGGER_ON_ARM to TRUE. The values to be logged are defined in the telemetry config file. An example is available in rotorcraft_with_logger.xml. The logmessage is defined as the default message in the process "Logger". A LOGGER_LED can be enabled which indicates if the logger is writing or reading data. -Downloading of the data occurs over an UART datalink. There is no check to verify that all data is transfered, so it is recommended to use an FTDI cable to the UART port. - -Do not use start/stop functionality of the module, the module is not intended to be used like this. +Downloading of the data occurs over the configured serial device using sw/logalizer/sdlogger_download. There is no check to verify that all data is transfered. For UART it is recommended to use an FTDI cable. + + - - - + + + @@ -36,7 +43,7 @@ Do not use start/stop functionality of the module, the module is not intended to - + @@ -55,6 +62,11 @@ Do not use start/stop functionality of the module, the module is not intended to + + + + + diff --git a/sw/airborne/modules/loggers/sdlogger_spi_direct.c b/sw/airborne/modules/loggers/sdlogger_spi_direct.c index 40fd370bd8..4175c13050 100644 --- a/sw/airborne/modules/loggers/sdlogger_spi_direct.c +++ b/sw/airborne/modules/loggers/sdlogger_spi_direct.c @@ -58,8 +58,8 @@ #error "You need to use a telemetry xml file with Logger process!" #endif -#ifndef DOWNLINK_DEVICE -#warning This module can only be used with uart downlink for now. +#ifndef SDLOGGER_DOWNLINK_DEVICE +#error No downlink device defined for SD Logger #endif struct sdlogger_spi_periph sdlogger_spi; @@ -190,12 +190,12 @@ void sdlogger_spi_direct_periodic(void) case SDLogger_Downloading: if (sdcard1.status == SDCard_Idle) { /* Put bytes to the buffer until all is written or buffer is full */ - for (uint16_t i = sdlogger_spi.sdcard_buf_idx; i < SD_BLOCK_SIZE; i++) { - long fd = 0; - if (uart_check_free_space(&(DOWNLINK_DEVICE), &fd, 1)) { - uart_put_byte(&(DOWNLINK_DEVICE), fd, sdcard1.input_buf[i]); - } - else { + long fd = 0; + uint16_t chunk_size = 64; + for (uint16_t i = sdlogger_spi.sdcard_buf_idx; i < SD_BLOCK_SIZE && chunk_size > 0; i++, chunk_size--) { + if ((SDLOGGER_DOWNLINK_DEVICE).device.check_free_space(&(SDLOGGER_DOWNLINK_DEVICE), &fd, 1)) { + (SDLOGGER_DOWNLINK_DEVICE).device.put_byte(&(SDLOGGER_DOWNLINK_DEVICE), fd, sdcard1.input_buf[i]); + } else { /* No free space left, abort for-loop */ break; } @@ -203,6 +203,7 @@ void sdlogger_spi_direct_periodic(void) } /* Request next block if entire buffer was written to uart */ if (sdlogger_spi.sdcard_buf_idx >= SD_BLOCK_SIZE) { + (SDLOGGER_DOWNLINK_DEVICE).device.send_message(&(SDLOGGER_DOWNLINK_DEVICE), fd); // Flush buffers if (sdlogger_spi.download_length > 0) { sdcard_spi_read_block(&sdcard1, sdlogger_spi.download_address, NULL); sdlogger_spi.download_address++; diff --git a/sw/lib/python/settings_xml_parse.py b/sw/lib/python/settings_xml_parse.py index 678c4422b2..32c67d8b99 100755 --- a/sw/lib/python/settings_xml_parse.py +++ b/sw/lib/python/settings_xml_parse.py @@ -6,6 +6,9 @@ import os import sys from lxml import etree +def printerr(*args, **kwargs): + print(*args, file=sys.stderr, **kwargs) + # if PAPARAZZI_HOME not set, then assume the tree containing this # file is a reasonable substitute PPRZ_HOME = os.getenv("PAPARAZZI_HOME", os.path.normpath(os.path.join(os.path.dirname(os.path.abspath(__file__)), @@ -29,7 +32,7 @@ class PaparazziACSettings: # extract aircraft node from conf.xml file ac_node = conf_tree.xpath('/conf/aircraft[@ac_id=%i]' % ac_id) if (len(ac_node) != 1): - print("Aircraft ID %i not found." % ac_id) + printerr("Aircraft ID %i not found." % ac_id) sys.exit(1) # save AC name for reference @@ -39,7 +42,7 @@ class PaparazziACSettings: settings_xml_path = os.path.join(paparazzi_home, 'var/aircrafts/' + self.name + '/settings.xml') if not os.path.isfile(settings_xml_path): - print("Could not find %s, did you build it?" % settings_xml_path) + printerr("Could not find %s, did you build it?" % settings_xml_path) sys.exit(1) index = 0 # keep track of index/id of setting starting at 0 @@ -52,7 +55,7 @@ class PaparazziACSettings: else: setting_group_name = the_tab.attrib['name'] except: - #print("Could not read name of settings group") + #printerr("Could not read name of settings group") continue #print("parsing setting group:", setting_group_name) @@ -71,7 +74,7 @@ class PaparazziACSettings: else: shortname = varname except: - print("Could not get name for setting in group", setting_group) + printerr("Could not get name for setting in group", setting_group) continue settings = PaparazziSetting(shortname, varname) @@ -93,14 +96,14 @@ class PaparazziACSettings: else: settings.step = float(the_setting.attrib['step']) except: - print("Could not get min/max/step for setting", name) + printerr("Could not get min/max/step for setting", name) continue if 'values' in the_setting.attrib: settings.values = the_setting.attrib['values'].split('|') count = int((settings.max_value - settings.min_value + settings.step) / settings.step) if len(settings.values) != count: - print("Warning: possibly wrong number of values (%i) for %s (expected %i)" % (len(settings.values), shortname, count)) + printerr("Warning: possibly wrong number of values (%i) for %s (expected %i)" % (len(settings.values), shortname, count)) setting_group.member_list.append(settings) self.lookup.append(settings) diff --git a/sw/logalizer/sdlogger_download.c b/sw/logalizer/sdlogger_download.c index 4485406505..59396f2c22 100644 --- a/sw/logalizer/sdlogger_download.c +++ b/sw/logalizer/sdlogger_download.c @@ -16,6 +16,13 @@ #include #endif +#define PRINT(string,...) fprintf(stderr, "[sdlogger_download->%s()] " string,__FUNCTION__ , ##__VA_ARGS__) +#ifdef DEBUG +#define DEBUG_PRINT PRINT +#else +#define DEBUG_PRINT(...) +#endif + #define PPRZ_STX 0x99 /* File pointer for temp.tlm */ @@ -55,6 +62,7 @@ enum normal_parser_states { SearchingPPRZ_STX, ParsingLength, ParsingSenderId, + SkippingTwoBytes, ParsingMsgId, ParsingMsgPayload, CheckingCRCA, @@ -270,10 +278,10 @@ void close_port(void) void write_command(float value) { - unsigned char msg[12]; + unsigned char msg[14]; unsigned char crc_a = 0; unsigned char crc_b = 0; - unsigned char length = 12; + unsigned char length = 14; unsigned char *pc; pc = (unsigned char*)&value; @@ -284,25 +292,31 @@ void write_command(float value) msg[2] = 0; // Sender ID crc_a += 0; crc_b += crc_a; - msg[3] = 4; // MSG_ID = SETTING + msg[3] = 0xFF; // Receiver ID (broadcast) + crc_a += 0xFF; crc_b += crc_a; + + msg[4] = 0x02; // Class/Component + crc_a += 0x02; crc_b += crc_a; + + msg[5] = 4; // MSG_ID = SETTING crc_a += 4; crc_b += crc_a; - msg[4] = setting; // setting index + msg[6] = setting; // setting index crc_a += setting; crc_b += crc_a; - msg[5] = ac_id; // AC_ID + msg[7] = ac_id; // AC_ID crc_a += ac_id; crc_b += crc_a; - msg[6] = pc[0]; // value - crc_a += msg[6]; crc_b += crc_a; - msg[7] = pc[1]; // value - crc_a += msg[7]; crc_b += crc_a; - msg[8] = pc[2]; // value + msg[8] = pc[0]; // value crc_a += msg[8]; crc_b += crc_a; - msg[9] = pc[3]; // value + msg[9] = pc[1]; // value crc_a += msg[9]; crc_b += crc_a; - msg[10] = crc_a; - msg[11] = crc_b; + msg[10] = pc[2]; // value + crc_a += msg[10]; crc_b += crc_a; + msg[11] = pc[3]; // value + crc_a += msg[11]; crc_b += crc_a; + msg[12] = crc_a; + msg[13] = crc_b; if (write(fd, msg, length) != length) { fprintf(stderr, "write_command: could not write %d bytes", length); @@ -323,11 +337,12 @@ void write_command(float value) */ void parse_single_byte(unsigned char byte) { + DEBUG_PRINT("parser.state %d\n", parser.state); switch (parser.state) { case SearchingPPRZ_STX: if (byte == PPRZ_STX) { - //printf("Got PPRZ_STX\n"); + DEBUG_PRINT("Got PPRZ_STX\n"); parser.crc_a = 0; parser.crc_b = 0; parser.counter = 1; @@ -348,7 +363,16 @@ void parse_single_byte(unsigned char byte) parser.crc_a += byte; parser.crc_b += parser.crc_a; parser.counter++; - parser.state = ParsingMsgId; + parser.state = SkippingTwoBytes; + break; + + case SkippingTwoBytes: + parser.crc_a += byte; + parser.crc_b += parser.crc_a; + parser.counter++; + if (parser.counter == 5) { + parser.state = ParsingMsgId; + } break; case ParsingMsgId: @@ -360,7 +384,7 @@ void parse_single_byte(unsigned char byte) break; case ParsingMsgPayload: - parser.payload[parser.counter-4] = byte; + parser.payload[parser.counter-6] = byte; parser.crc_a += byte; parser.crc_b += parser.crc_a; parser.counter++; @@ -370,7 +394,7 @@ void parse_single_byte(unsigned char byte) break; case CheckingCRCA: - //printf("CRCA: %d vs %d\n", byte, parser.crc_a); + DEBUG_PRINT("CRCA: %d vs %d\n", byte, parser.crc_a); if (byte == parser.crc_a) { parser.state = CheckingCRCB; } @@ -380,17 +404,17 @@ void parse_single_byte(unsigned char byte) break; case CheckingCRCB: - //printf("CRCB: %d vs %d\n", byte, parser.crc_b); + DEBUG_PRINT("CRCB: %d vs %d\n", byte, parser.crc_b); if (byte == parser.crc_b && parser.msg_id == 31) { - /*printf("MSG ID: %d \t" + DEBUG_PRINT("MSG ID: %d \t" "SENDER_ID: %d\t" "LEN: %d\t" "SETTING: %d\n", parser.msg_id, parser.sender_id, parser.length, - parser.payload[0]);*/ - //printf("Request confirmed\n"); + parser.payload[0]); + DEBUG_PRINT("Request confirmed\n"); /* Check what to do next if the command was received */ if (global_state == WaitingForIndexRequestConfirmation @@ -417,6 +441,7 @@ void parse_single_byte(unsigned char byte) void parse_index_byte(unsigned char byte) { + DEBUG_PRINT("parse_index_byte %d/512: %x\n", index_cnt, byte); unsigned char *pc; pc = (unsigned char*)&log_index; pc[index_cnt] = byte; @@ -440,7 +465,7 @@ void parse_download_byte(unsigned char byte) { static long long dcnt = 0; dcnt++; - //printf("Got \t (%d) \t 0x%02x\n", dcnt, byte); + DEBUG_PRINT("Got \t (%lld) \t 0x%02x\n", dcnt, byte); fputc(byte, fp); /* Show progress */