added version markers to settings, option to disable INI comments

framework/settings.c

@@ -2,6 +2,7 @@
 // Created by MightyPork on 2017/11/26.
 //
 
+#include <utils/avrlibc.h>
 #include "platform.h"
 #include "utils/hexdump.h"
 #include "settings.h"
@@ -9,9 +10,13 @@
 #include "system_settings.h"
 #include "utils/str_utils.h"
 
+// pre-declarations
+static void savebuf_flush(PayloadBuilder *pb, bool final);
+static bool savebuf_ovhandler(PayloadBuilder *pb, uint32_t more);
+
 // This is the first entry in a valid config.
 // Change with each breaking change to force config reset.
-#define CONFIG_MARKER 0xA55C
+#define CONFIG_MARKER 0xA55D
 
 void settings_load(void)
 {
@@ -32,6 +37,11 @@ void settings_load(void)
         return;
     }
 
+    uint8_t version = pp_u8(&pp); // top level settings format version
+
+    { // Settings
+        (void)version; // Conditional choices based on version
+
         // System section
         if (!systemsettings_load(&pp)) {
             dbg("!! System settings failed to load");
@@ -42,6 +52,7 @@ void settings_load(void)
             dbg("!! Unit settings failed to load");
             return;
         }
+    } // End settings
 
     dbg("System settings loaded OK");
 }
@@ -56,65 +67,6 @@ static uint32_t save_addr;
 #define fls_printf(fmt, ...) do {} while (0)
 #endif
 
-/**
- * Flush the save buffer to flash, moving leftovers from uneven half-words
- * to the beginning and adjusting the CWPack curent pointer accordingly.
- *
- * @param ctx - pack context
- * @param final - if true, flush uneven leftovers; else move them to the beginning and keep for next call.
- */
-static void savebuf_flush(PayloadBuilder *pb, bool final)
-{
-    // TODO this might be buggy, was not tested cross-boundary yet
-    // TODO remove those printf's after verifying correctness
-
-    uint32_t bytes = (uint32_t) pb_length(pb);
-
-    // Dump what we're flushing
-    fls_printf("Flush: ");
-    for (uint32_t i = 0; i < bytes; i++) {
-        fls_printf("%02X ", save_buffer[i]);
-    }
-    fls_printf("\r\n");
-
-    uint32_t halfwords = bytes >> 1;
-    uint32_t remain = bytes & 1; // how many bytes won't be programmed
-
-    fls_printf("Halfwords: %d, Remain: %d, last? %d\r\n", (int)halfwords, (int)remain, final);
-
-    uint16_t *hwbuf = (void*) &save_buffer[0];
-
-    for (; halfwords > 0; halfwords--) {
-        uint16_t hword = *hwbuf++;
-
-        fls_printf("%04X ", hword);
-
-        HAL_StatusTypeDef res = HAL_FLASH_Program(FLASH_TYPEPROGRAM_HALFWORD, save_addr, hword);
-        assert_param(HAL_OK == res);
-        save_addr += 2; // advance
-    }
-
-    // rewind the context buffer
-    pb->current = pb->start;
-
-    if (remain) {
-        // We have an odd byte left to write
-        if (final) {
-            // We're done writing, this is the last call. Append the last byte to flash.
-            uint16_t hword = save_buffer[bytes-1];
-            fls_printf("& %02X ", hword);
-
-            HAL_StatusTypeDef res = HAL_FLASH_Program(FLASH_TYPEPROGRAM_HALFWORD, save_addr, hword);
-            assert_param(HAL_OK == res);
-        } else {
-            // Move the leftover to the beginning of the buffer for next call.
-            save_buffer[0] = save_buffer[bytes-1];
-            pb->current++;
-        }
-    }
-    fls_printf("\r\n");
-}
-
 /**
  * Save buffer overflow handler.
  * This should flush whatever is in the buffer and let CWPack continue
@@ -130,7 +82,7 @@ static bool savebuf_ovhandler(PayloadBuilder *pb, uint32_t more)
     return true;
 }
 
-// Save settings to flash
+/** Save settings to flash */
 void settings_save(void)
 {
     HAL_StatusTypeDef hst;
@@ -173,10 +125,15 @@ void settings_save(void)
 
         // Marker that this is a valid save
         pb_u16(&pb, CONFIG_MARKER);
+        pb_u8(&pb, 0); // Settings format version
+
+        { // Settings
             fls_printf("Saving system settings\r\n");
             systemsettings_save(&pb);
+
             fls_printf("Saving units\r\n");
             ureg_save_units(&pb);
+        } // End settings
 
         fls_printf("Final flush\r\n");
         savebuf_flush(&pb, true);
@@ -193,18 +150,80 @@ void settings_save(void)
 #endif
 }
 
