Faster app_settings save (#1145)

* No-allocation settings save

* remove fixed TODO

---------

Co-authored-by: kallanreed <kallanreed@outlook.com>

firmware/application/app_settings.cpp

@@ -66,7 +66,15 @@ static void read_setting(
 
 template <typename T>
 static void write_setting(File& file, std::string_view setting_name, const T& value) {
-    file.write_line(std::string{setting_name} + to_string_dec_uint(value));
+    // NB: Not using file.write_line speed this up. This happens on every
+    // app exit when enabled so should be fast to keep the UX responsive.
+    StringFormatBuffer buffer;
+    size_t length = 0;
+    auto value_str = to_string_dec_uint(value, buffer, length);
+
+    file.write(setting_name.data(), setting_name.length());
+    file.write(value_str, length);
+    file.write("\r\n", 2);
 }
 
 static fs::path get_settings_path(const std::string& app_name) {
@@ -101,7 +109,6 @@ constexpr std::string_view volume = "volume="sv;
 //       be declaratively bound to a setting and persistence will be magic.
 // TODO: radio settings should be pushed and popped to prevent cross-app
 //       radio bugs caused by sharing a global model.
-// TODO: save is slow because of all of the allocations for File.write_line.
 
 ResultCode load_settings(const std::string& app_name, AppSettings& settings) {
     if (!portapack::persistent_memory::load_app_settings())

firmware/application/string_format.cpp

@@ -21,6 +21,10 @@
 
 #include "string_format.hpp"
 
+/* This takes a pointer to the end of a buffer
+ * and fills it backwards towards the front.
+ * The return value 'q' is a pointer to the start.
+ * TODO: use std::array for all this. */
 static char* to_string_dec_uint_internal(
     char* p,
     uint32_t n) {
@@ -44,6 +48,9 @@ static char* to_string_dec_uint_pad_internal(
     const char fill) {
     auto q = to_string_dec_uint_internal(term, n);
 
+    // Fill with padding if needed.
+    // TODO: use std::array instead. There's no
+    // bounds checks on any of this!
     if (fill) {
         while ((term - q) < l) {
             *(--q) = fill;
@@ -53,6 +60,13 @@ static char* to_string_dec_uint_pad_internal(
     return q;
 }
 
+char* to_string_dec_uint(const uint32_t n, StringFormatBuffer& buffer, size_t& length) {
+    auto end = &buffer.back();
+    auto start = to_string_dec_uint_internal(end, n);
+    length = end - start;
+    return start;
+}
+
 std::string to_string_bin(
     const uint32_t n,
     const uint8_t l) {
@@ -75,6 +89,7 @@ std::string to_string_dec_uint(
     auto term = p + sizeof(p) - 1;
     auto q = to_string_dec_uint_pad_internal(term, n, l, fill);
 
+    // TODO: is this needed, seems like pad already does this.
     // Right justify.
     while ((term - q) < l) {
         *(--q) = ' ';

firmware/application/string_format.hpp

@@ -23,6 +23,7 @@
 #define __STRING_FORMAT_H__
 
 #include <cstdint>
+#include <array>
 #include <string>
 
 #include "file.hpp"
@@ -39,6 +40,11 @@ enum TimeFormat {
 
 const char unit_prefix[7]{'n', 'u', 'm', 0, 'k', 'M', 'G'};
 
+using StringFormatBuffer = std::array<char, 16>;
+
+/* uint32_t conversion without memory allocations. */
+char* to_string_dec_uint(const uint32_t n, StringFormatBuffer& buffer, size_t& length);
+
 // TODO: Allow l=0 to not fill/justify? Already using this way in ui_spectrum.hpp...
 std::string to_string_bin(const uint32_t n, const uint8_t l = 0);
 std::string to_string_dec_uint(const uint32_t n, const int32_t l = 0, const char fill = ' ');