From 1d65daed2acd03ca097ec12f91346a928a2e7ed6 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 15:40:46 -0700 Subject: [PATCH 1/9] clean up RedirectablePrint::log so it doesn't have three very different implementations inline. --- src/RedirectablePrint.cpp | 235 ++++++++++++++++++++------------------ src/RedirectablePrint.h | 11 ++ src/SerialConsole.cpp | 5 +- 3 files changed, 139 insertions(+), 112 deletions(-) diff --git a/src/RedirectablePrint.cpp b/src/RedirectablePrint.cpp index b77720d85d9..10fdd051d61 100644 --- a/src/RedirectablePrint.cpp +++ b/src/RedirectablePrint.cpp @@ -39,7 +39,7 @@ void RedirectablePrint::setDestination(Print *_dest) size_t RedirectablePrint::write(uint8_t c) { // Always send the characters to our segger JTAG debugger -#ifdef SEGGER_STDOUT_CH +#ifdef USE_SEGGER SEGGER_RTT_PutChar(SEGGER_STDOUT_CH, c); #endif @@ -71,138 +71,154 @@ size_t RedirectablePrint::vprintf(const char *format, va_list arg) return len; } -size_t RedirectablePrint::log(const char *logLevel, const char *format, ...) +void RedirectablePrint::log_to_serial(const char *logLevel, const char *format, va_list arg) { -#ifdef ARCH_PORTDUINO - if (settingsMap[logoutputlevel] < level_debug && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_DEBUG) == 0) - return 0; - else if (settingsMap[logoutputlevel] < level_info && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_INFO) == 0) - return 0; - else if (settingsMap[logoutputlevel] < level_warn && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_WARN) == 0) - return 0; -#endif - if (moduleConfig.serial.override_console_serial_port && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_DEBUG) == 0) { - return 0; - } size_t r = 0; -#ifdef HAS_FREE_RTOS - if (inDebugPrint != nullptr && xSemaphoreTake(inDebugPrint, portMAX_DELAY) == pdTRUE) { -#else - if (!inDebugPrint) { - inDebugPrint = true; -#endif - - va_list arg; - va_start(arg, format); - // Cope with 0 len format strings, but look for new line terminator - bool hasNewline = *format && format[strlen(format) - 1] == '\n'; + // Cope with 0 len format strings, but look for new line terminator + bool hasNewline = *format && format[strlen(format) - 1] == '\n'; - // If we are the first message on a report, include the header - if (!isContinuationMessage) { - uint32_t rtc_sec = getValidTime(RTCQuality::RTCQualityDevice, true); // display local time on logfile - if (rtc_sec > 0) { - long hms = rtc_sec % SEC_PER_DAY; - // hms += tz.tz_dsttime * SEC_PER_HOUR; - // hms -= tz.tz_minuteswest * SEC_PER_MIN; - // mod `hms` to ensure in positive range of [0...SEC_PER_DAY) - hms = (hms + SEC_PER_DAY) % SEC_PER_DAY; + // If we are the first message on a report, include the header + if (!isContinuationMessage) { + uint32_t rtc_sec = getValidTime(RTCQuality::RTCQualityDevice, true); // display local time on logfile + if (rtc_sec > 0) { + long hms = rtc_sec % SEC_PER_DAY; + // hms += tz.tz_dsttime * SEC_PER_HOUR; + // hms -= tz.tz_minuteswest * SEC_PER_MIN; + // mod `hms` to ensure in positive range of [0...SEC_PER_DAY) + hms = (hms + SEC_PER_DAY) % SEC_PER_DAY; - // Tear apart hms into h:m:s - int hour = hms / SEC_PER_HOUR; - int min = (hms % SEC_PER_HOUR) / SEC_PER_MIN; - int sec = (hms % SEC_PER_HOUR) % SEC_PER_MIN; // or hms % SEC_PER_MIN + // Tear apart hms into h:m:s + int hour = hms / SEC_PER_HOUR; + int min = (hms % SEC_PER_HOUR) / SEC_PER_MIN; + int sec = (hms % SEC_PER_HOUR) % SEC_PER_MIN; // or hms % SEC_PER_MIN #ifdef ARCH_PORTDUINO - r += ::printf("%s | %02d:%02d:%02d %u ", logLevel, hour, min, sec, millis() / 1000); + r += ::printf("%s | %02d:%02d:%02d %u ", logLevel, hour, min, sec, millis() / 1000); #else - r += printf("%s | %02d:%02d:%02d %u ", logLevel, hour, min, sec, millis() / 1000); + r += printf("%s | %02d:%02d:%02d %u ", logLevel, hour, min, sec, millis() / 1000); #endif - } else + } else #ifdef ARCH_PORTDUINO - r += ::printf("%s | ??:??:?? %u ", logLevel, millis() / 1000); + r += ::printf("%s | ??:??:?? %u ", logLevel, millis() / 1000); #else - r += printf("%s | ??:??:?? %u ", logLevel, millis() / 1000); + r += printf("%s | ??:??:?? %u ", logLevel, millis() / 1000); #endif - auto thread = concurrency::OSThread::currentThread; - if (thread) { - print("["); - // printf("%p ", thread); - // assert(thread->ThreadName.length()); - print(thread->ThreadName); - print("] "); - } + auto thread = concurrency::OSThread::currentThread; + if (thread) { + print("["); + // printf("%p ", thread); + // assert(thread->ThreadName.length()); + print(thread->ThreadName); + print("] "); } - r += vprintf(format, arg); + } + r += vprintf(format, arg); + isContinuationMessage = !hasNewline; +} + +void RedirectablePrint::log_to_syslog(const char *logLevel, const char *format, va_list arg) +{ #if (HAS_WIFI || HAS_ETHERNET) && !defined(ARCH_PORTDUINO) - // if syslog is in use, collect the log messages and send them to syslog - if (syslog.isEnabled()) { - int ll = 0; - switch (logLevel[0]) { - case 'D': - ll = SYSLOG_DEBUG; - break; - case 'I': - ll = SYSLOG_INFO; - break; - case 'W': - ll = SYSLOG_WARN; - break; - case 'E': - ll = SYSLOG_ERR; - break; - case 'C': - ll = SYSLOG_CRIT; - break; - default: - ll = 0; - } - auto thread = concurrency::OSThread::currentThread; - if (thread) { - syslog.vlogf(ll, thread->ThreadName.c_str(), format, arg); - } else { - syslog.vlogf(ll, format, arg); - } + // if syslog is in use, collect the log messages and send them to syslog + if (syslog.isEnabled()) { + int ll = 0; + switch (logLevel[0]) { + case 'D': + ll = SYSLOG_DEBUG; + break; + case 'I': + ll = SYSLOG_INFO; + break; + case 'W': + ll = SYSLOG_WARN; + break; + case 'E': + ll = SYSLOG_ERR; + break; + case 'C': + ll = SYSLOG_CRIT; + break; + default: + ll = 0; + } + auto thread = concurrency::OSThread::currentThread; + if (thread) { + syslog.vlogf(ll, thread->ThreadName.c_str(), format, arg); + } else { + syslog.vlogf(ll, format, arg); } + } #endif +} - isContinuationMessage = !hasNewline; - - if (config.bluetooth.device_logging_enabled && !pauseBluetoothLogging) { - bool isBleConnected = false; +void RedirectablePrint::log_to_ble(const char *logLevel, const char *format, va_list arg) +{ + if (config.bluetooth.device_logging_enabled && !pauseBluetoothLogging) { + bool isBleConnected = false; #ifdef ARCH_ESP32 - isBleConnected = nimbleBluetooth && nimbleBluetooth->isActive() && nimbleBluetooth->isConnected(); + isBleConnected = nimbleBluetooth && nimbleBluetooth->isActive() && nimbleBluetooth->isConnected(); #elif defined(ARCH_NRF52) - isBleConnected = nrf52Bluetooth != nullptr && nrf52Bluetooth->isConnected(); + isBleConnected = nrf52Bluetooth != nullptr && nrf52Bluetooth->isConnected(); #endif - if (isBleConnected) { - char *message; - size_t initialLen; - size_t len; - initialLen = strlen(format); - message = new char[initialLen + 1]; - len = vsnprintf(message, initialLen + 1, format, arg); - if (len > initialLen) { - delete[] message; - message = new char[len + 1]; - vsnprintf(message, len + 1, format, arg); - } - auto thread = concurrency::OSThread::currentThread; + if (isBleConnected) { + char *message; + size_t initialLen; + size_t len; + initialLen = strlen(format); + message = new char[initialLen + 1]; + len = vsnprintf(message, initialLen + 1, format, arg); + if (len > initialLen) { + delete[] message; + message = new char[len + 1]; + vsnprintf(message, len + 1, format, arg); + } + auto thread = concurrency::OSThread::currentThread; #ifdef ARCH_ESP32 - if (thread) - nimbleBluetooth->sendLog(mt_sprintf("%s | [%s] %s", logLevel, thread->ThreadName.c_str(), message).c_str()); - else - nimbleBluetooth->sendLog(mt_sprintf("%s | %s", logLevel, message).c_str()); + if (thread) + nimbleBluetooth->sendLog(mt_sprintf("%s | [%s] %s", logLevel, thread->ThreadName.c_str(), message).c_str()); + else + nimbleBluetooth->sendLog(mt_sprintf("%s | %s", logLevel, message).c_str()); #elif defined(ARCH_NRF52) - if (thread) - nrf52Bluetooth->sendLog(mt_sprintf("%s | [%s] %s", logLevel, thread->ThreadName.c_str(), message).c_str()); - else - nrf52Bluetooth->sendLog(mt_sprintf("%s | %s", logLevel, message).c_str()); + if (thread) + nrf52Bluetooth->sendLog(mt_sprintf("%s | [%s] %s", logLevel, thread->ThreadName.c_str(), message).c_str()); + else + nrf52Bluetooth->sendLog(mt_sprintf("%s | %s", logLevel, message).c_str()); #endif - delete[] message; - } + delete[] message; } + } +} + +size_t RedirectablePrint::log(const char *logLevel, const char *format, ...) +{ +#ifdef ARCH_PORTDUINO + if (settingsMap[logoutputlevel] < level_debug && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_DEBUG) == 0) + return 0; + else if (settingsMap[logoutputlevel] < level_info && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_INFO) == 0) + return 0; + else if (settingsMap[logoutputlevel] < level_warn && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_WARN) == 0) + return 0; +#endif + if (moduleConfig.serial.override_console_serial_port && strcmp(logLevel, MESHTASTIC_LOG_LEVEL_DEBUG) == 0) { + return 0; + } + +#ifdef HAS_FREE_RTOS + if (inDebugPrint != nullptr && xSemaphoreTake(inDebugPrint, portMAX_DELAY) == pdTRUE) { +#else + if (!inDebugPrint) { + inDebugPrint = true; +#endif + + va_list arg; + va_start(arg, format); + + log_to_serial(logLevel, format, arg); + log_to_syslog(logLevel, format, arg); + log_to_ble(logLevel, format, arg); + va_end(arg); #ifdef HAS_FREE_RTOS xSemaphoreGive(inDebugPrint); @@ -211,7 +227,8 @@ size_t RedirectablePrint::log(const char *logLevel, const char *format, ...) #endif } - return r; + return 0; // We assume callers don't really care about the # of chars printed (given that this method now sometimes goes out + // via BLE or syslog) } void RedirectablePrint::hexDump(const char *logLevel, unsigned char *buf, uint16_t len) diff --git a/src/RedirectablePrint.h b/src/RedirectablePrint.h index 31cc1b6ef78..23f83b825b3 100644 --- a/src/RedirectablePrint.h +++ b/src/RedirectablePrint.h @@ -23,6 +23,12 @@ class RedirectablePrint : public Print #else volatile bool inDebugPrint = false; #endif + protected: + /** + * If true we are talking to a smart host and all messages (including log messages) must be framed as protobufs. + */ + bool usingProtobufs = false; + public: explicit RedirectablePrint(Print *_dest) : dest(_dest) {} @@ -49,6 +55,11 @@ class RedirectablePrint : public Print void hexDump(const char *logLevel, unsigned char *buf, uint16_t len); std::string mt_sprintf(const std::string fmt_str, ...); + + private: + void log_to_serial(const char *logLevel, const char *format, va_list arg); + void log_to_syslog(const char *logLevel, const char *format, va_list arg); + void log_to_ble(const char *logLevel, const char *format, va_list arg); }; class NoopPrint : public Print diff --git a/src/SerialConsole.cpp b/src/SerialConsole.cpp index 53ece0fa3db..18d6c1f9f9b 100644 --- a/src/SerialConsole.cpp +++ b/src/SerialConsole.cpp @@ -81,9 +81,8 @@ bool SerialConsole::handleToRadio(const uint8_t *buf, size_t len) { // only talk to the API once the configuration has been loaded and we're sure the serial port is not disabled. if (config.has_lora && config.device.serial_enabled) { - // Turn off debug serial printing once the API is activated, because other threads could print and corrupt packets - if (!config.device.debug_log_enabled) - setDestination(&noopPrint); + // Switch to protobufs for log messages + usingProtobufs = true; canWrite = true; return StreamAPI::handleToRadio(buf, len); From 5464310761713f37b92b735c54c2f6c44231bba6 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 15:44:02 -0700 Subject: [PATCH 2/9] remove NoopPrint - it is no longer needed --- src/RedirectablePrint.cpp | 8 ++------ src/RedirectablePrint.h | 13 +------------ src/SerialConsole.cpp | 1 - 3 files changed, 3 insertions(+), 19 deletions(-) diff --git a/src/RedirectablePrint.cpp b/src/RedirectablePrint.cpp index 10fdd051d61..9e872dbd739 100644 --- a/src/RedirectablePrint.cpp +++ b/src/RedirectablePrint.cpp @@ -15,11 +15,6 @@ #include "platform/portduino/PortduinoGlue.h" #endif -/** - * A printer that doesn't go anywhere - */ -NoopPrint noopPrint; - #if HAS_WIFI || HAS_ETHERNET extern Syslog syslog; #endif @@ -215,7 +210,8 @@ size_t RedirectablePrint::log(const char *logLevel, const char *format, ...) va_list arg; va_start(arg, format); - log_to_serial(logLevel, format, arg); + if (!usingProtobufs) + log_to_serial(logLevel, format, arg); log_to_syslog(logLevel, format, arg); log_to_ble(logLevel, format, arg); diff --git a/src/RedirectablePrint.h b/src/RedirectablePrint.h index 23f83b825b3..53d23978ded 100644 --- a/src/RedirectablePrint.h +++ b/src/RedirectablePrint.h @@ -60,15 +60,4 @@ class RedirectablePrint : public Print void log_to_serial(const char *logLevel, const char *format, va_list arg); void log_to_syslog(const char *logLevel, const char *format, va_list arg); void log_to_ble(const char *logLevel, const char *format, va_list arg); -}; - -class NoopPrint : public Print -{ - public: - virtual size_t write(uint8_t c) { return 1; } -}; - -/** - * A printer that doesn't go anywhere - */ -extern NoopPrint noopPrint; \ No newline at end of file +}; \ No newline at end of file diff --git a/src/SerialConsole.cpp b/src/SerialConsole.cpp index 18d6c1f9f9b..68766bd1eb8 100644 --- a/src/SerialConsole.cpp +++ b/src/SerialConsole.cpp @@ -34,7 +34,6 @@ SerialConsole::SerialConsole() : StreamAPI(&Port), RedirectablePrint(&Port), con assert(!console); console = this; canWrite = false; // We don't send packets to our port until it has talked to us first - // setDestination(&noopPrint); for testing, try turning off 'all' debug output and see what leaks #ifdef RP2040_SLOW_CLOCK Port.setTX(SERIAL2_TX); From a94a5e661d1e25da584442dccffb87a0061c8aa1 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 16:24:26 -0700 Subject: [PATCH 3/9] when talking to API clients via serial, don't turn off log msgs instead encapsuate them --- src/RedirectablePrint.cpp | 3 +-- src/RedirectablePrint.h | 11 ++++------- src/SerialConsole.cpp | 28 ++++++++++++++++++++++++++++ src/SerialConsole.h | 10 +++++++++- src/mesh/StreamAPI.cpp | 19 +++++++++++++++++-- src/mesh/StreamAPI.h | 5 ++++- 6 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/RedirectablePrint.cpp b/src/RedirectablePrint.cpp index 9e872dbd739..60bef95bf42 100644 --- a/src/RedirectablePrint.cpp +++ b/src/RedirectablePrint.cpp @@ -210,8 +210,7 @@ size_t RedirectablePrint::log(const char *logLevel, const char *format, ...) va_list arg; va_start(arg, format); - if (!usingProtobufs) - log_to_serial(logLevel, format, arg); + log_to_serial(logLevel, format, arg); log_to_syslog(logLevel, format, arg); log_to_ble(logLevel, format, arg); diff --git a/src/RedirectablePrint.h b/src/RedirectablePrint.h index 53d23978ded..11a02f18736 100644 --- a/src/RedirectablePrint.h +++ b/src/RedirectablePrint.h @@ -23,12 +23,6 @@ class RedirectablePrint : public Print #else volatile bool inDebugPrint = false; #endif - protected: - /** - * If true we are talking to a smart host and all messages (including log messages) must be framed as protobufs. - */ - bool usingProtobufs = false; - public: explicit RedirectablePrint(Print *_dest) : dest(_dest) {} @@ -56,8 +50,11 @@ class RedirectablePrint : public Print std::string mt_sprintf(const std::string fmt_str, ...); + protected: + /// Subclasses can override if they need to change how we format over the serial port + virtual void log_to_serial(const char *logLevel, const char *format, va_list arg); + private: - void log_to_serial(const char *logLevel, const char *format, va_list arg); void log_to_syslog(const char *logLevel, const char *format, va_list arg); void log_to_ble(const char *logLevel, const char *format, va_list arg); }; \ No newline at end of file diff --git a/src/SerialConsole.cpp b/src/SerialConsole.cpp index 68766bd1eb8..26ece3d0191 100644 --- a/src/SerialConsole.cpp +++ b/src/SerialConsole.cpp @@ -88,4 +88,32 @@ bool SerialConsole::handleToRadio(const uint8_t *buf, size_t len) } else { return false; } +} + +void SerialConsole::log_to_serial(const char *logLevel, const char *format, va_list arg) +{ + if (usingProtobufs) { + meshtastic_LogRecord_Level ll = meshtastic_LogRecord_Level_UNSET; // default to unset + switch (logLevel[0]) { + case 'D': + ll = meshtastic_LogRecord_Level_DEBUG; + break; + case 'I': + ll = meshtastic_LogRecord_Level_INFO; + break; + case 'W': + ll = meshtastic_LogRecord_Level_WARNING; + break; + case 'E': + ll = meshtastic_LogRecord_Level_ERROR; + break; + case 'C': + ll = meshtastic_LogRecord_Level_CRITICAL; + break; + } + + auto thread = concurrency::OSThread::currentThread; + emitLogRecord(ll, thread ? thread->ThreadName.c_str() : "", format, arg); + } else + RedirectablePrint::log_to_serial(logLevel, format, arg); } \ No newline at end of file diff --git a/src/SerialConsole.h b/src/SerialConsole.h index f8891ba14fb..f1e636c9de7 100644 --- a/src/SerialConsole.h +++ b/src/SerialConsole.h @@ -8,6 +8,11 @@ */ class SerialConsole : public StreamAPI, public RedirectablePrint, private concurrency::OSThread { + /** + * If true we are talking to a smart host and all messages (including log messages) must be framed as protobufs. + */ + bool usingProtobufs = false; + public: SerialConsole(); @@ -31,10 +36,13 @@ class SerialConsole : public StreamAPI, public RedirectablePrint, private concur protected: /// Check the current underlying physical link to see if the client is currently connected virtual bool checkIsConnected() override; + + /// Possibly switch to protobufs if we see a valid protobuf message + virtual void log_to_serial(const char *logLevel, const char *format, va_list arg); }; // A simple wrapper to allow non class aware code write to the console void consolePrintf(const char *format, ...); void consoleInit(); -extern SerialConsole *console; +extern SerialConsole *console; \ No newline at end of file diff --git a/src/mesh/StreamAPI.cpp b/src/mesh/StreamAPI.cpp index 4d04dffe48f..84a7663921b 100644 --- a/src/mesh/StreamAPI.cpp +++ b/src/mesh/StreamAPI.cpp @@ -1,5 +1,6 @@ #include "StreamAPI.h" #include "PowerFSM.h" +#include "RTC.h" #include "configuration.h" #define START1 0x94 @@ -96,7 +97,6 @@ void StreamAPI::writeStream() void StreamAPI::emitTxBuffer(size_t len) { if (len != 0) { - // LOG_DEBUG("emit tx %d\n", len); txBuf[0] = START1; txBuf[1] = START2; txBuf[2] = (len >> 8) & 0xff; @@ -119,6 +119,21 @@ void StreamAPI::emitRebooted() emitTxBuffer(pb_encode_to_bytes(txBuf + HEADER_LEN, meshtastic_FromRadio_size, &meshtastic_FromRadio_msg, &fromRadioScratch)); } +void StreamAPI::emitLogRecord(meshtastic_LogRecord_Level level, const char *src, const char *format, va_list arg) +{ + // In case we send a FromRadio packet + memset(&fromRadioScratch, 0, sizeof(fromRadioScratch)); + fromRadioScratch.which_payload_variant = meshtastic_FromRadio_log_record_tag; + fromRadioScratch.log_record.level = level; + + uint32_t rtc_sec = getValidTime(RTCQuality::RTCQualityDevice, true); + fromRadioScratch.log_record.time = rtc_sec; + strncpy(fromRadioScratch.log_record.source, src, sizeof(fromRadioScratch.log_record.source)); + + vsnprintf(fromRadioScratch.log_record.message, sizeof(fromRadioScratch.log_record.message), format, arg); + emitTxBuffer(pb_encode_to_bytes(txBuf + HEADER_LEN, meshtastic_FromRadio_size, &meshtastic_FromRadio_msg, &fromRadioScratch)); +} + /// Hookable to find out when connection changes void StreamAPI::onConnectionChanged(bool connected) { @@ -131,4 +146,4 @@ void StreamAPI::onConnectionChanged(bool connected) // received a packet in a while powerFSM.trigger(EVENT_SERIAL_DISCONNECTED); } -} +} \ No newline at end of file diff --git a/src/mesh/StreamAPI.h b/src/mesh/StreamAPI.h index 3196e96f8b0..45cbb231c7b 100644 --- a/src/mesh/StreamAPI.h +++ b/src/mesh/StreamAPI.h @@ -82,4 +82,7 @@ class StreamAPI : public PhoneAPI /// Subclasses can use this scratch buffer if they wish uint8_t txBuf[MAX_STREAM_BUF_SIZE] = {0}; -}; + + /// Low level function to emit a protobuf encapsulated log record + void emitLogRecord(meshtastic_LogRecord_Level level, const char *src, const char *format, va_list arg); +}; \ No newline at end of file From c0482f5b6f3e1d032c61a0a7f5e98005eec31c1c Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 17:25:01 -0700 Subject: [PATCH 4/9] fix the build - would loop forever if there were no files to send --- src/mesh/PhoneAPI.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 57a42651acd..399715f6142 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -354,15 +354,14 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf) case STATE_SEND_FILEMANIFEST: { LOG_INFO("getFromRadio=STATE_SEND_FILEMANIFEST\n"); - fromRadioScratch.which_payload_variant = meshtastic_FromRadio_fileInfo_tag; - if (config_state < filesManifest.size()) { + // last element + if (config_state == filesManifest.size()) { // also handles an empty filesManifest + state = STATE_SEND_COMPLETE_ID; + config_state = 0; + } else { + fromRadioScratch.which_payload_variant = meshtastic_FromRadio_fileInfo_tag; fromRadioScratch.fileInfo = filesManifest.at(config_state); config_state++; - // last element - if (config_state == filesManifest.size()) { - state = STATE_SEND_COMPLETE_ID; - config_state = 0; - } } break; } From 43015449dca8b9e194833427a918e9dd6921f044 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 18:00:07 -0700 Subject: [PATCH 5/9] don't use Segger code if not talking to a Segger debugger --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 462eaa0f4eb..196eae525b6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -221,7 +221,7 @@ void setup() meshtastic_Config_DisplayConfig_OledType::meshtastic_Config_DisplayConfig_OledType_OLED_AUTO; OLEDDISPLAY_GEOMETRY screen_geometry = GEOMETRY_128_64; -#ifdef SEGGER_STDOUT_CH +#ifdef USE_SEGGER auto mode = false ? SEGGER_RTT_MODE_BLOCK_IF_FIFO_FULL : SEGGER_RTT_MODE_NO_BLOCK_TRIM; #ifdef NRF52840_XXAA auto buflen = 4096; // this board has a fair amount of ram From b69956e51834ba2d7a0f7fd88e416261135bb854 Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 18:59:18 -0700 Subject: [PATCH 6/9] when encapsulating logs, make sure the strings always has nul terminators --- src/mesh/StreamAPI.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesh/StreamAPI.cpp b/src/mesh/StreamAPI.cpp index 84a7663921b..d4f60bed1d7 100644 --- a/src/mesh/StreamAPI.cpp +++ b/src/mesh/StreamAPI.cpp @@ -128,9 +128,9 @@ void StreamAPI::emitLogRecord(meshtastic_LogRecord_Level level, const char *src, uint32_t rtc_sec = getValidTime(RTCQuality::RTCQualityDevice, true); fromRadioScratch.log_record.time = rtc_sec; - strncpy(fromRadioScratch.log_record.source, src, sizeof(fromRadioScratch.log_record.source)); + strncpy(fromRadioScratch.log_record.source, src, sizeof(fromRadioScratch.log_record.source) - 1); - vsnprintf(fromRadioScratch.log_record.message, sizeof(fromRadioScratch.log_record.message), format, arg); + vsnprintf(fromRadioScratch.log_record.message, sizeof(fromRadioScratch.log_record.message) - 1, format, arg); emitTxBuffer(pb_encode_to_bytes(txBuf + HEADER_LEN, meshtastic_FromRadio_size, &meshtastic_FromRadio_msg, &fromRadioScratch)); } From ca53564cb15460337256a2584b82e94290722e2b Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Thu, 27 Jun 2024 19:00:38 -0700 Subject: [PATCH 7/9] nrf52 soft device will watchdog if you use ICE while BT on... so have debugger disable bluetooth. --- src/platform/nrf52/main-nrf52.cpp | 3 ++- variants/rak4631/platformio.ini | 1 + variants/wio-sdk-wm1110/platformio.ini | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/platform/nrf52/main-nrf52.cpp b/src/platform/nrf52/main-nrf52.cpp index 86575bda6f6..b79f28f1393 100644 --- a/src/platform/nrf52/main-nrf52.cpp +++ b/src/platform/nrf52/main-nrf52.cpp @@ -63,7 +63,8 @@ static void initBrownout() // We don't bother with setting up brownout if soft device is disabled - because during production we always use softdevice } -static const bool useSoftDevice = true; // Set to false for easier debugging +// This is a public global so that the debugger can set it to false automatically from our gdbinit +bool useSoftDevice = true; // Set to false for easier debugging #if !MESHTASTIC_EXCLUDE_BLUETOOTH void setBluetoothEnable(bool enable) diff --git a/variants/rak4631/platformio.ini b/variants/rak4631/platformio.ini index beffa7d3dfa..6a67b008357 100644 --- a/variants/rak4631/platformio.ini +++ b/variants/rak4631/platformio.ini @@ -79,6 +79,7 @@ debug_extra_cmds = commands 1 echo Breakpoint at setup() has semihosting console, connect to it with "telnet localhost 5555" set wantSemihost = true + set useSoftDevice = false end diff --git a/variants/wio-sdk-wm1110/platformio.ini b/variants/wio-sdk-wm1110/platformio.ini index 7ca82e4c681..4a23e7a1197 100644 --- a/variants/wio-sdk-wm1110/platformio.ini +++ b/variants/wio-sdk-wm1110/platformio.ini @@ -15,5 +15,19 @@ build_src_filter = ${nrf52_base.build_src_filter} +<../variants/wio-sdk-wm1110> lib_deps = ${nrf52840_base.lib_deps} debug_tool = jlink +;debug_tool = stlink +;debug_speed = 4000 +; No need to reflash if the binary hasn't changed +debug_load_mode = modified ; If not set we will default to uploading over serial (first it forces bootloader entry by talking 1200bps to cdcacm) upload_protocol = jlink +;upload_protocol = stlink +; we prefer to stop in setup() because we are an 'ardiuno' app +debug_init_break = tbreak setup + +; we need to turn off BLE/soft device if we are debugging otherwise it will watchdog reset us. +debug_extra_cmds = + echo Running .gdbinit script + commands 1 + set useSoftDevice = false + end \ No newline at end of file From f37bed4faad23e083ff699f481fdba0e5a46fecc Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Fri, 28 Jun 2024 08:23:37 -0700 Subject: [PATCH 8/9] Important to not print debug messages while writing to the toPhone scratch buffer --- src/mesh/PhoneAPI.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 399715f6142..f6fad95aed3 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -410,7 +410,9 @@ size_t PhoneAPI::getFromRadio(uint8_t *buf) // Encapsulate as a FromRadio packet size_t numbytes = pb_encode_to_bytes(buf, meshtastic_FromRadio_size, &meshtastic_FromRadio_msg, &fromRadioScratch); - LOG_DEBUG("encoding toPhone packet to phone variant=%d, %d bytes\n", fromRadioScratch.which_payload_variant, numbytes); + // VERY IMPORTANT to not print debug messages while writing to fromRadioScratch - because we use that same buffer + // for logging (when we are encapsulating with protobufs) + // LOG_DEBUG("encoding toPhone packet to phone variant=%d, %d bytes\n", fromRadioScratch.which_payload_variant, numbytes); return numbytes; } From d5bcfc6233bdefa9e6e57f4d82b892530793db5a Mon Sep 17 00:00:00 2001 From: Kevin Hester Date: Fri, 28 Jun 2024 08:37:29 -0700 Subject: [PATCH 9/9] don't include newlines if encapsulating log records as protobufs --- src/mesh/StreamAPI.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mesh/StreamAPI.cpp b/src/mesh/StreamAPI.cpp index d4f60bed1d7..9f59aa971c3 100644 --- a/src/mesh/StreamAPI.cpp +++ b/src/mesh/StreamAPI.cpp @@ -130,7 +130,11 @@ void StreamAPI::emitLogRecord(meshtastic_LogRecord_Level level, const char *src, fromRadioScratch.log_record.time = rtc_sec; strncpy(fromRadioScratch.log_record.source, src, sizeof(fromRadioScratch.log_record.source) - 1); - vsnprintf(fromRadioScratch.log_record.message, sizeof(fromRadioScratch.log_record.message) - 1, format, arg); + auto num_printed = + vsnprintf(fromRadioScratch.log_record.message, sizeof(fromRadioScratch.log_record.message) - 1, format, arg); + if (num_printed > 0 && fromRadioScratch.log_record.message[num_printed - 1] == + '\n') // Strip any ending newline, because we have records for framing instead. + fromRadioScratch.log_record.message[num_printed - 1] = '\0'; emitTxBuffer(pb_encode_to_bytes(txBuf + HEADER_LEN, meshtastic_FromRadio_size, &meshtastic_FromRadio_msg, &fromRadioScratch)); }