diff --git a/libraries/AP_Logger/AP_Logger.cpp b/libraries/AP_Logger/AP_Logger.cpp index 58ab87a50c..43bdad5952 100644 --- a/libraries/AP_Logger/AP_Logger.cpp +++ b/libraries/AP_Logger/AP_Logger.cpp @@ -1076,7 +1076,7 @@ bool AP_Logger::assert_same_fmt_for_name(const AP_Logger::log_write_fmt *f, } #endif -AP_Logger::log_write_fmt *AP_Logger::msg_fmt_for_name(const char *name, const char *labels, const char *units, const char *mults, const char *fmt, const bool direct_comp) +AP_Logger::log_write_fmt *AP_Logger::msg_fmt_for_name(const char *name, const char *labels, const char *units, const char *mults, const char *fmt, const bool direct_comp, const bool copy_strings) { WITH_SEMAPHORE(log_write_fmts_sem); struct log_write_fmt *f; @@ -1117,11 +1117,40 @@ AP_Logger::log_write_fmt *AP_Logger::msg_fmt_for_name(const char *name, const ch return nullptr; } f->msg_type = msg_type; - f->name = name; - f->fmt = fmt; - f->labels = labels; - f->units = units; - f->mults = mults; + + if (copy_strings) { + // cannot use pointers to memory that might move, must allocate and copy + struct log_write_fmt_strings *ls_copy = (struct log_write_fmt_strings*)malloc(sizeof(log_write_fmt_strings)); + if (ls_copy == nullptr) { + free(f); + return nullptr; + } + + strncpy_noterm(ls_copy->name, name, sizeof(ls_copy->name)); + strncpy_noterm(ls_copy->format, fmt, sizeof(ls_copy->format)); + strncpy_noterm(ls_copy->labels, labels, sizeof(ls_copy->labels)); + + f->name = ls_copy->name; + f->fmt = ls_copy->format; + f->labels = ls_copy->labels; + + if (units != nullptr) { + strncpy_noterm(ls_copy->units, units, sizeof(ls_copy->units)); + f->units = ls_copy->units; + } + + if (mults != nullptr) { + strncpy_noterm(ls_copy->multipliers, mults, sizeof(ls_copy->multipliers)); + f->mults = ls_copy->multipliers; + } + + } else { + f->name = name; + f->fmt = fmt; + f->labels = labels; + f->units = units; + f->mults = mults; + } int16_t tmp = Write_calc_msg_len(fmt); if (tmp == -1) { @@ -1131,37 +1160,41 @@ AP_Logger::log_write_fmt *AP_Logger::msg_fmt_for_name(const char *name, const ch f->msg_len = tmp; - // add to front of list - f->next = log_write_fmts; - log_write_fmts = f; + // add direct_comp formats to start of list, otherwise add to the end, this minimises the number of string comparisons when walking the list in future calls + if (direct_comp || (log_write_fmts == nullptr)) { + f->next = log_write_fmts; + log_write_fmts = f; + } else { + struct log_write_fmt *list_end = log_write_fmts; + while (list_end->next) { + list_end=list_end->next; + } + list_end->next = f; + } #if CONFIG_HAL_BOARD == HAL_BOARD_SITL - char ls_name[LS_NAME_SIZE] = {}; - char ls_format[LS_FORMAT_SIZE] = {}; - char ls_labels[LS_LABELS_SIZE] = {}; - char ls_units[LS_UNITS_SIZE] = {}; - char ls_multipliers[LS_MULTIPLIERS_SIZE] = {}; + struct log_write_fmt_strings ls_strings = {}; struct LogStructure ls = { f->msg_type, f->msg_len, - ls_name, - ls_format, - ls_labels, - ls_units, - ls_multipliers + ls_strings.name, + ls_strings.format, + ls_strings.labels, + ls_strings.units, + ls_strings.multipliers }; - memcpy((char*)ls_name, f->name, MIN(sizeof(ls_name), strlen(f->name))); - memcpy((char*)ls_format, f->fmt, MIN(sizeof(ls_format), strlen(f->fmt))); - memcpy((char*)ls_labels, f->labels, MIN(sizeof(ls_labels), strlen(f->labels))); + memcpy((char*)ls_strings.name, f->name, MIN(sizeof(ls_strings.name), strlen(f->name))); + memcpy((char*)ls_strings.format, f->fmt, MIN(sizeof(ls_strings.format), strlen(f->fmt))); + memcpy((char*)ls_strings.labels, f->labels, MIN(sizeof(ls_strings.labels), strlen(f->labels))); if (f->units != nullptr) { - memcpy((char*)ls_units, f->units, MIN(sizeof(ls_units), strlen(f->units))); + memcpy((char*)ls_strings.units, f->units, MIN(sizeof(ls.units), strlen(f->units))); } else { - memset((char*)ls_units, '?', MIN(sizeof(ls_format), strlen(f->fmt))); + memset((char*)ls_strings.units, '?', MIN(sizeof(ls.format), strlen(f->fmt))); } if (f->mults != nullptr) { - memcpy((char*)ls_multipliers, f->mults, MIN(sizeof(ls_multipliers), strlen(f->mults))); + memcpy((char*)ls_strings.multipliers, f->mults, MIN(sizeof(ls_strings.multipliers), strlen(f->mults))); } else { - memset((char*)ls_multipliers, '?', MIN(sizeof(ls_format), strlen(f->fmt))); + memset((char*)ls_strings.multipliers, '?', MIN(sizeof(ls_strings.format), strlen(f->fmt))); } if (!validate_structure(&ls, (int16_t)-1)) { AP_BoardConfig::config_error("See console: Log structure invalid"); diff --git a/libraries/AP_Logger/AP_Logger.h b/libraries/AP_Logger/AP_Logger.h index 5ce75274e9..2bc1de8f75 100644 --- a/libraries/AP_Logger/AP_Logger.h +++ b/libraries/AP_Logger/AP_Logger.h @@ -435,7 +435,7 @@ public: } *log_write_fmts; // return (possibly allocating) a log_write_fmt for a name - struct log_write_fmt *msg_fmt_for_name(const char *name, const char *labels, const char *units, const char *mults, const char *fmt, const bool direct_comp = false); + struct log_write_fmt *msg_fmt_for_name(const char *name, const char *labels, const char *units, const char *mults, const char *fmt, const bool direct_comp = false, const bool copy_strings = false); // output a FMT message for each backend if not already done so void Safe_Write_Emit_FMT(log_write_fmt *f); @@ -525,6 +525,14 @@ private: bool _force_log_disarmed:1; bool _force_long_log_persist:1; + struct log_write_fmt_strings { + char name[LS_NAME_SIZE]; + char format[LS_FORMAT_SIZE]; + char labels[LS_LABELS_SIZE]; + char units[LS_UNITS_SIZE]; + char multipliers[LS_MULTIPLIERS_SIZE]; + }; + // remember formats for replay void save_format_Replay(const void *pBuffer); diff --git a/libraries/AP_Logger/AP_Logger_Backend.cpp b/libraries/AP_Logger/AP_Logger_Backend.cpp index 7638bb9d90..16f162ed7c 100644 --- a/libraries/AP_Logger/AP_Logger_Backend.cpp +++ b/libraries/AP_Logger/AP_Logger_Backend.cpp @@ -181,20 +181,16 @@ bool AP_Logger_Backend::Write_Emit_FMT(uint8_t msg_type) #endif // get log structure from front end: - char ls_name[LS_NAME_SIZE] = {}; - char ls_format[LS_FORMAT_SIZE] = {}; - char ls_labels[LS_LABELS_SIZE] = {}; - char ls_units[LS_UNITS_SIZE] = {}; - char ls_multipliers[LS_MULTIPLIERS_SIZE] = {}; + struct AP_Logger::log_write_fmt_strings ls = {}; struct LogStructure logstruct = { // these will be overwritten, but need to keep the compiler happy: 0, 0, - ls_name, - ls_format, - ls_labels, - ls_units, - ls_multipliers + ls.name, + ls.format, + ls.labels, + ls.units, + ls.multipliers }; if (!_front.fill_log_write_logstructure(logstruct, msg_type)) { // this is a bug; we've been asked to write out the FMT