reviewing

Author: brentru

src/Wippersnapper_V2.cpp

@@ -1231,11 +1231,9 @@ void Wippersnapper_V2::connect() {
 #endif
     WS_DEBUG_PRINTLN("[APP] Performing I2C Autoscan...");
     WsV2._i2c_controller->ScanI2cBus(true);
-    WS_DEBUG_PRINTLN("[APP] Scan results: ");
-    WsV2._i2c_controller->PrintScanResults();
     // Parse the JSON file
     if (!WsV2._sdCardV2->ParseFileConfig())
-      haltErrorV2("Failed to parse config.json!");
+      haltErrorV2("[APP] Failed to parse config.json!");
     WS_DEBUG_PRINTLN("[APP] Attempting to configure hardware...");
 #ifndef OFFLINE_MODE_DEBUG
     if (!WsV2._sdCardV2->CreateNewLogFile())

src/Wippersnapper_V2.h

@@ -128,7 +128,7 @@
 #endif
 
 #define WS_VERSION                                                             \
-  "1.0.0-offline-beta.autoconfig" ///< WipperSnapper app. version
+  "1.0.0-offline-beta.3" ///< WipperSnapper app. version
                                   ///< (semver-formatted)
 
 #define WS_WDT_TIMEOUT 60000       ///< WDT timeout

src/components/i2c/controller.cpp

@@ -572,27 +572,61 @@ bool I2cController::InitMux(const char *name, uint32_t address,
                             bool is_alt_bus) {
   if (is_alt_bus) {
     if (!_i2c_bus_alt->HasMux()) {
-      WS_DEBUG_PRINTLN("[i2c] Initializing MUX driver on alt bus...");
       if (!_i2c_bus_alt->AddMuxToBus(address, name)) {
         return false;
       }
-      WS_DEBUG_PRINTLN("OK!");
     }
   } else {
     if (!_i2c_bus_default->HasMux()) {
-      WS_DEBUG_PRINTLN("[i2c] Initializing MUX driver on default bus...");
-      WS_DEBUG_PRINT("[i2c] addr: ");
-      WS_DEBUG_PRINT(address, HEX);
       if (!_i2c_bus_default->AddMuxToBus(address, name)) {
         return false;
       }
-      WS_DEBUG_PRINTLN("OK!");
     }
   }
   // TODO [Online]: Publish back out to IO here!
   return true;
 }
 
+/***********************************************************************/
+/*!
+    @brief    Checks if a driver has already been initialized with the
+              given device descriptor.
+    @param    device_descriptor
+                The I2cDeviceDescriptor message.
+    @returns  True if a driver has already been initialized, False
+              otherwise.
+*/
+/***********************************************************************/
+bool I2cController::IsDriverInitialized(
+    wippersnapper_i2c_I2cDeviceDescriptor &device_descriptor) {
+  // Before we do anything, check if a driver has been already initialized with
+  // the device_descriptor if so, we log and skip
+  for (auto &driver : _i2c_drivers) {
+    // Do they share the same address?
+    if (driver->GetAddress() == device_descriptor.i2c_device_address) {
+      // Okay - do they sit on different i2c buses?
+      bool is_driver_bus_alt = driver->HasAltI2CBus();
+      bool is_device_bus_alt =
+          (strcmp(device_descriptor.i2c_bus_scl, "default") != 0) ||
+          (strcmp(device_descriptor.i2c_bus_sda, "default") != 0);
+      // Bus descriptors do not match, so we haven't initialized this candidate
+      if (is_driver_bus_alt != is_device_bus_alt)
+        continue;
+
+      // What about the MUX?
+      if (driver->HasMux() &&
+          driver->GetMuxAddress() == device_descriptor.i2c_mux_address &&
+          driver->GetMuxChannel() != device_descriptor.i2c_mux_channel) {
+        continue;
+      }
+
+      WS_DEBUG_PRINTLN("[i2c] Descriptor already initialized...");
+      return true;
+    }
+  }
+  return false;
+}
+
 /***********************************************************************/
 /*!
     @brief    Implements handling for a I2cDeviceAddOrReplace message
@@ -622,38 +656,10 @@ bool I2cController::Handle_I2cDeviceAddOrReplace(pb_istream_t *stream) {
   wippersnapper_i2c_I2cDeviceDescriptor device_descriptor =
       _i2c_model->GetI2cDeviceAddOrReplaceMsg()->i2c_device_description;
 
-  // Before we do anything, check if a driver has been already initialized with
-  // the device_descriptor if so, we log and skip
-  // TODO: Break this out into a new func.
-  bool did_init_already = false;
-  for (auto &driver : _i2c_drivers) {
-    // Do they share the same address?
-    if (driver->GetAddress() == device_descriptor.i2c_device_address) {
-      // Okay - do they sit on different i2c buses?
-      bool is_driver_bus_alt = driver->HasAltI2CBus();
-      bool is_device_bus_alt =
-          (strcmp(device_descriptor.i2c_bus_scl, "default") != 0) ||
-          (strcmp(device_descriptor.i2c_bus_sda, "default") != 0);
-
-      // Bus descriptors do not match, we haven't initialized this candidate
-      if (is_driver_bus_alt != is_device_bus_alt)
-        continue;
-
-      // What about the MUX?
-      if (driver->HasMux() &&
-          driver->GetMuxAddress() == device_descriptor.i2c_mux_address) {
-        if (driver->GetMuxChannel() != device_descriptor.i2c_mux_channel) {
-          continue;
-        }
-      }
-      WS_DEBUG_PRINTLN("[i2c] Descriptor already initialized...");
-      did_init_already = true;
-      break;
-    }
-  }
 
-  if (did_init_already) {
-    WS_DEBUG_PRINTLN("[i2c] Device already initialized, ignoring...");
+  // Did the driver initialize correctly?
+  if ( IsDriverInitialized(device_descriptor)) {
+    WS_DEBUG_PRINTLN("[i2c] Driver already initialized, skipping...");
     return true;
   }
 
@@ -718,10 +724,8 @@ bool I2cController::Handle_I2cDeviceAddOrReplace(pb_istream_t *stream) {
   // Assign I2C bus
   TwoWire *bus = nullptr;
   if (use_alt_bus) {
-    WS_DEBUG_PRINTLN("[i2c] Using alt. I2C bus...");
     bus = _i2c_bus_alt->GetBus();
   } else {
-    WS_DEBUG_PRINTLN("[i2c] Using default I2C bus...");
     bus = _i2c_bus_default->GetBus();
   }
 
@@ -735,7 +739,7 @@ bool I2cController::Handle_I2cDeviceAddOrReplace(pb_istream_t *stream) {
     WS_DEBUG_PRINTLN(device_descriptor.i2c_device_address);
     if (device_descriptor.i2c_device_address == 0x68 ||
         device_descriptor.i2c_device_address == 0x70) {
-      WS_DEBUG_PRINTLN("[i2c] Device address is shared with rtx/mux, can not "
+      WS_DEBUG_PRINTLN("[i2c] Device address is shared with RTC/MUX, can not "
                        "auto-init, skipping!");
       return true;
     }
@@ -769,6 +773,8 @@ bool I2cController::Handle_I2cDeviceAddOrReplace(pb_istream_t *stream) {
         break;
       }
     }
+    WS_DEBUG_PRINTLN("[i2c] ERROR - Candidates exhausted, driver not found!");
+    return true; // dont cause an error in the app
   } else {
     WS_DEBUG_PRINTLN("[i2c] Device in message/cfg file.");
     // Create new driver
@@ -806,12 +812,12 @@ bool I2cController::Handle_I2cDeviceAddOrReplace(pb_istream_t *stream) {
 
     if (!drv->begin()) {
       if (WsV2._sdCardV2->isModeOffline()) {
-        WsV2.haltErrorV2("[i2c] Driver failed to initialize!\n\tDid you set "
+        WS_DEBUG_PRINTLN("[i2c] Failed to initialize driver!\n\tDid you set "
                          "the correct value for i2cDeviceName?\n\tDid you set "
                          "the correct value for"
-                         "i2cDeviceAddress?",
-                         WS_LED_STATUS_ERROR_RUNTIME, false);
+                         "i2cDeviceAddress?");
       }
+      return true; // don't cause an error during runtime if the device is not found
     }
     WS_DEBUG_PRINTLN("[i2c] Driver successfully initialized!");
   }

src/components/i2c/controller.h

@@ -108,6 +108,7 @@ class I2cController {
   void PrintScanResults();
   uint32_t GetScanDeviceAddress(int index);
   size_t GetScanDeviceCount();
+  bool IsDriverInitialized(wippersnapper_i2c_I2cDeviceDescriptor& device_descriptor);
 
 private:
   I2cModel *_i2c_model;                ///< Pointer to an I2C model object

src/provisioning/sdcard/ws_sdcard.cpp

@@ -494,12 +494,9 @@ bool ws_sdcard::ParseI2cDeviceAddReplace(
       HexStrToInt(addr_device);
 
   // MUXes, Seesaw, special devices should have an auto-init flag set to false
-  const char *is_auto = component["autoInit"] | "true";
-  WS_DEBUG_PRINT("[SD] Found autoInit = ");
-  WS_DEBUG_PRINTLN(is_auto);
+  const char *is_auto = component["autoConfig"] | "true";
   if (strcmp(is_auto, "false") == 0) {
-    WS_DEBUG_PRINTLN(
-        "[SD] Found autoInit = false, do not initialize this address");
+    WS_DEBUG_PRINTLN("[SD] autoConfig = false, do not attempt to automatically initialize this address");
     _cfg_i2c_addresses.push_back(
         msg_i2c_add.i2c_device_description.i2c_device_address);
   }

src/provisioning/tinyusb/Wippersnapper_FS.cpp

@@ -7,7 +7,7 @@
  * please support Adafruit and open-source hardware by purchasing
  * products from Adafruit!
  *
- * Copyright (c) Brent Rubell 2024 for Adafruit Industries.
+ * Copyright (c) Brent Rubell 2024-2025 for Adafruit Industries.
  *
  * BSD license, all text here must be included in any redistribution.
  *
@@ -107,8 +107,7 @@ Wippersnapper_FS::Wippersnapper_FS() {
   _fs_changed = false;
 
   usb_cdc.begin(115200);
-  // If already enumerated, additional class driver begin() e.g msc, hid, midi
-  // won't take effect until re-enumeration
+  // Re-enumerate to allow cdc class begin() to take effect
   if (TinyUSBDevice.mounted()) {
     TinyUSBDevice.detach();
     delay(10);
@@ -121,13 +120,9 @@ Wippersnapper_FS::Wippersnapper_FS() {
     HaltFilesystem("Failed to initialize the flash chip!");
   }
 
-  // Attempt to initialize the filesystem
-  bool is_fs_formatted = wipperFatFs_v2.begin(&flash_v2);
-
   // If we are not formatted, attempt to format the filesystem as fat12
-  if (!is_fs_formatted) {
+  if (! wipperFatFs_v2.begin(&flash_v2)) {
     FRESULT rc = format_fs_fat12();
-
     if (rc != FR_OK) {
       setStatusLEDColor(RED);
       HaltFilesystem("FATAL ERROR: Failed to format the filesystem!");
@@ -147,7 +142,9 @@ Wippersnapper_FS::Wippersnapper_FS() {
   }
 
   // Initialize USB-MSC
+  #ifndef BUILD_OFFLINE_ONLY
   InitUsbMsc();
+  #endif
 
   // If we wrote a fresh secrets.json file, halt until user edits the file and
   // RESETs the device Signal to user that action must be taken (edit
@@ -280,30 +277,29 @@ void Wippersnapper_FS::InitUsbMsc() {
   // Set disk vendor id, product id and revision with string up to 8, 16, 4
   // characters respectively
   usb_msc_v2.setID("Adafruit", "External Flash", "1.0");
-  // Set callback
+  // Set r/w callbacks
   usb_msc_v2.setReadWriteCallback(qspi_msc_read_cb_v2, qspi_msc_write_cb_v2,
                                   qspi_msc_flush_cb_v2);
 
   // Set disk size, block size should be 512 regardless of spi flash page size
   usb_msc_v2.setCapacity(flash_v2.pageSize() * flash_v2.numPages() / 512, 512);
 
   // MSC is ready for read/write
-  usb_msc_v2.setUnitReady(false);
+  usb_msc_v2.setUnitReady(true);
 
-  // Set callback when MSC ready
+  // Setup callback for when MSC ready
   _fs_changed = false;
   usb_msc_v2.setReadyCallback(0, msc_ready_callback);
 
   // init MSC
-  // usb_msc_v2.begin();
+  usb_msc_v2.begin();
 
-  // If already enumerated, additional class driverr begin() e.g msc, hid, midi
-  // won't take effect until re-enumeration
-  // Attach MSC and wait for enumeration
-  // #ifndef BUILD_OFFLINE_ONLY
-  TinyUSBDevice.attach();
-  delay(500);
-  // #endif
+  // Re-enumerate to allow msc class begin() to take effect
+  if (TinyUSBDevice.mounted()) {
+    TinyUSBDevice.detach();
+    delay(10);
+    TinyUSBDevice.attach();
+  }
 }
 
 /**************************************************************************/
@@ -460,6 +456,7 @@ void Wippersnapper_FS::AddI2cDeviceToFileConfig(
   new_component["componentAPI"] = "i2c";
   new_component["i2cDeviceName"] = driver_name;
   new_component["period"] = 30;
+  new_component["autoConfig"] = "true";
   char address_str[6];
   sprintf(address_str, "0x%02X", address);
   new_component["i2cDeviceAddress"] = address_str;
@@ -491,19 +488,8 @@ bool Wippersnapper_FS::WriteFileConfig() {
   file_cfg.flush();
   flash_v2.syncBlocks();
   refreshMassStorage();
-  // Re-attach USB-MSC with updated filesystem
-  // NOTE: This is required to ensure the filesystem is sync'd between host and
-  // device
-  usb_msc_v2.begin();
-  usb_msc_v2.setUnitReady(true);
-  if (TinyUSBDevice.mounted()) {
-    TinyUSBDevice.detach();
-    delay(10);
-    TinyUSBDevice.attach();
-  }
-  // TODO: This is debug, we can remove it!
-  WS_DEBUG_PRINT("Bytes written to config.json: ");
-  WS_DEBUG_PRINTLN(bytes_written);
+  delay(500);
+  InitUsbMsc();
   return true;
 }