+/**
+ * Flush the save buffer to flash, moving leftovers from uneven half-words
+ * to the beginning and adjusting the CWPack curent pointer accordingly.
+ *
+ * @param ctx - pack context
+ * @param final - if true, flush uneven leftovers; else move them to the beginning and keep for next call.
+ */
+static void savebuf_flush(PayloadBuilder *pb, bool final)
+{
+    // TODO this might be buggy, was not tested cross-boundary yet
+    // TODO remove those printf's after verifying correctness
+
+    uint32_t bytes = (uint32_t) pb_length(pb);
+
+    // Dump what we're flushing
+    fls_printf("Flush: ");
+    for (uint32_t i = 0; i < bytes; i++) {
+        fls_printf("%02X ", save_buffer[i]);
+    }
+    fls_printf("\r\n");
+
+    uint32_t halfwords = bytes >> 1;
+    uint32_t remain = bytes & 1; // how many bytes won't be programmed
+
+    fls_printf("Halfwords: %d, Remain: %d, last? %d\r\n", (int)halfwords, (int)remain, final);
+
+    uint16_t *hwbuf = (void*) &save_buffer[0];
+
+    for (; halfwords > 0; halfwords--) {
+        uint16_t hword = *hwbuf++;
+
+        fls_printf("%04X ", hword);
+
+        HAL_StatusTypeDef res = HAL_FLASH_Program(FLASH_TYPEPROGRAM_HALFWORD, save_addr, hword);
+        assert_param(HAL_OK == res);
+        save_addr += 2; // advance
+    }
+
+    // rewind the context buffer
+    pb->current = pb->start;
+
+    if (remain) {
+        // We have an odd byte left to write
+        if (final) {
+            // We're done writing, this is the last call. Append the last byte to flash.
+            uint16_t hword = save_buffer[bytes-1];
+            fls_printf("& %02X ", hword);
+
+            HAL_StatusTypeDef res = HAL_FLASH_Program(FLASH_TYPEPROGRAM_HALFWORD, save_addr, hword);
+            assert_param(HAL_OK == res);
+        } else {
+            // Move the leftover to the beginning of the buffer for next call.
+            save_buffer[0] = save_buffer[bytes-1];
+            pb->current++;
+        }
+    }
+    fls_printf("\r\n");
+}
+
 /**
  * Write system settings to INI (without section)
  */
 void settings_build_ini(IniWriter *iw)
 {
     // File header
-    iw_comment(iw, "CONFIG.INI");
+    iw_hdr_comment(iw, "CONFIG.INI");
+    iw_hdr_comment(iw, "GEX v%s on %s", GEX_VERSION, GEX_PLATFORM);
+    iw_hdr_comment(iw, "built %s at %s", __DATE__, __TIME__);
+    iw_cmt_newline(iw);
+
     iw_comment(iw, "Overwrite this file to change settings.");
     iw_comment(iw, "Close the LOCK jumper to save them to Flash.");
 
     systemsettings_build_ini(iw);
-    iw_newline(iw);
 
     ureg_build_ini(iw);
 }
