From c00f4f480a4913b21a6edbc92715ae4711336b00 Mon Sep 17 00:00:00 2001 From: Bertrand Lemasle Date: Sat, 4 Aug 2018 20:06:44 +1200 Subject: [PATCH 1/4] Using a PROGMEM format string for http body formatting --- GpsTracker/NetworkPositionsBackup.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GpsTracker/NetworkPositionsBackup.cpp b/GpsTracker/NetworkPositionsBackup.cpp index fdc8633..5ec6c1e 100644 --- a/GpsTracker/NetworkPositionsBackup.cpp +++ b/GpsTracker/NetworkPositionsBackup.cpp @@ -26,7 +26,7 @@ namespace positions { bool NetworkPositionsBackup::appendPosition(PositionEntry &entry) { char buffer[BUFFER_SIZE]; - snprintf(buffer, BUFFER_SIZE, "%d,%d,%d,%d,%d,%d,%d,", + snprintf_P(buffer, BUFFER_SIZE, PSTR("%d,%d,%d,%d,%d,%d,%d,"), debug::freeRam(), hardware::sim808::device.getSignalQuality().attenuation, entry.metadata.batteryLevel, From 5aa4eed1744362aed1bc010b02cd6d6b99a71717 Mon Sep 17 00:00:00 2001 From: Bertrand Lemasle Date: Sat, 4 Aug 2018 20:13:43 +1200 Subject: [PATCH 2/4] Using a common buffer for both http and sms --- GpsTracker/Buffer.cpp | 9 +++++++++ GpsTracker/Buffer.h | 11 +++++++++++ GpsTracker/Core.cpp | 20 +++++++++++--------- GpsTracker/NetworkPositionsBackup.cpp | 14 +++++++------- 4 files changed, 38 insertions(+), 16 deletions(-) create mode 100644 GpsTracker/Buffer.cpp create mode 100644 GpsTracker/Buffer.h diff --git a/GpsTracker/Buffer.cpp b/GpsTracker/Buffer.cpp new file mode 100644 index 0000000..c119b2f --- /dev/null +++ b/GpsTracker/Buffer.cpp @@ -0,0 +1,9 @@ +#include "Buffer.h" +#include "string.h" + + +namespace buffer { + char value[BUFFER_SIZE]; + + void clear() { memset(buffer::value, NULL, BUFFER_SIZE); } +} \ No newline at end of file diff --git a/GpsTracker/Buffer.h b/GpsTracker/Buffer.h new file mode 100644 index 0000000..8b92e30 --- /dev/null +++ b/GpsTracker/Buffer.h @@ -0,0 +1,11 @@ +#pragma once + +#define BUFFER_SIZE 170 + + +namespace buffer { + extern char value[BUFFER_SIZE]; + + void clear(); + +} \ No newline at end of file diff --git a/GpsTracker/Core.cpp b/GpsTracker/Core.cpp index 60384ca..b04ec30 100644 --- a/GpsTracker/Core.cpp +++ b/GpsTracker/Core.cpp @@ -2,9 +2,10 @@ #include "Config.h" #include "Flash.h" #include "Alerts.h" +#include "Buffer.h" #define LOGGER_NAME "Core" -#define SMS_BUFFER_SIZE 140 +#define SMS_MAX_SIZE 140 #define NO_ALERTS_NOTIFIED 0 using namespace utils; @@ -16,12 +17,12 @@ namespace core { namespace details { - void appendToSmsBuffer(char * buffer, const char * fmt, ...) { + void appendToSmsBuffer(const char * fmt, ...) { va_list args; va_start(args, fmt); - size_t bufferLeft = SMS_BUFFER_SIZE - strlen(buffer); - char * p = buffer + strlen(buffer); + size_t bufferLeft = SMS_MAX_SIZE - strlen(buffer::value); + char * p = buffer::value + strlen(buffer::value); vsnprintf_P(p, bufferLeft, fmt, args); va_end(args); @@ -54,7 +55,6 @@ namespace core { uint8_t notifyFailures(PositionEntryMetadata &metadata) { SIM808RegistrationStatus networkStatus; - char buffer[SMS_BUFFER_SIZE] = "Alerts !\n"; const __FlashStringHelper * backupFailureString = F(" Backup battery failure ?\n"); uint8_t triggered = alerts::getTriggered(metadata); @@ -65,19 +65,21 @@ namespace core { if (!network::isAvailable(networkStatus.stat)) return NO_ALERTS_NOTIFIED; + buffer::clear(); + strcpy_P(buffer::value, PSTR("Alerts !\n")); if (bitRead(triggered, ALERT_BATTERY_LEVEL_1) || bitRead(triggered, ALERT_BATTERY_LEVEL_2)) { - details::appendToSmsBuffer(buffer, PSTR("- Battery at %d%%.\n"), metadata.batteryLevel); + details::appendToSmsBuffer(PSTR("- Battery at %d%%.\n"), metadata.batteryLevel); } if (bitRead(triggered, ALERT_RTC_CLOCK_FAILURE)) { - details::appendToSmsBuffer(buffer, PSTR("-RTC was stopped. %S"), backupFailureString); + details::appendToSmsBuffer(PSTR("-RTC was stopped. %S"), backupFailureString); } if (bitRead(triggered, ALERT_RTC_TEMPERATURE_FAILURE)) { - details::appendToSmsBuffer(buffer, PSTR("- Temperature is %dC. %S"), static_cast(metadata.temperature * 100), backupFailureString); + details::appendToSmsBuffer(PSTR("- Temperature is %dC. %S"), static_cast(metadata.temperature * 100), backupFailureString); } - bool notified = network::sendSms(buffer); + bool notified = network::sendSms(buffer::value); if (!notified) NOTICE_MSG("notifyFailure", "SMS not sent !"); network::powerOff(); diff --git a/GpsTracker/NetworkPositionsBackup.cpp b/GpsTracker/NetworkPositionsBackup.cpp index 5ec6c1e..95f8651 100644 --- a/GpsTracker/NetworkPositionsBackup.cpp +++ b/GpsTracker/NetworkPositionsBackup.cpp @@ -7,9 +7,9 @@ #include "Debug.h" #include "Hardware.h" #include "Network.h" +#include "Buffer.h" #define LOGGER_NAME "Positions::backup::network" -#define BUFFER_SIZE 170 namespace positions { namespace backup { @@ -25,8 +25,8 @@ namespace positions { } bool NetworkPositionsBackup::appendPosition(PositionEntry &entry) { - char buffer[BUFFER_SIZE]; - snprintf_P(buffer, BUFFER_SIZE, PSTR("%d,%d,%d,%d,%d,%d,%d,"), + buffer::clear(); + snprintf_P(buffer::value, BUFFER_SIZE, PSTR("%d,%d,%d,%d,%d,%d,%d,"), debug::freeRam(), hardware::sim808::device.getSignalQuality().attenuation, entry.metadata.batteryLevel, @@ -35,14 +35,14 @@ namespace positions { static_cast(entry.metadata.status), entry.metadata.timeToFix); - strcat(buffer, entry.position); + strcat(buffer::value, entry.position); - NOTICE_FORMAT("appendPosition", "Sending : %s", buffer); + NOTICE_FORMAT("appendPosition", "Sending : %s", buffer::value); uint16_t responseCode = hardware::sim808::device.httpPost( config::main::value.network.url, F("text/gpstracker"), - buffer, - buffer, + buffer::value, + buffer::value, BUFFER_SIZE ) == POSITIONS_CONFIG_NET_DEFAULT_EXPECTED_RESPONSE; From 94d0b53a8eff2419a465fca57bf9a9c925cf4075 Mon Sep 17 00:00:00 2001 From: Bertrand Lemasle Date: Sat, 4 Aug 2018 21:15:26 +1200 Subject: [PATCH 3/4] Revert "Using a common buffer for both http and sms" This reverts commit 3cfc7074d1ad15a3ea13c2eef45c2aac6e3d161e. Common buffer implies that the buffer goes to static memory, which overflow the available SRAM. --- GpsTracker/Buffer.cpp | 9 --------- GpsTracker/Buffer.h | 11 ----------- GpsTracker/Core.cpp | 20 +++++++++----------- GpsTracker/NetworkPositionsBackup.cpp | 14 +++++++------- 4 files changed, 16 insertions(+), 38 deletions(-) delete mode 100644 GpsTracker/Buffer.cpp delete mode 100644 GpsTracker/Buffer.h diff --git a/GpsTracker/Buffer.cpp b/GpsTracker/Buffer.cpp deleted file mode 100644 index c119b2f..0000000 --- a/GpsTracker/Buffer.cpp +++ /dev/null @@ -1,9 +0,0 @@ -#include "Buffer.h" -#include "string.h" - - -namespace buffer { - char value[BUFFER_SIZE]; - - void clear() { memset(buffer::value, NULL, BUFFER_SIZE); } -} \ No newline at end of file diff --git a/GpsTracker/Buffer.h b/GpsTracker/Buffer.h deleted file mode 100644 index 8b92e30..0000000 --- a/GpsTracker/Buffer.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -#define BUFFER_SIZE 170 - - -namespace buffer { - extern char value[BUFFER_SIZE]; - - void clear(); - -} \ No newline at end of file diff --git a/GpsTracker/Core.cpp b/GpsTracker/Core.cpp index b04ec30..60384ca 100644 --- a/GpsTracker/Core.cpp +++ b/GpsTracker/Core.cpp @@ -2,10 +2,9 @@ #include "Config.h" #include "Flash.h" #include "Alerts.h" -#include "Buffer.h" #define LOGGER_NAME "Core" -#define SMS_MAX_SIZE 140 +#define SMS_BUFFER_SIZE 140 #define NO_ALERTS_NOTIFIED 0 using namespace utils; @@ -17,12 +16,12 @@ namespace core { namespace details { - void appendToSmsBuffer(const char * fmt, ...) { + void appendToSmsBuffer(char * buffer, const char * fmt, ...) { va_list args; va_start(args, fmt); - size_t bufferLeft = SMS_MAX_SIZE - strlen(buffer::value); - char * p = buffer::value + strlen(buffer::value); + size_t bufferLeft = SMS_BUFFER_SIZE - strlen(buffer); + char * p = buffer + strlen(buffer); vsnprintf_P(p, bufferLeft, fmt, args); va_end(args); @@ -55,6 +54,7 @@ namespace core { uint8_t notifyFailures(PositionEntryMetadata &metadata) { SIM808RegistrationStatus networkStatus; + char buffer[SMS_BUFFER_SIZE] = "Alerts !\n"; const __FlashStringHelper * backupFailureString = F(" Backup battery failure ?\n"); uint8_t triggered = alerts::getTriggered(metadata); @@ -65,21 +65,19 @@ namespace core { if (!network::isAvailable(networkStatus.stat)) return NO_ALERTS_NOTIFIED; - buffer::clear(); - strcpy_P(buffer::value, PSTR("Alerts !\n")); if (bitRead(triggered, ALERT_BATTERY_LEVEL_1) || bitRead(triggered, ALERT_BATTERY_LEVEL_2)) { - details::appendToSmsBuffer(PSTR("- Battery at %d%%.\n"), metadata.batteryLevel); + details::appendToSmsBuffer(buffer, PSTR("- Battery at %d%%.\n"), metadata.batteryLevel); } if (bitRead(triggered, ALERT_RTC_CLOCK_FAILURE)) { - details::appendToSmsBuffer(PSTR("-RTC was stopped. %S"), backupFailureString); + details::appendToSmsBuffer(buffer, PSTR("-RTC was stopped. %S"), backupFailureString); } if (bitRead(triggered, ALERT_RTC_TEMPERATURE_FAILURE)) { - details::appendToSmsBuffer(PSTR("- Temperature is %dC. %S"), static_cast(metadata.temperature * 100), backupFailureString); + details::appendToSmsBuffer(buffer, PSTR("- Temperature is %dC. %S"), static_cast(metadata.temperature * 100), backupFailureString); } - bool notified = network::sendSms(buffer::value); + bool notified = network::sendSms(buffer); if (!notified) NOTICE_MSG("notifyFailure", "SMS not sent !"); network::powerOff(); diff --git a/GpsTracker/NetworkPositionsBackup.cpp b/GpsTracker/NetworkPositionsBackup.cpp index 95f8651..5ec6c1e 100644 --- a/GpsTracker/NetworkPositionsBackup.cpp +++ b/GpsTracker/NetworkPositionsBackup.cpp @@ -7,9 +7,9 @@ #include "Debug.h" #include "Hardware.h" #include "Network.h" -#include "Buffer.h" #define LOGGER_NAME "Positions::backup::network" +#define BUFFER_SIZE 170 namespace positions { namespace backup { @@ -25,8 +25,8 @@ namespace positions { } bool NetworkPositionsBackup::appendPosition(PositionEntry &entry) { - buffer::clear(); - snprintf_P(buffer::value, BUFFER_SIZE, PSTR("%d,%d,%d,%d,%d,%d,%d,"), + char buffer[BUFFER_SIZE]; + snprintf_P(buffer, BUFFER_SIZE, PSTR("%d,%d,%d,%d,%d,%d,%d,"), debug::freeRam(), hardware::sim808::device.getSignalQuality().attenuation, entry.metadata.batteryLevel, @@ -35,14 +35,14 @@ namespace positions { static_cast(entry.metadata.status), entry.metadata.timeToFix); - strcat(buffer::value, entry.position); + strcat(buffer, entry.position); - NOTICE_FORMAT("appendPosition", "Sending : %s", buffer::value); + NOTICE_FORMAT("appendPosition", "Sending : %s", buffer); uint16_t responseCode = hardware::sim808::device.httpPost( config::main::value.network.url, F("text/gpstracker"), - buffer::value, - buffer::value, + buffer, + buffer, BUFFER_SIZE ) == POSITIONS_CONFIG_NET_DEFAULT_EXPECTED_RESPONSE; From 56fa127bb75a38296b3f21264b1eda7fdca62fa4 Mon Sep 17 00:00:00 2001 From: Bertrand Lemasle Date: Sat, 4 Aug 2018 21:17:55 +1200 Subject: [PATCH 4/4] Using a PROGMEM string for sms buffer init. Side effects : avoid gcc optimization that declares the buffer as a global variable, which overflow the available SRAM --- GpsTracker/Core.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/GpsTracker/Core.cpp b/GpsTracker/Core.cpp index 60384ca..ecb9f37 100644 --- a/GpsTracker/Core.cpp +++ b/GpsTracker/Core.cpp @@ -54,7 +54,7 @@ namespace core { uint8_t notifyFailures(PositionEntryMetadata &metadata) { SIM808RegistrationStatus networkStatus; - char buffer[SMS_BUFFER_SIZE] = "Alerts !\n"; + char buffer[SMS_BUFFER_SIZE]; const __FlashStringHelper * backupFailureString = F(" Backup battery failure ?\n"); uint8_t triggered = alerts::getTriggered(metadata); @@ -65,6 +65,7 @@ namespace core { if (!network::isAvailable(networkStatus.stat)) return NO_ALERTS_NOTIFIED; + details::appendToSmsBuffer(buffer, PSTR("Alerts !\n")); if (bitRead(triggered, ALERT_BATTERY_LEVEL_1) || bitRead(triggered, ALERT_BATTERY_LEVEL_2)) { details::appendToSmsBuffer(buffer, PSTR("- Battery at %d%%.\n"), metadata.batteryLevel); }