Revert "SPIFFS persistent storage for motor calibration (#122)"

Seems to be causing weird behavior/performance with the motor control loop, particularly noticeable with fine detents. Reverting for now until I have a chance to investigate further.

This reverts commit 1d73af17cc24d0cd464f24ee4073363a7b0045f0.

firmware/src/configuration.cpp

@@ -1,125 +0,0 @@
-#include <SPIFFS.h>
-
-#include "pb_decode.h"
-#include "pb_encode.h"
-
-#include "proto_gen/smartknob.pb.h"
-#include "semaphore_guard.h"
-
-#include "configuration.h"
-
-static const char* CONFIG_PATH = "/config.pb";
-
-Configuration::Configuration() {
-  mutex_ = xSemaphoreCreateMutex();
-  assert(mutex_ != NULL);
-}
-
-Configuration::~Configuration() {
-  vSemaphoreDelete(mutex_);
-}
-
-bool Configuration::loadFromDisk() {
-    SemaphoreGuard lock(mutex_);
-    if (!SPIFFS.begin(true)) {
-        log("Failed to mount SPIFFS");
-        return false;
-    }
-
-    File f = SPIFFS.open(CONFIG_PATH);
-    if (!f) {
-        log("Failed to read config file");
-        return false;
-    }
-
-    size_t read = f.readBytes((char*)buffer_, sizeof(buffer_));
-    f.close();
-
-    pb_istream_t stream = pb_istream_from_buffer(buffer_, read);
-    if (!pb_decode(&stream, PB_PersistentConfiguration_fields, &pb_buffer_)) {
-        char buf[200];
-        snprintf(buf, sizeof(buf), "Decoding failed: %s", PB_GET_ERROR(&stream));
-        log(buf);
-        return false;
-    }
-
-    if (pb_buffer_.version != PERSISTENT_CONFIGURATION_VERSION) {
-        char buf[200];
-        snprintf(buf, sizeof(buf), "Invalid config version. Expected %u, received %u", PERSISTENT_CONFIGURATION_VERSION, pb_buffer_.version);
-        log(buf);
-        return false;
-    }
-    loaded_ = true;
-
-    char buf[200];
-    snprintf(
-        buf,
-        sizeof(buf),
-        "Motor calibration: calib=%u, pole_pairs=%u, zero_offset=%.2f, cw=%u",
-        pb_buffer_.motor.calibrated,
-        pb_buffer_.motor.pole_pairs,
-        pb_buffer_.motor.zero_electrical_offset,
-        pb_buffer_.motor.direction_cw
-    );
-    log(buf);
-    return true;
-}
-
-bool Configuration::saveToDisk() {
-    SemaphoreGuard lock(mutex_);
-
-    pb_ostream_t stream = pb_ostream_from_buffer(buffer_, sizeof(buffer_));
-    pb_buffer_.version = PERSISTENT_CONFIGURATION_VERSION;
-    if (!pb_encode(&stream, PB_PersistentConfiguration_fields, &pb_buffer_)) {
-        char buf[200];
-        snprintf(buf, sizeof(buf), "Encoding failed: %s", PB_GET_ERROR(&stream));
-        log(buf);
-        return false;
-    }
-
-    File f = SPIFFS.open(CONFIG_PATH, FILE_WRITE);
-    if (!f) {
-        log("Failed to read config file");
-        return false;
-    }
-    size_t written = f.write(buffer_, stream.bytes_written);
-    f.close();
-
-    char buf[20];
-    snprintf(buf, sizeof(buf), "Wrote %d bytes", written);
-    log(buf);
-
-    if (written != stream.bytes_written) {
-        log("Failed to write all bytes to file");
-        return false;
-    }
-
-    return true;
-}
-
-PB_PersistentConfiguration Configuration::get() {
-    SemaphoreGuard lock(mutex_);
-    if (!loaded_) {
-        return PB_PersistentConfiguration();
-    }
-    return pb_buffer_;
-}
-
-bool Configuration::setMotorCalibrationAndSave(PB_MotorCalibration& motor_calibration) {
-    {
-        SemaphoreGuard lock(mutex_);
-        pb_buffer_.motor = motor_calibration;
-        pb_buffer_.has_motor = true;
-    }
-    return saveToDisk();
-}
-
-void Configuration::setLogger(Logger* logger) {
-    logger_ = logger;
-}
-
-void Configuration::log(const char* msg) {
-    if (logger_ != nullptr) {
-        logger_->log(msg);
-    }
-}

firmware/src/configuration.h

