Make NVS access synchronous again

Everything relevant is back in internal ram, and likely to stay there.

src/audio/audio_fsm.cpp

@@ -83,7 +83,7 @@ void AudioState::react(const OutputModeChanged& ev) {
   // TODO: handle SetInUse
   ESP_LOGI(kTag, "output mode changed");
   auto new_mode = sServices->nvs().OutputMode();
-  switch (new_mode.get()) {
+  switch (new_mode) {
     case drivers::NvsStorage::Output::kBluetooth:
       sOutput = sBtOutput;
       break;
@@ -118,10 +118,10 @@ void Uninitialised::react(const system_fsm::BootComplete& ev) {
   sBtOutput.reset(new BluetoothAudioOutput(stream, sServices->bluetooth()));
 
   auto& nvs = sServices->nvs();
-  sI2SOutput->SetMaxVolume(nvs.AmpMaxVolume().get());
+  sI2SOutput->SetMaxVolume(nvs.AmpMaxVolume());
-  sI2SOutput->SetVolumeDb(nvs.AmpCurrentVolume().get());
+  sI2SOutput->SetVolumeDb(nvs.AmpCurrentVolume());
 
-  if (sServices->nvs().OutputMode().get() ==
+  if (sServices->nvs().OutputMode() ==
       drivers::NvsStorage::Output::kHeadphones) {
     sOutput = sI2SOutput;
   } else {

src/drivers/bluetooth.cpp

@@ -134,7 +134,7 @@ std::function<void(Event)> BluetoothState::sEventHandler_;
 
 auto BluetoothState::Init(NvsStorage& storage) -> void {
   sStorage_ = &storage;
-  sPreferredDevice_ = storage.PreferredBluetoothDevice().get();
+  sPreferredDevice_ = storage.PreferredBluetoothDevice();
   tinyfsm::FsmList<bluetooth::BluetoothState>::start();
 }
 
@@ -451,7 +451,7 @@ void Connecting::react(const events::internal::A2dp& ev) {
 void Connected::entry() {
   ESP_LOGI(kTag, "entering connected state");
 
-  auto stored_pref = sStorage_->PreferredBluetoothDevice().get();
+  auto stored_pref = sStorage_->PreferredBluetoothDevice();
   if (stored_pref != sPreferredDevice_) {
     sStorage_->PreferredBluetoothDevice(sPreferredDevice_);
   }

src/drivers/include/nvs.hpp

@@ -22,38 +22,35 @@ class NvsStorage {
  public:
   static auto OpenSync() -> NvsStorage*;
 
-  auto PreferredBluetoothDevice()
+  auto PreferredBluetoothDevice() -> std::optional<bluetooth::mac_addr_t>;
-      -> std::future<std::optional<bluetooth::mac_addr_t>>;
+  auto PreferredBluetoothDevice(std::optional<bluetooth::mac_addr_t>) -> bool;
-  auto PreferredBluetoothDevice(std::optional<bluetooth::mac_addr_t>)
-      -> std::future<bool>;
 
   enum class Output : uint8_t {
     kHeadphones = 0,
     kBluetooth = 1,
   };
-  auto OutputMode() -> std::future<Output>;
+  auto OutputMode() -> Output;
-  auto OutputMode(Output) -> std::future<bool>;
+  auto OutputMode(Output) -> bool;
 
-  auto ScreenBrightness() -> std::future<uint_fast8_t>;
+  auto ScreenBrightness() -> uint_fast8_t;
-  auto ScreenBrightness(uint_fast8_t) -> std::future<bool>;
+  auto ScreenBrightness(uint_fast8_t) -> bool;
 
-  auto AmpMaxVolume() -> std::future<uint16_t>;
+  auto AmpMaxVolume() -> uint16_t;
-  auto AmpMaxVolume(uint16_t) -> std::future<bool>;
+  auto AmpMaxVolume(uint16_t) -> bool;
 
-  auto AmpCurrentVolume() -> std::future<uint16_t>;
+  auto AmpCurrentVolume() -> uint16_t;
-  auto AmpCurrentVolume(uint16_t) -> std::future<bool>;
+  auto AmpCurrentVolume(uint16_t) -> bool;
 
-  auto HasShownOnboarding() -> std::future<bool>;
+  auto HasShownOnboarding() -> bool;
-  auto HasShownOnboarding(bool) -> std::future<bool>;
+  auto HasShownOnboarding(bool) -> bool;
 
-  explicit NvsStorage(std::unique_ptr<tasks::Worker>, nvs_handle_t);
+  explicit NvsStorage(nvs_handle_t);
   ~NvsStorage();
 
  private:
   auto DowngradeSchemaSync() -> bool;
   auto SchemaVersionSync() -> uint8_t;
 
-  std::unique_ptr<tasks::Worker> writer_;
   nvs_handle_t handle_;
 };
 

src/drivers/nvs.cpp

@@ -50,10 +50,7 @@ auto NvsStorage::OpenSync() -> NvsStorage* {
     return nullptr;
   }
 
-  std::unique_ptr<NvsStorage> instance = std::make_unique<NvsStorage>(
+  std::unique_ptr<NvsStorage> instance = std::make_unique<NvsStorage>(handle);
-      std::unique_ptr<tasks::Worker>(
-          tasks::Worker::Start<tasks::Type::kNvsWriter>()),
-      handle);
   if (instance->SchemaVersionSync() < kSchemaVersion &&
       !instance->DowngradeSchemaSync()) {
     ESP_LOGW(kTag, "failed to init namespace");
@@ -64,9 +61,7 @@ auto NvsStorage::OpenSync() -> NvsStorage* {
   return instance.release();
 }
 
-NvsStorage::NvsStorage(std::unique_ptr<tasks::Worker> worker,
+NvsStorage::NvsStorage(nvs_handle_t handle) : handle_(handle) {}
-                       nvs_handle_t handle)
-    : writer_(std::move(worker)), handle_(handle) {}
 
 NvsStorage::~NvsStorage() {
   nvs_close(handle_);
@@ -75,133 +70,100 @@ NvsStorage::~NvsStorage() {
 
 auto NvsStorage::DowngradeSchemaSync() -> bool {
   ESP_LOGW(kTag, "namespace needs downgrading");
-  return writer_
+  nvs_erase_all(handle_);
-             ->Dispatch<bool>([&]() -> bool {
+  nvs_set_u8(handle_, kKeyVersion, kSchemaVersion);
-               nvs_erase_all(handle_);
+  return nvs_commit(handle_);
-               nvs_set_u8(handle_, kKeyVersion, kSchemaVersion);
-               return nvs_commit(handle_);
-             })
-             .get() == ESP_OK;
 }
 
 auto NvsStorage::SchemaVersionSync() -> uint8_t {
-  return writer_
+  uint8_t ret;
-      ->Dispatch<uint8_t>([&]() -> uint8_t {
+  if (nvs_get_u8(handle_, kKeyVersion, &ret) != ESP_OK) {
-        uint8_t ret;
+    return UINT8_MAX;
-        if (nvs_get_u8(handle_, kKeyVersion, &ret) != ESP_OK) {
+  }
-          return UINT8_MAX;
+  return ret;
-        }
-        return ret;
-      })
-      .get();
 }
 
 auto NvsStorage::PreferredBluetoothDevice()
-    -> std::future<std::optional<bluetooth::mac_addr_t>> {
+    -> std::optional<bluetooth::mac_addr_t> {
-  return writer_->Dispatch<std::optional<bluetooth::mac_addr_t>>(
+  bluetooth::mac_addr_t out{0};
-      [&]() -> std::optional<bluetooth::mac_addr_t> {
+  size_t size = out.size();
-        bluetooth::mac_addr_t out{0};
+  if (nvs_get_blob(handle_, kKeyBluetooth, out.data(), &size) != ESP_OK) {
-        size_t size = out.size();
+    return {};
-        if (nvs_get_blob(handle_, kKeyBluetooth, out.data(), &size) != ESP_OK) {
+  }
-          return {};
+  return out;
-        }
-        return out;
-      });
 }
 
 auto NvsStorage::PreferredBluetoothDevice(
-    std::optional<bluetooth::mac_addr_t> addr) -> std::future<bool> {
+    std::optional<bluetooth::mac_addr_t> addr) -> bool {
-  return writer_->Dispatch<bool>([&]() {
+  if (!addr) {
-    if (!addr) {
+    nvs_erase_key(handle_, kKeyBluetooth);
-      nvs_erase_key(handle_, kKeyBluetooth);
+  } else {
-    } else {
+    nvs_set_blob(handle_, kKeyBluetooth, addr.value().data(),
-      nvs_set_blob(handle_, kKeyBluetooth, addr.value().data(),
+                 addr.value().size());
-                   addr.value().size());
+  }
-    }
+  return nvs_commit(handle_) == ESP_OK;
-    return nvs_commit(handle_) == ESP_OK;
+}
-  });
+
-}
+auto NvsStorage::OutputMode() -> Output {
-
+  uint8_t out = 0;
-auto NvsStorage::OutputMode() -> std::future<Output> {
+  nvs_get_u8(handle_, kKeyOutput, &out);
-  return writer_->Dispatch<Output>([&]() -> Output {
+  switch (out) {
-    uint8_t out = 0;
+    case static_cast<uint8_t>(Output::kBluetooth):
-    nvs_get_u8(handle_, kKeyOutput, &out);
+      return Output::kBluetooth;
-    switch (out) {
+    case static_cast<uint8_t>(Output::kHeadphones):
-      case static_cast<uint8_t>(Output::kBluetooth):
+    default:
-        return Output::kBluetooth;
+      return Output::kHeadphones;
-      case static_cast<uint8_t>(Output::kHeadphones):
+  }
-      default:
+}
-        return Output::kHeadphones;
+
-    }
+auto NvsStorage::OutputMode(Output out) -> bool {
-  });
+  uint8_t as_int = static_cast<uint8_t>(out);
-}
+  nvs_set_u8(handle_, kKeyOutput, as_int);
-
+  return nvs_commit(handle_) == ESP_OK;
-auto NvsStorage::OutputMode(Output out) -> std::future<bool> {
+}
-  return writer_->Dispatch<bool>([&]() {
+
-    uint8_t as_int = static_cast<uint8_t>(out);
+auto NvsStorage::ScreenBrightness() -> uint_fast8_t {
-    nvs_set_u8(handle_, kKeyOutput, as_int);
+  uint8_t out = 50;
-    return nvs_commit(handle_) == ESP_OK;
+  nvs_get_u8(handle_, kKeyBrightness, &out);
-  });
+  return out;
-}
+}
-
+
-auto NvsStorage::ScreenBrightness() -> std::future<uint_fast8_t> {
+auto NvsStorage::ScreenBrightness(uint_fast8_t val) -> bool {
-  return writer_->Dispatch<uint_fast8_t>([&]() -> uint_fast8_t {
+  nvs_set_u8(handle_, kKeyBrightness, val);
-    uint8_t out = 50;
+  return nvs_commit(handle_) == ESP_OK;
-    nvs_get_u8(handle_, kKeyBrightness, &out);
+}
-    return out;
+
-  });
+auto NvsStorage::AmpMaxVolume() -> uint16_t {
-}
+  uint16_t out = wm8523::kDefaultMaxVolume;
-
+  nvs_get_u16(handle_, kKeyAmpMaxVolume, &out);
-auto NvsStorage::ScreenBrightness(uint_fast8_t val) -> std::future<bool> {
+  return out;
-  return writer_->Dispatch<bool>([&]() {
+}
-    nvs_set_u8(handle_, kKeyBrightness, val);
+
-    return nvs_commit(handle_) == ESP_OK;
+auto NvsStorage::AmpMaxVolume(uint16_t val) -> bool {
-  });
+  nvs_set_u16(handle_, kKeyAmpMaxVolume, val);
-}
+  return nvs_commit(handle_) == ESP_OK;
-
+}
-auto NvsStorage::AmpMaxVolume() -> std::future<uint16_t> {
+
-  return writer_->Dispatch<uint16_t>([&]() -> uint16_t {
+auto NvsStorage::AmpCurrentVolume() -> uint16_t {
-    uint16_t out = wm8523::kDefaultMaxVolume;
+  uint16_t out = wm8523::kDefaultVolume;
-    nvs_get_u16(handle_, kKeyAmpMaxVolume, &out);
+  nvs_get_u16(handle_, kKeyAmpCurrentVolume, &out);
-    return out;
+  return out;
-  });
+}
-}
+
-
+auto NvsStorage::AmpCurrentVolume(uint16_t val) -> bool {
-auto NvsStorage::AmpMaxVolume(uint16_t val) -> std::future<bool> {
+  nvs_set_u16(handle_, kKeyAmpCurrentVolume, val);
-  return writer_->Dispatch<bool>([&]() {
+  return nvs_commit(handle_) == ESP_OK;
-    nvs_set_u16(handle_, kKeyAmpMaxVolume, val);
+}
-    return nvs_commit(handle_) == ESP_OK;
+
-  });
+auto NvsStorage::HasShownOnboarding() -> bool {
-}
+  uint8_t out = false;
-
+  nvs_get_u8(handle_, kKeyOnboarded, &out);
-auto NvsStorage::AmpCurrentVolume() -> std::future<uint16_t> {
+  return out;
-  return writer_->Dispatch<uint16_t>([&]() -> uint16_t {
+}
-    uint16_t out = wm8523::kDefaultVolume;
+
-    nvs_get_u16(handle_, kKeyAmpCurrentVolume, &out);
+auto NvsStorage::HasShownOnboarding(bool val) -> bool {
-    return out;
+  nvs_set_u8(handle_, kKeyOnboarded, val);
-  });
+  return nvs_commit(handle_) == ESP_OK;
-}
-
-auto NvsStorage::AmpCurrentVolume(uint16_t val) -> std::future<bool> {
-  return writer_->Dispatch<bool>([&]() {
-    nvs_set_u16(handle_, kKeyAmpCurrentVolume, val);
-    return nvs_commit(handle_) == ESP_OK;
-  });
-}
-
-auto NvsStorage::HasShownOnboarding() -> std::future<bool> {
-  return writer_->Dispatch<bool>([&]() -> bool {
-    uint8_t out = false;
-    nvs_get_u8(handle_, kKeyOnboarded, &out);
-    return out;
-  });
-}
-
-auto NvsStorage::HasShownOnboarding(bool val) -> std::future<bool> {
-  return writer_->Dispatch<bool>([&]() {
-    nvs_set_u8(handle_, kKeyOnboarded, val);
-    return nvs_commit(handle_) == ESP_OK;
-  });
 }
 
 }  // namespace drivers

src/system_fsm/booting.cpp

@@ -82,7 +82,7 @@ auto Booting::entry() -> void {
   sServices->bluetooth(std::make_unique<drivers::Bluetooth>(sServices->nvs()));
   sServices->bluetooth().SetEventHandler(bt_event_cb);
 
-  if (sServices->nvs().OutputMode().get() ==
+  if (sServices->nvs().OutputMode() ==
       drivers::NvsStorage::Output::kBluetooth) {
     ESP_LOGI(kTag, "enabling bluetooth");
     sServices->bluetooth().Enable();

src/tasks/tasks.cpp

@@ -39,10 +39,6 @@ template <>
 auto Name<Type::kDatabaseBackground>() -> std::pmr::string {
   return "db_bg";
 }
-template <>
-auto Name<Type::kNvsWriter>() -> std::pmr::string {
-  return "nvs";
-}
 
 template <Type t>
 auto AllocateStack() -> cpp::span<StackType_t>;
@@ -86,12 +82,6 @@ auto AllocateStack<Type::kDatabaseBackground>() -> cpp::span<StackType_t> {
   return {static_cast<StackType_t*>(heap_caps_malloc(size, MALLOC_CAP_SPIRAM)),
           size};
 }
-template <>
-auto AllocateStack<Type::kNvsWriter>() -> cpp::span<StackType_t> {
-  constexpr std::size_t size = 4 * 1024;
-  static StackType_t sStack[size];
-  return {sStack, size};
-}
 
 // 2 KiB in internal ram
 // 612 KiB in external ram.
@@ -132,13 +122,6 @@ template <>
 auto Priority<Type::kDatabaseBackground>() -> UBaseType_t {
   return 1;
 }
-// NVS writing requires suspending one of our cores, and disabling tasks with
-// their stacks in PSRAM. Only do it when there's not more important work
-// pending.
-template <>
-auto Priority<Type::kNvsWriter>() -> UBaseType_t {
-  return 2;
-}
 
 template <Type t>
 auto WorkerQueueSize() -> std::size_t;
@@ -152,11 +135,6 @@ auto WorkerQueueSize<Type::kDatabaseBackground>() -> std::size_t {
   return 8;
 }
 
-template <>
-auto WorkerQueueSize<Type::kNvsWriter>() -> std::size_t {
-  return 2;
-}
-
 auto PersistentMain(void* fn) -> void {
   auto* function = reinterpret_cast<std::function<void(void)>*>(fn);
   std::invoke(*function);

src/tasks/tasks.hpp

@@ -39,9 +39,6 @@ enum class Type {
   kDatabase,
   // Task for internal database operations
   kDatabaseBackground,
-  // Task for interacting with NVS -- this needs to be done with an internal
-  // stack.
-  kNvsWriter,
 };
 
 template <Type t>

src/ui/screen_settings.cpp

@@ -138,10 +138,10 @@ Bluetooth::Bluetooth(drivers::Bluetooth& bt, drivers::NvsStorage& nvs)
 auto Bluetooth::ChangeEnabledState(bool enabled) -> void {
   if (enabled) {
     events::System().RunOnTask([&]() { bt_.Enable(); });
-    nvs_.OutputMode(drivers::NvsStorage::Output::kBluetooth).get();
+    nvs_.OutputMode(drivers::NvsStorage::Output::kBluetooth);
   } else {
     events::System().RunOnTask([&]() { bt_.Disable(); });
-    nvs_.OutputMode(drivers::NvsStorage::Output::kHeadphones).get();
+    nvs_.OutputMode(drivers::NvsStorage::Output::kHeadphones);
   }
   events::Audio().Dispatch(audio::OutputModeChanged{});
   RefreshDevicesList();
@@ -156,7 +156,7 @@ auto Bluetooth::RefreshDevicesList() -> void {
 
   auto devices = bt_.KnownDevices();
   std::optional<drivers::bluetooth::mac_addr_t> preferred_device =
-      nvs_.PreferredBluetoothDevice().get();
+      nvs_.PreferredBluetoothDevice();
 
   // If the user's current selection is within the devices list, then we need
   // to be careful not to rearrange the list items underneath them.
@@ -283,7 +283,7 @@ Headphones::Headphones(drivers::NvsStorage& nvs)
                           "before clipping (+10dB)\nCustom");
   lv_group_add_obj(group_, vol_dropdown);
 
-  uint16_t level = nvs.AmpMaxVolume().get();
+  uint16_t level = nvs.AmpMaxVolume();
   for (int i = 0; i < index_to_level_.size() + 1; i++) {
     if (i == index_to_level_.size() || index_to_level_[i] == level) {
       lv_dropdown_set_selected(vol_dropdown, i);
@@ -386,7 +386,7 @@ Appearance::Appearance(drivers::NvsStorage& nvs, drivers::Display& display)
   lv_obj_t* toggle = lv_switch_create(toggle_container);
   lv_group_add_obj(group_, toggle);
 
-  uint_fast8_t initial_brightness = nvs_.ScreenBrightness().get();
+  uint_fast8_t initial_brightness = nvs_.ScreenBrightness();
 
   lv_obj_t* brightness_label = lv_label_create(content_);
   lv_label_set_text(brightness_label, "Brightness");

src/ui/ui_fsm.cpp

@@ -141,7 +141,7 @@ void Splash::react(const system_fsm::BootComplete& ev) {
   lv_disp_set_theme(NULL, base_theme);
   themes::Theme::instance()->Apply();
 
-  sDisplay->SetBrightness(sServices->nvs().ScreenBrightness().get());
+  sDisplay->SetBrightness(sServices->nvs().ScreenBrightness());
 
   auto touchwheel = sServices->touchwheel();
   if (touchwheel) {
@@ -153,7 +153,7 @@ void Splash::react(const system_fsm::BootComplete& ev) {
     ESP_LOGE(kTag, "no input devices initialised!");
   }
 
-  if (sServices->nvs().HasShownOnboarding().get()) {
+  if (sServices->nvs().HasShownOnboarding()) {
     transit<Browse>();
   } else {
     transit<Onboarding>();