fix(thermocycler-gen2): I2C error mitigation (#321)

* Added some more error enumerations
* Increased priority of I2C interrupt
* Change I2C speed to Fast Mode (400kHz)
* Added retries to ADC reading
* make _i2c_task_to_notify atomic 
* Added retries & error reporting to the lid heater thermistor

stm32-modules/include/thermocycler-gen2/firmware/ads1115.hpp

@@ -14,9 +14,11 @@
 namespace ADS1115 {
 
 enum class Error {
-    ADCTimeout, /**< Timed out waiting for ADC.*/
-    ADCPin,     /**< Pin is not allowed.*/
-    ADCInit     /**< ADC is not initialized.*/
+    ADCTimeout = 1, /**< Timed out waiting for ADC.*/
+    I2CTimeout = 2, /**< Timed out waiting for I2C.*/
+    DoubleArm = 3,  /**< ADC already armed.*/
+    ADCPin = 4,     /**< Pin is not allowed.*/
+    ADCInit = 5     /**< ADC is not initialized.*/
 };
 
 class ADC {

stm32-modules/thermocycler-gen2/firmware/thermal/ads1115.cpp

@@ -82,7 +82,7 @@ auto ADC::read(uint16_t pin) -> ADC::ReadVal {
     i2c_ret = thermal_arm_adc_for_read(_id);
     if (!i2c_ret) {
         static_cast<void>(release_lock());
-        return ReadVal(Error::ADCTimeout);
+        return ReadVal(Error::DoubleArm);
     }
     // This kicks off the conversion on the selected pin
     i2c_ret = thermal_i2c_write_16(
@@ -101,10 +101,12 @@ auto ADC::read(uint16_t pin) -> ADC::ReadVal {
     }
 
     static_cast<void>(release_lock());
-    if ((!i2c_ret) || (notification_val == 0)) {
+    if (!i2c_ret) {
+        return ReadVal(Error::I2CTimeout);
+    }
+    if (notification_val == 0) {
         return ReadVal(Error::ADCTimeout);
     }
-
     return ReadVal(_last_result);
 }
 

stm32-modules/thermocycler-gen2/firmware/thermal/freertos_lid_heater_task.cpp

@@ -12,6 +12,8 @@
 
 namespace lid_heater_control_task {
 
+static constexpr uint8_t MAX_RETRIES = 5;
+
 constexpr uint8_t _adc_address = (0x49 << 1);
 
 constexpr uint8_t _adc_lid_pin = 1;
@@ -74,12 +76,25 @@ static void run_thermistor_task(void *param) {
             &last_wake_time,
             // NOLINTNEXTLINE(readability-static-accessed-through-instance)
             _main_task.CONTROL_PERIOD_TICKS);
+        bool done = false;
+        uint8_t retries = 0;
         auto result = adc.read(_adc_lid_pin);
-        if (std::holds_alternative<ADS1115::Error>(result)) {
-            readings.lid_temp = 0;
-        } else {
-            readings.lid_temp = std::get<uint16_t>(result);
+        while (!done) {
+            if (std::holds_alternative<uint16_t>(result)) {
+                done = true;
+                readings.lid_temp = std::get<uint16_t>(result);
+            } else if (++retries < MAX_RETRIES) {
+                // Short delay for reliability
+                vTaskDelay(pdMS_TO_TICKS(5));
+                result = adc.read(_adc_lid_pin);
+            } else {
+                // Retries expired
+                done = true;
+                readings.lid_temp =
+                    static_cast<uint16_t>(std::get<ADS1115::Error>(result));
+            }
         }
+
         readings.timestamp_ms = xTaskGetTickCount();
         auto send_ret = _main_task.get_message_queue().try_send(readings);
         static_cast<void>(

stm32-modules/thermocycler-gen2/firmware/thermal/freertos_thermal_plate_task.cpp

@@ -15,6 +15,8 @@
 
 namespace thermal_plate_control_task {
 
+static constexpr uint8_t MAX_RETRIES = 5;
+
 enum class ADCAddress : uint8_t {
     ADC_FRONT = ((0x48) << 1),  // AKA ADC1
     ADC_REAR = ((0x49) << 1)    // AKA ADC2
@@ -79,11 +81,25 @@ static StaticTask_t
  * ADC cannot be read.
  */
 static auto read_thermistor(const ADCPinMap &pin) -> uint16_t {
+    uint8_t retries = 0;
+    bool done = false;
+    // Keep trying to read
     auto result =
         _adc.at(static_cast<uint8_t>(pin.adc_index)).read(pin.adc_pin);
-    if (std::holds_alternative<ADS1115::Error>(result)) {
-        return 0;
+    while (!done) {
+        if (std::holds_alternative<uint16_t>(result)) {
+            done = true;
+        } else if (++retries < MAX_RETRIES) {
+            // Short delay for reliability
+            vTaskDelay(pdMS_TO_TICKS(5));
+            result =
+                _adc.at(static_cast<uint8_t>(pin.adc_index)).read(pin.adc_pin);
+        } else {
+            // Retries expired
+            return static_cast<uint16_t>(std::get<ADS1115::Error>(result));
+        }
     }
+
     return std::get<uint16_t>(result);
 }
 
@@ -117,7 +133,6 @@ static void run_thermistor_task(void *param) {
             &last_wake_time,
             // NOLINTNEXTLINE(readability-static-accessed-through-instance)
             _main_task.CONTROL_PERIOD_TICKS);
-
         readings.front_right = read_thermistor(
             _adc_map[thermal_general::ThermistorID::THERM_FRONT_RIGHT]);
         readings.front_left = read_thermistor(

stm32-modules/thermocycler-gen2/firmware/thermal/thermal_hardware.c

@@ -46,17 +46,17 @@
 /** Private definitions */
 
 #define I2C_INSTANCE (I2C2)
-/* Driven by PCLK1 to 1MHz */
-#define I2C_TIMING   (0x00802172)
+/* Driven by PCLK1 to Fast Mode - just shy of 400kHz */
+#define I2C_TIMING   (0x80500D1D)
 /** Max buffer: 2 data bytes*/
 #define I2C_BUF_MAX (2)
 /** Size of register address: 1 byte.*/
 #define REGISTER_ADDR_LEN (1)
-/** NVIC priority of ADC interrupts.
- * On the higher end (low-priority) because timing
- * is not critical compared to other interrupts.
+/** 
+ * NVIC priority of ADC interrupts.
+ * Matches the priority of motor interrupts.
  */
-#define ADC_READY_ITR_PRIO (10)
+#define ADC_READY_ITR_PRIO (4)
 
 /** EEPROM write protect pin */
 #define EEPROM_WRITE_PROTECT_PIN  (GPIO_PIN_10)
@@ -65,7 +65,7 @@
 
 /** Local variables */
 
-static TaskHandle_t _i2c_task_to_notify = NULL;
+static _Atomic TaskHandle_t _i2c_task_to_notify = NULL;
 static atomic_flag _initialization_started = ATOMIC_FLAG_INIT;
 static bool _initialization_done = false;
 
@@ -187,10 +187,6 @@ bool thermal_i2c_write_16(uint16_t addr, uint8_t reg, uint16_t val) {
     }
 
     // Set up notification info
-    if(_i2c_task_to_notify != NULL) {
-        xSemaphoreGive(_i2c_semaphore);
-        return false;
-    }
     _i2c_task_to_notify = xTaskGetCurrentTaskHandle();
 
     // Prepare buffer & send it