@@ -221,7 +240,8 @@ void settings_load_ini_begin(void)
 
 void settings_load_ini_key(const char *restrict section, const char *restrict key, const char *restrict value)
 {
-//    dbg("[%s] %s = %s", section, key, value);
+    dbg("[%s] %s = %s", section, key, value);
+    static char namebuf[INI_KEY_MAX];
 
     if (streq(section, "SYSTEM")) {
         // system is always at the top
@@ -235,8 +255,15 @@ void settings_load_ini_key(const char *restrict section, const char *restrict ke
         // not a standard section, may be some unit config
         // all unit sections contain the colon character [TYPE:NAME]
         const char *nameptr = strchr(section, ':');
-        if (nameptr) {
-            ureg_load_unit_ini_key(nameptr + 1, key, value);
+        const char *csptr = strchr(section, '@');
+
+        dbg("cs = %s", csptr);
+        if (nameptr && csptr) {
+            strncpy(namebuf, nameptr+1, csptr - nameptr - 1);
+            uint8_t cs = (uint8_t) avr_atoi(csptr + 1);
+            dbg("cs is %d", cs);
+            dbg("name is %s", namebuf);
+            ureg_load_unit_ini_key(namebuf, key, value, cs);
         } else {
             dbg("! Bad config key: [%s] %s = %s", section, key, value);
         }

framework/system_settings.c

@@ -3,8 +3,8 @@
 //
 
 #include "platform.h"
-#include "utils/str_utils.h"
 #include "system_settings.h"
+#include "utils/str_utils.h"
 #include "platform/lock_jumper.h"
 
 struct system_settings SystemSettings;
@@ -13,6 +13,7 @@ struct system_settings SystemSettings;
 void systemsettings_loadDefaults(void)
 {
     SystemSettings.visible_vcom = true;
+    SystemSettings.ini_comments = true;
 }
 
 /** Load defaults and init flags */
@@ -29,14 +30,27 @@ void systemsettings_init(void)
 void systemsettings_save(PayloadBuilder *pb)
 {
     pb_char(pb, 'S');
+    pb_u8(pb, 0); // settings format version
+
+    { // system settings
         pb_bool(pb, SystemSettings.visible_vcom);
+        pb_bool(pb, SystemSettings.ini_comments);
+    } // end system settings
 }
 
 // from binary
 bool systemsettings_load(PayloadParser *pp)
 {
     if (pp_char(pp) != 'S') return false;
+    uint8_t version = pp_u8(pp);
+
+    { // system settings
         SystemSettings.visible_vcom = pp_bool(pp);
+        SystemSettings.ini_comments = pp_bool(pp);
+
+        // conditional fields based on version
+        (void) version;
+    } // end system settings
 
     return pp->ok;
 }
@@ -48,8 +62,12 @@ bool systemsettings_load(PayloadParser *pp)
 void systemsettings_build_ini(IniWriter *iw)
 {
     iw_section(iw, "SYSTEM");
+
     iw_comment(iw, "Data link accessible as virtual comport (Y, N)");
     iw_entry(iw, "expose_vcom", str_yn(SystemSettings.visible_vcom));
+
+    iw_comment(iw, "Show comments in INI files (Y, N)");
+    iw_entry(iw, "ini_comments", str_yn(SystemSettings.ini_comments));
 }
 
 /**
@@ -63,5 +81,10 @@ bool systemsettings_load_ini(const char *restrict key, const char *restrict valu
         if (suc) SystemSettings.visible_vcom = yn;
     }
 
+    if (streq(key, "ini_comments")) {
+        bool yn = str_parse_yn(value, &suc);
+        if (suc) SystemSettings.ini_comments = yn;
+    }
+
     return suc;
 }

framework/system_settings.h

@@ -12,6 +12,7 @@
 
 struct system_settings {
     bool visible_vcom;
+    bool ini_comments;
 
     // Support flags put here for scoping, but not atcually part of the persistent settings
     volatile bool editable; //!< True if we booted with the LOCK jumper removed

framework/unit_registry.c

@@ -2,14 +2,15 @@
 // Created by MightyPork on 2017/11/26.
 //
 
-#include <utils/hexdump.h>
 #include "platform.h"
+#include "utils/hexdump.h"
 #include "utils/avrlibc.h"
 #include "comm/messages.h"
 #include "utils/ini_writer.h"
 #include "utils/str_utils.h"
 #include "utils/malloc_safe.h"
 #include "unit_registry.h"
+#include "system_settings.h"
 #include "resources.h"
 
 // ** Unit repository **
@@ -212,12 +213,16 @@ void ureg_save_units(PayloadBuilder *pb)
     uint32_t count = ureg_get_num_units();
 
     pb_char(pb, 'U');
+    pb_u8(pb, 0); // Format version
+
+    { // Units list
         pb_u16(pb, (uint16_t) count);
 
         UlistEntry *le = ulist_head;
         while (le != NULL) {
             Unit *const pUnit = &le->unit;
-        pb_char(pb, 'u');
+            pb_char(pb, 'u'); // marker
+            { // Single unit
                 pb_string(pb, pUnit->driver->name);
                 pb_string(pb, pUnit->name);
                 pb_u8(pb, pUnit->callsign);
@@ -225,8 +230,10 @@ void ureg_save_units(PayloadBuilder *pb)
                 // Now all the rest, unit-specific
                 pUnit->driver->cfgWriteBinary(pUnit, pb);
                 assert_param(pb->ok);
+            } // end single unit
             le = le->next;
         }
+    } // end units list
 }
 
 bool ureg_load_units(PayloadParser *pp)
@@ -236,7 +243,13 @@ bool ureg_load_units(PayloadParser *pp)
 
     assert_param(pp->ok);
 
+    // Check units list marker byte
     if (pp_char(pp) != 'U') return false;
+    uint8_t version = pp_u8(pp); // units list format version
+
+    (void)version; // version can affect the format
+
+    { // units list
         uint16_t unit_count = pp_u16(pp);
         dbg("Units to load: %d", (int) unit_count);
 
@@ -246,6 +259,7 @@ bool ureg_load_units(PayloadParser *pp)
             // Marker that this is a unit - it could get out of alignment if structure changed
             if (pp_char(pp) != 'u') return false;
 
+            { // Single unit
                 // TYPE
                 pp_string(pp, typebuf, 16);
                 Unit *const pUnit = ureg_instantiate(typebuf);
@@ -273,7 +287,9 @@ bool ureg_load_units(PayloadParser *pp)
                 if (pUnit->status == E_LOADING) {
                     pUnit->status = suc ? E_SUCCESS : E_BAD_CONFIG;
                 }
+            } // end unit
         }
+    } // end units list
 
     return pp->ok;
 }
@@ -298,7 +314,7 @@ void ureg_remove_all_units(void)
     ulist_head = ulist_tail = NULL;
 }
 
-
+/** Create unit instances from the [UNITS] overview section */
 bool ureg_instantiate_by_ini(const char *restrict driver_name, const char *restrict names)
 {
     UregEntry *re = ureg_head;
@@ -342,29 +358,26 @@ bool ureg_instantiate_by_ini(const char *restrict driver_name, const char *restr
     return false;
 }
 
+/** Load unit key-value */
 bool ureg_load_unit_ini_key(const char *restrict name,
                             const char *restrict key,
-                            const char *restrict value)
+                            const char *restrict value,
+                            uint8_t callsign)
 {
     UlistEntry *li = ulist_head;
     while (li != NULL) {
         if (streq(li->unit.name, name)) {
             Unit *const pUnit = &li->unit;
-
-            if (streq(key, "callsign")) {
-                // handled separately from unit data
-                pUnit->callsign = (uint8_t) avr_atoi(value);
-                return true;
-            } else {
+            pUnit->callsign = callsign;
             return pUnit->driver->cfgLoadIni(pUnit, key, value);
         }
-        }
 
         li = li->next;
     }
     return false;
 }
 
+/** Finalize untis init */
 bool ureg_finalize_all_init(void)
 {
     dbg("Finalizing units init...");
@@ -404,17 +417,11 @@ static void export_unit_do(UlistEntry *li, IniWriter *iw)
 {
     Unit *const pUnit = &li->unit;
 
-    iw_section(iw, "%s:%s", pUnit->driver->name, pUnit->name);
+    iw_section(iw, "%s:%s@%d", pUnit->driver->name, pUnit->name, (int)pUnit->callsign);
     if (pUnit->status != E_SUCCESS) {
         iw_comment(iw, "!!! %s", error_get_string(pUnit->status));
     }
-    iw_newline(iw);
-
-    iw_comment(iw, "Unit address 1-255");
-    iw_entry(iw, "callsign", "%d", pUnit->callsign);
-
     pUnit->driver->cfgWriteIni(pUnit, iw);
-    iw_newline(iw);
 }
 
 // unit to INI
@@ -455,7 +462,7 @@ void ureg_build_ini(IniWriter *iw)
 
         const UnitDriver *const pDriver = re->driver;
 
-        iw_newline(iw);
+        iw_cmt_newline(iw);
         iw_comment(iw, pDriver->description);
         iw_string(iw, pDriver->name);
         iw_string(iw, "=");
@@ -474,7 +481,6 @@ void ureg_build_ini(IniWriter *iw)
         re = re->next;
         iw_newline(iw);
     }
-    iw_newline(iw); // space before the unit sections
 
     // Now we dump all the units
     li = ulist_head;

framework/unit_registry.h

@@ -96,11 +96,13 @@ bool ureg_instantiate_by_ini(const char *restrict driver_name, const char *restr
  * @param name - unit name (for look-up)
  * @param key - property key
  * @param value - value to set as string
+ * @param callsign - callsign (is part of the section string)
  * @return success
  */
 bool ureg_load_unit_ini_key(const char *restrict name,
                             const char *restrict key,
-                            const char *restrict value);
+                            const char *restrict value,
+                            uint8_t callsign);
 
 /**
  * Run init() for all unit instances.

platform/plat_compat.h

@@ -6,8 +6,8 @@
 #define GEX_PLAT_COMPAT_H
 
 // -------- Static buffers ---------
-#define TSK_STACK_MAIN      200 // USB / VFS task stack size
-#define TSK_STACK_MSG       180 // TF message handler task stack size
+#define TSK_STACK_MAIN      220 // USB / VFS task stack size
+#define TSK_STACK_MSG       200 // TF message handler task stack size
 #define TSK_STACK_JOBRUNNER 80 // Job runner task stack size
 
 #define BULKREAD_MAX_CHUNK  256 // Bulk read buffer

units/digital_out/unit_dout.c

@@ -76,7 +76,7 @@ static void DO_writeIni(Unit *unit, IniWriter *iw)
     iw_comment(iw, "Port name");
     iw_entry(iw, "port", "%c", priv->port_name);
 
-    iw_comment(iw, "Pins (descending, csv, ranges using colon)");
+    iw_comment(iw, "Pins (comma separated, supports ranges)");
     iw_entry(iw, "pins", "%s", str_pinmask(priv->pins, buf));
 
     iw_comment(iw, "Initially high pins");

utils/ini_parser.c

@@ -44,7 +44,7 @@ static void *userdata = NULL; //!< Currently assigned user data for the callback
 
 // Buffers
 static char keybuf[INI_KEY_MAX];
-static char secbuf[INI_KEY_MAX];
+static char secbuf[INI_KEY_MAX+10];
 static char valbuf[INI_VALUE_MAX];
 
 // See header for doxygen!

utils/ini_writer.c

@@ -2,6 +2,7 @@
 // Created by MightyPork on 2017/12/01.
 //
 
+#include <framework/system_settings.h>
 #include "platform.h"
 #include "ini_writer.h"
 
@@ -65,12 +66,22 @@ void iw_section(IniWriter *iw, const char *format, ...)
 void iw_comment(IniWriter *iw, const char *format, ...)
 {
     if (iw->count == 0) return;
+    if (!SystemSettings.ini_comments) return;
 
     iw_string(iw, "# ");
     IW_VPRINTF();
     iw_newline(iw);
 }
 
+void iw_hdr_comment(IniWriter *iw, const char *format, ...)
+{
+    if (iw->count == 0) return;
+
+    iw_string(iw, "## ");
+    IW_VPRINTF();
+    iw_newline(iw);
+}
+
 void iw_entry(IniWriter *iw, const char *key, const char *format, ...)
 {
     if (iw->count == 0) return;

utils/ini_writer.h

@@ -46,6 +46,9 @@ static inline void iw_string(IniWriter *iw, const char *str)
 }
 
 #define iw_newline(iw) iw_string(iw, "\r\n")
+#define iw_cmt_newline(iw) do { \
+    if (SystemSettings.ini_comments) iw_string(iw, "\r\n"); \
+} while (0)
 
 /**
  * Try to snprintf to the file
@@ -77,6 +80,12 @@ void iw_section(IniWriter *iw, const char *format, ...)
 void iw_comment(IniWriter *iw, const char *format, ...)
     __attribute__((format(printf,2,3)));
 
+/**
+ * Same as iw_comment(), but ignores the systemsettings option to disable comments
+ */
+void iw_hdr_comment(IniWriter *iw, const char *format, ...)
+__attribute__((format(printf,2,3)));
+
 /**
  * Try to write a key-value entry
  *

utils/payload_builder.c

@@ -17,6 +17,17 @@ bool pb_buf(PayloadBuilder *pb, const uint8_t *buf, uint32_t len)
     return true;
 }
 
+/** Add some zeros */
+bool pb_reserve(PayloadBuilder *pb, uint32_t len)
+{
+    pb_check_capacity(pb, len);
+    if (!pb->ok) return false;
+
+    memset(pb->current, 0, len);
+    pb->current += len;
+    return true;
+}
+
 /** Write s zero terminated string */
 bool pb_string(PayloadBuilder *pb, const char *str)
 {

utils/payload_builder.h

@@ -71,6 +71,9 @@ struct PayloadBuilder_ {
 /** Write from a buffer */
 bool pb_buf(PayloadBuilder *pb, const uint8_t *buf, uint32_t len);
 
+/** Write zeros */
+bool pb_reserve(PayloadBuilder *pb, uint32_t len);
+
 /** Write a zero terminated string */
 bool pb_string(PayloadBuilder *pb, const char *str);
 

vfs/vfs_manager.c

@@ -718,7 +718,9 @@ static void transfer_update_file_info(vfs_file_t file, uint32_t start_sector, ui
         vfs_printf("    error: starting sector changed from %i to %i\r\n", file_transfer_state.start_sector, start_sector);
         // this is probably a new file
 
-        trap("Changed start offset");//FIXME this sometimes happens, need to find how to reproduce
+        dbg("WARN! Changed start offset");
+
+        //trap("Changed start offset");//FIXME this sometimes happens, need to find how to reproduce
         switch_to_new_file(stream, start_sector, true);
     }