From 6a98d9131c0cb1a59169d3de62acb8ed91236ac2 Mon Sep 17 00:00:00 2001 From: Michael Oborne Date: Thu, 3 Sep 2020 18:08:24 +0800 Subject: [PATCH 1/6] HAL_ChibiOS: fix sdcard param init the write was failing because of a 0 byte write attempt, the response was -1 vs 0 this results in not using the sdcard backend for storage for all boots after the initial --- libraries/AP_HAL_ChibiOS/Storage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/AP_HAL_ChibiOS/Storage.cpp b/libraries/AP_HAL_ChibiOS/Storage.cpp index bc1b10e30b..e91f7ce66f 100644 --- a/libraries/AP_HAL_ChibiOS/Storage.cpp +++ b/libraries/AP_HAL_ChibiOS/Storage.cpp @@ -103,7 +103,7 @@ void Storage::_storage_open(void) } // pre-fill to full size if (AP::FS().lseek(log_fd, ret, SEEK_SET) != ret || - AP::FS().write(log_fd, &_buffer[ret], CH_STORAGE_SIZE-ret) != CH_STORAGE_SIZE-ret) { + (CH_STORAGE_SIZE-ret > 0 && AP::FS().write(log_fd, &_buffer[ret], CH_STORAGE_SIZE-ret) != CH_STORAGE_SIZE-ret)) { ::printf("setup failed for " HAL_STORAGE_FILE "\n"); AP::FS().close(log_fd); log_fd = -1; From b9b89a06bb3cd5684d410087696211b750d531df Mon Sep 17 00:00:00 2001 From: chobits Date: Wed, 8 Apr 2020 14:58:10 +0800 Subject: [PATCH 2/6] AP_Logger: constraints time spended in header writing --- libraries/AP_Logger/LoggerMessageWriter.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/AP_Logger/LoggerMessageWriter.cpp b/libraries/AP_Logger/LoggerMessageWriter.cpp index a27de642f0..750579dab8 100644 --- a/libraries/AP_Logger/LoggerMessageWriter.cpp +++ b/libraries/AP_Logger/LoggerMessageWriter.cpp @@ -1,5 +1,6 @@ #include "AP_Common/AP_FWVersion.h" #include "LoggerMessageWriter.h" +#include #define FORCE_VERSION_H_INCLUDE #include "ap_version.h" @@ -35,6 +36,9 @@ void LoggerMessageWriter_DFLogStart::reset() void LoggerMessageWriter_DFLogStart::process() { + if (AP::scheduler().time_available_usec() < 200) { + return; + } switch(stage) { case Stage::FORMATS: // write log formats so the log is self-describing From 218c3d7e4c46668f6800b26233c3036d752818b4 Mon Sep 17 00:00:00 2001 From: chobits Date: Thu, 9 Apr 2020 15:50:54 +0800 Subject: [PATCH 3/6] AP_Logger: constraints time spend in header writing, more complete --- libraries/AP_Logger/LoggerMessageWriter.cpp | 29 ++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/libraries/AP_Logger/LoggerMessageWriter.cpp b/libraries/AP_Logger/LoggerMessageWriter.cpp index 750579dab8..49c18ba761 100644 --- a/libraries/AP_Logger/LoggerMessageWriter.cpp +++ b/libraries/AP_Logger/LoggerMessageWriter.cpp @@ -6,6 +6,8 @@ #include "ap_version.h" #undef FORCE_VERSION_H_INCLUDE +#define MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US 200 + extern const AP_HAL::HAL& hal; /* LogStartup - these are simple state machines which allow us to @@ -36,13 +38,13 @@ void LoggerMessageWriter_DFLogStart::reset() void LoggerMessageWriter_DFLogStart::process() { - if (AP::scheduler().time_available_usec() < 200) { - return; - } switch(stage) { case Stage::FORMATS: // write log formats so the log is self-describing while (next_format_to_send < _logger_backend->num_types()) { + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_logger_backend->Write_Format(_logger_backend->structure(next_format_to_send))) { return; // call me again! } @@ -54,6 +56,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::UNITS: while (_next_unit_to_send < _logger_backend->num_units()) { + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_logger_backend->Write_Unit(_logger_backend->unit(_next_unit_to_send))) { return; // call me again! } @@ -64,6 +69,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::MULTIPLIERS: while (_next_multiplier_to_send < _logger_backend->num_multipliers()) { + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_logger_backend->Write_Multiplier(_logger_backend->multiplier(_next_multiplier_to_send))) { return; // call me again! } @@ -74,6 +82,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::FORMAT_UNITS: while (_next_format_unit_to_send < _logger_backend->num_types()) { + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_logger_backend->Write_Format_Units(_logger_backend->structure(_next_format_unit_to_send))) { return; // call me again! } @@ -84,6 +95,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::PARMS: while (ap) { + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_logger_backend->Write_Parameter(ap, token, type)) { return; } @@ -95,6 +109,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::SYSINFO: _writesysinfo.process(); + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_writesysinfo.finished()) { return; } @@ -103,6 +120,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::WRITE_ENTIRE_MISSION: _writeentiremission.process(); + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_writeentiremission.finished()) { return; } @@ -111,6 +131,9 @@ void LoggerMessageWriter_DFLogStart::process() case Stage::WRITE_ALL_RALLY_POINTS: _writeallrallypoints.process(); + if (AP::scheduler().time_available_usec() < MIN_LOOP_TIME_REMAINING_FOR_MESSAGE_WRITE_US) { + return; + } if (!_writeallrallypoints.finished()) { return; } From 1b36c4bc7cb35de7733e7c6e30b139e4e6f31899 Mon Sep 17 00:00:00 2001 From: Peter Barker Date: Mon, 11 May 2020 12:45:23 +1000 Subject: [PATCH 4/6] AP_Logger: process pending rotate on arming If the user arms the vehicle during the logging-persist-timeout we should rotate the log immediately. --- libraries/AP_Logger/AP_Logger_File.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libraries/AP_Logger/AP_Logger_File.cpp b/libraries/AP_Logger/AP_Logger_File.cpp index 06992441c6..f84d0fa10c 100644 --- a/libraries/AP_Logger/AP_Logger_File.cpp +++ b/libraries/AP_Logger/AP_Logger_File.cpp @@ -792,6 +792,10 @@ void AP_Logger_File::stop_logging(void) void AP_Logger_File::PrepForArming() { + if (_rotate_pending) { + _rotate_pending = false; + stop_logging(); + } if (logging_started() && _front._params.file_disarm_rot == 0) { return; From 1e27a493261cc7f2e3b080347e1d0d65c89a2a03 Mon Sep 17 00:00:00 2001 From: Peter Barker Date: Sat, 20 Mar 2021 08:28:29 +1100 Subject: [PATCH 5/6] AP_Logger: do not log soon after file transfer unless we're armed mavlink log reads fail randomly, and you end up with a very large number of log files as we keep closing logs off and then discovering we should be logging --- libraries/AP_Logger/AP_Logger.h | 3 +++ libraries/AP_Logger/AP_Logger_Backend.cpp | 11 +++++++++++ libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp | 1 + 3 files changed, 15 insertions(+) diff --git a/libraries/AP_Logger/AP_Logger.h b/libraries/AP_Logger/AP_Logger.h index 3e2741f81c..b2d32a221f 100644 --- a/libraries/AP_Logger/AP_Logger.h +++ b/libraries/AP_Logger/AP_Logger.h @@ -488,6 +488,9 @@ class AP_Logger SENDING, // actively sending log_sending packets } transfer_activity = IDLE; + // last time we handled a log-transfer-over-mavlink message: + uint32_t _last_mavlink_log_transfer_message_handled_ms; + // next log list entry to send uint16_t _log_next_list_entry; diff --git a/libraries/AP_Logger/AP_Logger_Backend.cpp b/libraries/AP_Logger/AP_Logger_Backend.cpp index 1c93d62025..0abdc9069c 100644 --- a/libraries/AP_Logger/AP_Logger_Backend.cpp +++ b/libraries/AP_Logger/AP_Logger_Backend.cpp @@ -381,6 +381,17 @@ bool AP_Logger_Backend::ShouldLog(bool is_critical) return false; } + if (_front._last_mavlink_log_transfer_message_handled_ms != 0) { + if (AP_HAL::millis() - _front._last_mavlink_log_transfer_message_handled_ms < 10000) { + if (!_front.vehicle_is_armed()) { + // user is transfering files via mavlink + return false; + } + } else { + _front._last_mavlink_log_transfer_message_handled_ms = 0; + } + } + if (is_critical && have_logged_armed && !_front._params.file_disarm_rot) { // if we have previously logged while armed then we log all // critical messages from then on. That fixes a problem where diff --git a/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp b/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp index 7a93f859f8..585099a989 100644 --- a/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp +++ b/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp @@ -45,6 +45,7 @@ void AP_Logger::handle_log_message(GCS_MAVLINK &link, const mavlink_message_t &m if (!should_handle_log_message()) { return; } + _last_mavlink_log_transfer_message_handled_ms = AP_HAL::millis(); switch (msg.msgid) { case MAVLINK_MSG_ID_LOG_REQUEST_LIST: handle_log_request_list(link, msg); From 7cd2ccf9cdcf2c532de4d25d37c82cd244ca799c Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 1 May 2021 11:15:11 +1000 Subject: [PATCH 6/6] AP_Logger: minimal fix for allowing log listing while logging this prevents loss of log data when listing logs on file backend --- libraries/AP_Logger/AP_Logger.cpp | 13 +++++++++++++ libraries/AP_Logger/AP_Logger.h | 2 +- libraries/AP_Logger/AP_Logger_Backend.cpp | 3 ++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/libraries/AP_Logger/AP_Logger.cpp b/libraries/AP_Logger/AP_Logger.cpp index 1a3bbcccf5..0efe6e478f 100644 --- a/libraries/AP_Logger/AP_Logger.cpp +++ b/libraries/AP_Logger/AP_Logger.cpp @@ -542,6 +542,19 @@ bool AP_Logger::should_log(const uint32_t mask) const return true; } +/* + return true if in log download which should prevent logging + */ +bool AP_Logger::in_log_download() const +{ + if (uint8_t(_params.backend_types) & uint8_t(Backend_Type::BLOCK)) { + // when we have a BLOCK backend then listing completely prevents logging + return transfer_activity != transfer_activity_t::IDLE; + } + // for other backends listing does not interfere with logging + return transfer_activity == transfer_activity_t::SENDING; +} + const struct UnitStructure *AP_Logger::unit(uint16_t num) const { return &_units[num]; diff --git a/libraries/AP_Logger/AP_Logger.h b/libraries/AP_Logger/AP_Logger.h index b2d32a221f..fa5a6ef479 100644 --- a/libraries/AP_Logger/AP_Logger.h +++ b/libraries/AP_Logger/AP_Logger.h @@ -364,7 +364,7 @@ class AP_Logger bool vehicle_is_armed() const { return _armed; } void handle_log_send(); - bool in_log_download() const { return transfer_activity != IDLE; } + bool in_log_download() const; float quiet_nanf() const { return nanf("0x4152"); } // "AR" double quiet_nan() const { return nan("0x4152445550490a"); } // "ARDUPI" diff --git a/libraries/AP_Logger/AP_Logger_Backend.cpp b/libraries/AP_Logger/AP_Logger_Backend.cpp index 0abdc9069c..96abb83337 100644 --- a/libraries/AP_Logger/AP_Logger_Backend.cpp +++ b/libraries/AP_Logger/AP_Logger_Backend.cpp @@ -381,7 +381,8 @@ bool AP_Logger_Backend::ShouldLog(bool is_critical) return false; } - if (_front._last_mavlink_log_transfer_message_handled_ms != 0) { + if (_front.in_log_download() && + _front._last_mavlink_log_transfer_message_handled_ms != 0) { if (AP_HAL::millis() - _front._last_mavlink_log_transfer_message_handled_ms < 10000) { if (!_front.vehicle_is_armed()) { // user is transfering files via mavlink