@@ -1,33 +0,0 @@
-#pragma once
-
-#include <SPIFFS.h>
-#include <PacketSerial.h>
-
-#include "proto_gen/smartknob.pb.h"
-
-#include "logger.h"
-
-const uint32_t PERSISTENT_CONFIGURATION_VERSION = 1;
-
-class Configuration {
-    public:
-        Configuration();
-        ~Configuration();
-
-        void setLogger(Logger* logger);
-        bool loadFromDisk();
-        bool saveToDisk();
-        PB_PersistentConfiguration get();
-        bool setMotorCalibrationAndSave(PB_MotorCalibration& motor_calibration);
-
-    private:
-        SemaphoreHandle_t mutex_;
-
-        Logger* logger_ = nullptr;
-        bool loaded_ = false;
-        PB_PersistentConfiguration pb_buffer_ = {};
-
-        uint8_t buffer_[PB_PersistentConfiguration_size];
-
-        void log(const char* msg);
-};

firmware/src/main.cpp

@@ -1,19 +1,16 @@
 #include <Arduino.h>
 
-#include "configuration.h"
 #include "display_task.h"
 #include "interface_task.h"
 #include "motor_task.h"
 
-Configuration config;
-
 #if SK_DISPLAY
 static DisplayTask display_task(0);
 static DisplayTask* display_task_p = &display_task;
 #else
 static DisplayTask* display_task_p = nullptr;
 #endif
-static MotorTask motor_task(1, config);
+static MotorTask motor_task(1);
 
 
 InterfaceTask interface_task(0, motor_task, display_task_p);
@@ -27,13 +24,9 @@ void setup() {
   motor_task.addListener(display_task.getKnobStateQueue());
   #endif
 
-  interface_task.begin();
-
-  config.setLogger(&interface_task);
-  config.loadFromDisk();
-
   motor_task.setLogger(&interface_task);
   motor_task.begin();
+  interface_task.begin();
 
   // Free up the Arduino loop task
   vTaskDelete(NULL);

firmware/src/motor_task.cpp

@@ -12,6 +12,15 @@
 #include "motors/motor_config.h"
 #include "util.h"
 
+// #### 
+// Hardware-specific motor calibration constants.
+// Run calibration once at startup, then update these constants with the calibration results.
+static const float ZERO_ELECTRICAL_OFFSET = 7.61;
+static const Direction FOC_DIRECTION = Direction::CW;
+static const int MOTOR_POLE_PAIRS = 7;
+// ####
+
+
 static const float DEAD_ZONE_DETENT_PERCENT = 0.2;
 static const float DEAD_ZONE_RAD = 1 * _PI / 180;
 
@@ -22,7 +31,7 @@ static const float IDLE_CORRECTION_MAX_ANGLE_RAD = 5 * PI / 180;
 static const float IDLE_CORRECTION_RATE_ALPHA = 0.0005;
 
 
-MotorTask::MotorTask(const uint8_t task_core, Configuration& configuration) : Task("Motor", 3200, 1, task_core), configuration_(configuration) {
+MotorTask::MotorTask(const uint8_t task_core) : Task("Motor", 2500, 1, task_core) {
     queue_ = xQueueCreate(5, sizeof(Command));
     assert(queue_ != NULL);
 }
@@ -77,9 +86,8 @@ void MotorTask::run() {
     encoder.update();
     delay(10);
 
-    PB_PersistentConfiguration c = configuration_.get();
-    motor.pole_pairs = c.motor.calibrated ? c.motor.pole_pairs : 7;
-    motor.initFOC(c.motor.zero_electrical_offset, c.motor.direction_cw ? Direction::CW : Direction::CCW);
+    motor.pole_pairs = MOTOR_POLE_PAIRS;
+    motor.initFOC(ZERO_ELECTRICAL_OFFSET, FOC_DIRECTION);
 
     motor.monitor_downsample = 0; // disable monitor at first - optional
 
@@ -516,13 +524,13 @@ void MotorTask::calibrate() {
 
 
     // #### Apply settings
+    // TODO: save to non-volatile storage
     motor.pole_pairs = measured_pole_pairs;
     motor.zero_electric_angle = avg_offset_angle + _3PI_2;
     motor.voltage_limit = FOC_VOLTAGE_LIMIT;
     motor.controller = MotionControlType::torque;
 
-    log("");
-    log("RESULTS:");
+    log("\n\nRESULTS:\n  Update these constants at the top of " __FILE__);
     snprintf(buf_, sizeof(buf_), "  ZERO_ELECTRICAL_OFFSET: %.2f", motor.zero_electric_angle);
     log(buf_);
     if (motor.sensor_direction == Direction::CW) {
@@ -532,18 +540,7 @@ void MotorTask::calibrate() {
     }
     snprintf(buf_, sizeof(buf_), "  MOTOR_POLE_PAIRS: %d", motor.pole_pairs);
     log(buf_);
-
-    log("");
-    log("Saving to persistent configuration...");
-    PB_MotorCalibration calibration = {
-        .calibrated = true,
-        .zero_electrical_offset = motor.zero_electric_angle,
-        .direction_cw = motor.sensor_direction == Direction::CW,
-        .pole_pairs = motor.pole_pairs,
-    };
-    if (configuration_.setMotorCalibrationAndSave(calibration)) {
-        log("Success!");
-    }
+    delay(2000);
 }
 
 void MotorTask::checkSensorError() {

firmware/src/motor_task.h

@@ -4,7 +4,6 @@
 #include <SimpleFOC.h>
 #include <vector>
 
-#include "configuration.h"
 #include "logger.h"
 #include "proto_gen/smartknob.pb.h"
 #include "task.h"
@@ -34,7 +33,7 @@ class MotorTask : public Task<MotorTask> {
     friend class Task<MotorTask>; // Allow base Task to invoke protected run()
 
     public:
-        MotorTask(const uint8_t task_core, Configuration& configuration);
+        MotorTask(const uint8_t task_core);
         ~MotorTask();
 
         void setConfig(const PB_SmartKnobConfig& config);
@@ -48,7 +47,6 @@ class MotorTask : public Task<MotorTask> {
         void run();
 
     private:
-        Configuration& configuration_;
         QueueHandle_t queue_;
         Logger* logger_;
         std::vector<QueueHandle_t> listeners_;

firmware/src/proto_gen/smartknob.pb.c

@@ -27,10 +27,4 @@ PB_BIND(PB_SmartKnobConfig, PB_SmartKnobConfig, AUTO)
 PB_BIND(PB_RequestState, PB_RequestState, AUTO)
 
 
-PB_BIND(PB_PersistentConfiguration, PB_PersistentConfiguration, AUTO)
-
-
-PB_BIND(PB_MotorCalibration, PB_MotorCalibration, AUTO)
-
-
 

firmware/src/proto_gen/smartknob.pb.h

@@ -73,19 +73,6 @@ typedef struct _PB_ToSmartknob {
     } payload;
 } PB_ToSmartknob;
 
-typedef struct _PB_MotorCalibration {
-    bool calibrated;
-    float zero_electrical_offset;
-    bool direction_cw;
-    uint32_t pole_pairs;
-} PB_MotorCalibration;
-
-typedef struct _PB_PersistentConfiguration {
-    uint32_t version;
-    bool has_motor;
-    PB_MotorCalibration motor;
-} PB_PersistentConfiguration;
-
 
 #ifdef __cplusplus
 extern "C" {
@@ -99,8 +86,6 @@ extern "C" {
 #define PB_SmartKnobState_init_default           {0, 0, false, PB_SmartKnobConfig_init_default}
 #define PB_SmartKnobConfig_init_default          {0, 0, 0, 0, 0, 0, 0, 0, 0, "", 0, {0, 0, 0, 0, 0}, 0}
 #define PB_RequestState_init_default             {0}
-#define PB_PersistentConfiguration_init_default  {0, false, PB_MotorCalibration_init_default}
-#define PB_MotorCalibration_init_default         {0, 0, 0, 0}
 #define PB_FromSmartKnob_init_zero               {0, 0, {PB_Ack_init_zero}}
 #define PB_ToSmartknob_init_zero                 {0, 0, 0, {PB_RequestState_init_zero}}
 #define PB_Ack_init_zero                         {0}
@@ -108,8 +93,6 @@ extern "C" {
 #define PB_SmartKnobState_init_zero              {0, 0, false, PB_SmartKnobConfig_init_zero}
 #define PB_SmartKnobConfig_init_zero             {0, 0, 0, 0, 0, 0, 0, 0, 0, "", 0, {0, 0, 0, 0, 0}, 0}
 #define PB_RequestState_init_zero                {0}
-#define PB_PersistentConfiguration_init_zero     {0, false, PB_MotorCalibration_init_zero}
-#define PB_MotorCalibration_init_zero            {0, 0, 0, 0}
 
 /* Field tags (for use in manual encoding/decoding) */
 #define PB_Ack_nonce_tag                         1
@@ -137,12 +120,6 @@ extern "C" {
 #define PB_ToSmartknob_nonce_tag                 2
 #define PB_ToSmartknob_request_state_tag         3
 #define PB_ToSmartknob_smartknob_config_tag      4
-#define PB_MotorCalibration_calibrated_tag       1
-#define PB_MotorCalibration_zero_electrical_offset_tag 2
-#define PB_MotorCalibration_direction_cw_tag     3
-#define PB_MotorCalibration_pole_pairs_tag       4
-#define PB_PersistentConfiguration_version_tag   1
-#define PB_PersistentConfiguration_motor_tag     2
 
 /* Struct field encoding specification for nanopb */
 #define PB_FromSmartKnob_FIELDLIST(X, a) \
@@ -205,21 +182,6 @@ X(a, STATIC,   SINGULAR, FLOAT,    snap_point_bias,  12)
 #define PB_RequestState_CALLBACK NULL
 #define PB_RequestState_DEFAULT NULL
 
-#define PB_PersistentConfiguration_FIELDLIST(X, a) \
-X(a, STATIC,   SINGULAR, UINT32,   version,           1) \
-X(a, STATIC,   OPTIONAL, MESSAGE,  motor,             2)
-#define PB_PersistentConfiguration_CALLBACK NULL
-#define PB_PersistentConfiguration_DEFAULT NULL
-#define PB_PersistentConfiguration_motor_MSGTYPE PB_MotorCalibration
-
-#define PB_MotorCalibration_FIELDLIST(X, a) \
-X(a, STATIC,   SINGULAR, BOOL,     calibrated,        1) \
-X(a, STATIC,   SINGULAR, FLOAT,    zero_electrical_offset,   2) \
-X(a, STATIC,   SINGULAR, BOOL,     direction_cw,      3) \
-X(a, STATIC,   SINGULAR, UINT32,   pole_pairs,        4)
-#define PB_MotorCalibration_CALLBACK NULL
-#define PB_MotorCalibration_DEFAULT NULL
-
 extern const pb_msgdesc_t PB_FromSmartKnob_msg;
 extern const pb_msgdesc_t PB_ToSmartknob_msg;
 extern const pb_msgdesc_t PB_Ack_msg;
@@ -227,8 +189,6 @@ extern const pb_msgdesc_t PB_Log_msg;
 extern const pb_msgdesc_t PB_SmartKnobState_msg;
 extern const pb_msgdesc_t PB_SmartKnobConfig_msg;
 extern const pb_msgdesc_t PB_RequestState_msg;
-extern const pb_msgdesc_t PB_PersistentConfiguration_msg;
-extern const pb_msgdesc_t PB_MotorCalibration_msg;
 
 /* Defines for backwards compatibility with code written before nanopb-0.4.0 */
 #define PB_FromSmartKnob_fields &PB_FromSmartKnob_msg
@@ -238,15 +198,11 @@ extern const pb_msgdesc_t PB_MotorCalibration_msg;
 #define PB_SmartKnobState_fields &PB_SmartKnobState_msg
 #define PB_SmartKnobConfig_fields &PB_SmartKnobConfig_msg
 #define PB_RequestState_fields &PB_RequestState_msg
-#define PB_PersistentConfiguration_fields &PB_PersistentConfiguration_msg
-#define PB_MotorCalibration_fields &PB_MotorCalibration_msg
 
 /* Maximum encoded size of messages (where known) */
 #define PB_Ack_size                              6
 #define PB_FromSmartKnob_size                    264
 #define PB_Log_size                              258
-#define PB_MotorCalibration_size                 15
-#define PB_PersistentConfiguration_size          23
 #define PB_RequestState_size                     0
 #define PB_SmartKnobConfig_size                  173
 #define PB_SmartKnobState_size                   192

platformio.ini

@@ -128,9 +128,8 @@ build_flags =
 
 [env:nanofoc]
 extends = base_config
-platform = espressif32@6.3.1
+platform = espressif32
 board = adafruit_feather_esp32s3
-board_build.partitions = firmware/partitions-4MB-spiffs.csv
 lib_deps =
   ${base_config.lib_deps}
   askuric/Simple FOC@^2.3.0

proto/smartknob.proto

@@ -70,14 +70,3 @@ message SmartKnobConfig {
 
 message RequestState {}
 
-message PersistentConfiguration {
-    uint32 version = 1;
-    MotorCalibration motor = 2;
-}
-
-message MotorCalibration {
-    bool calibrated = 1;
-    float zero_electrical_offset = 2;
-    bool direction_cw = 3;
-    uint32 pole_pairs = 4;
-}