From 1e08f21994b5cd87f01274feca00356f8f8394ef Mon Sep 17 00:00:00 2001 From: Dan Green Date: Fri, 23 Aug 2024 23:05:34 -0700 Subject: [PATCH 1/5] Patch selector gets patchlist the first time its opened Fixes a bug where if patchlist is refreshed from the SaveDialog before PatchSel is shown, Patch Selector ne ver sees the changes since it gets PatchListUnchanged --- firmware/src/gui/pages/patch_selector.hh | 5 ++++- firmware/src/patch_file/open_patch_manager.hh | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/firmware/src/gui/pages/patch_selector.hh b/firmware/src/gui/pages/patch_selector.hh index aa7ec7a5d..9a490d4b8 100644 --- a/firmware/src/gui/pages/patch_selector.hh +++ b/firmware/src/gui/pages/patch_selector.hh @@ -39,7 +39,10 @@ struct PatchSelectorPage : PageBase { // Don't persist module selection args.module_id = std::nullopt; - state = State::TryingToRequestPatchList; + if (last_refresh_check_tm == 0) + state = State::ReloadingPatchList; + else + state = State::TryingToRequestPatchList; hide_spinner(); blur_subdir_panel(); diff --git a/firmware/src/patch_file/open_patch_manager.hh b/firmware/src/patch_file/open_patch_manager.hh index 1057d5ec6..5a8ec65d9 100644 --- a/firmware/src/patch_file/open_patch_manager.hh +++ b/firmware/src/patch_file/open_patch_manager.hh @@ -128,7 +128,6 @@ public: PatchData *get_playing_patch() { if (playing_patch_) { if (open_patches_.exists(playing_patch_)) { - pr_dbg("Playing patch exists\n"); return &playing_patch_->patch; } else { pr_err("Playing patch not null and not found in open patches!\n"); From 90fe65f6ea831b4229f660049cd0c5dd91b984e2 Mon Sep 17 00:00:00 2001 From: Dan Green Date: Fri, 23 Aug 2024 23:26:10 -0700 Subject: [PATCH 2/5] Patch Selector stays in sync with changes to PatchDirList Does not copy it. The copy is hard to keep in sync, as the PatchDirList could be updated in the save dialog, and possible more places in the future --- firmware/src/gui/pages/patch_selector.hh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/firmware/src/gui/pages/patch_selector.hh b/firmware/src/gui/pages/patch_selector.hh index 9a490d4b8..1b202b85f 100644 --- a/firmware/src/gui/pages/patch_selector.hh +++ b/firmware/src/gui/pages/patch_selector.hh @@ -21,7 +21,8 @@ struct PatchSelectorPage : PageBase { PatchSelectorPage(PatchContext info, PatchSelectorSubdirPanel &subdir_panel) : PageBase{info, PageId::PatchSel} - , subdir_panel{subdir_panel} { + , subdir_panel{subdir_panel} + , patchfiles{patch_storage.get_patch_list()} { init_bg(ui_PatchSelectorPage); @@ -94,6 +95,7 @@ struct PatchSelectorPage : PageBase { is_populating_subdir_panel = true; update_open_patches(); subdir_panel.populate(patchfiles); + subdir_panel.show_recent_files(); populate_roller(); } @@ -299,7 +301,6 @@ struct PatchSelectorPage : PageBase { } break; case State::ReloadingPatchList: - patchfiles = patch_storage.get_patch_list(); //copy refresh_patchlist(); refresh_subdir_panel(); hide_spinner(); @@ -468,7 +469,7 @@ private: unsigned last_selected_idx = 0; PatchSelectorSubdirPanel &subdir_panel; - PatchDirList patchfiles; + PatchDirList &patchfiles; bool is_populating_subdir_panel = false; From aed2bc5b6e29ffba0eee7cd8e94e80b211866d5a Mon Sep 17 00:00:00 2001 From: Dan Green Date: Fri, 23 Aug 2024 23:27:34 -0700 Subject: [PATCH 3/5] Save Dialog volume sidebar does not show 'Recent' --- firmware/src/gui/pages/patch_selector_sidebar.hh | 8 ++++++++ firmware/src/gui/pages/save_dialog.hh | 2 ++ 2 files changed, 10 insertions(+) diff --git a/firmware/src/gui/pages/patch_selector_sidebar.hh b/firmware/src/gui/pages/patch_selector_sidebar.hh index caf74db11..32677f5c8 100644 --- a/firmware/src/gui/pages/patch_selector_sidebar.hh +++ b/firmware/src/gui/pages/patch_selector_sidebar.hh @@ -70,6 +70,14 @@ struct PatchSelectorSubdirPanel { } } + void hide_recent_files() { + lv_hide(vol_conts[0]); + } + + void show_recent_files() { + lv_show(vol_conts[0]); + } + void refresh(EntryInfo const &selected_patch) { // TODO: check if list was re-populated, or if entry info dir name changed, and only refresh if so diff --git a/firmware/src/gui/pages/save_dialog.hh b/firmware/src/gui/pages/save_dialog.hh index 8243fd8d5..65eb16d97 100644 --- a/firmware/src/gui/pages/save_dialog.hh +++ b/firmware/src/gui/pages/save_dialog.hh @@ -79,6 +79,7 @@ struct SaveDialog { case RefreshState::ReloadingPatchList: subdir_panel.populate(patch_storage.get_patch_list()); + subdir_panel.hide_recent_files(); // hide_spinner(); subdir_panel.focus(); refresh_state = RefreshState::Idle; @@ -188,6 +189,7 @@ struct SaveDialog { EntryInfo selected_patch{.kind = DirEntryKind::Dir, .vol = patches.get_view_patch_vol(), .path = file_path}; subdir_panel.refresh(selected_patch); + subdir_panel.hide_recent_files(); } void hide_subdir_panel() { From 7c26916193345ac20cf177d2b4c47773453a091c Mon Sep 17 00:00:00 2001 From: Dan Green Date: Fri, 23 Aug 2024 23:28:00 -0700 Subject: [PATCH 4/5] Make patchdir functions const-correct --- firmware/src/gui/pages/patch_selector_sidebar.hh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/firmware/src/gui/pages/patch_selector_sidebar.hh b/firmware/src/gui/pages/patch_selector_sidebar.hh index 32677f5c8..5068b23e7 100644 --- a/firmware/src/gui/pages/patch_selector_sidebar.hh +++ b/firmware/src/gui/pages/patch_selector_sidebar.hh @@ -29,7 +29,7 @@ struct PatchSelectorSubdirPanel { } } - void populate(PatchDirList &patchfiles) { + void populate(PatchDirList const &patchfiles) { if (group == nullptr) group = lv_group_create(); @@ -58,7 +58,7 @@ struct PatchSelectorSubdirPanel { } // Add root-level dir on volume - lv_obj_set_user_data(vol_item, &root); + lv_obj_set_user_data(vol_item, (void *)&root); lv_group_add_obj(group, vol_item); // Add all dirs on volume @@ -163,7 +163,7 @@ struct PatchSelectorSubdirPanel { } std::string_view dirname = ""; - if (auto dir = static_cast(event->target->user_data); dir) { + if (auto dir = static_cast(event->target->user_data); dir) { dirname = std::string_view{dir->name}; } @@ -192,7 +192,7 @@ struct PatchSelectorSubdirPanel { } std::string_view dirname = ""; - if (auto dir = static_cast(event->target->user_data); dir) { + if (auto dir = static_cast(event->target->user_data); dir) { dirname = std::string_view{dir->name}; } @@ -206,7 +206,7 @@ struct PatchSelectorSubdirPanel { std::function click_cb; private: - void add_subdir_to_panel(PatchDir &dir, lv_obj_t *vol_label) { + void add_subdir_to_panel(PatchDir const &dir, lv_obj_t *vol_label) { if (dir.files.size() == 0) return; @@ -223,7 +223,7 @@ private: lv_obj_set_width(name_label, LV_PCT(100)); lv_obj_add_flag(btn, LV_OBJ_FLAG_SCROLL_ON_FOCUS); - lv_obj_set_user_data(btn, &dir); + lv_obj_set_user_data(btn, (void *)&dir); while (lv_obj_remove_event_cb(btn, nullptr)) { } From 6a6a5125e8cbff22d87720ece8383409a8eaf7c6 Mon Sep 17 00:00:00 2001 From: Dan Green Date: Fri, 23 Aug 2024 23:49:51 -0700 Subject: [PATCH 5/5] Add lock to patchlist Not a real thread-safe lock, but works to document when we have access to the patchfile list and when M4 does --- firmware/src/gui/pages/patch_selector.hh | 26 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/firmware/src/gui/pages/patch_selector.hh b/firmware/src/gui/pages/patch_selector.hh index 1b202b85f..eebea8fbe 100644 --- a/firmware/src/gui/pages/patch_selector.hh +++ b/firmware/src/gui/pages/patch_selector.hh @@ -62,6 +62,7 @@ struct PatchSelectorPage : PageBase { is_populating_subdir_panel = true; setup_subdir_panel(); + patchfiles_locked = false; refresh_patchlist(); } @@ -92,11 +93,14 @@ struct PatchSelectorPage : PageBase { } void refresh_patchlist() { - is_populating_subdir_panel = true; - update_open_patches(); - subdir_panel.populate(patchfiles); - subdir_panel.show_recent_files(); - populate_roller(); + // Do not access patchfiles while M4 is accessing them + if (!patchfiles_locked) { + is_populating_subdir_panel = true; + update_open_patches(); + subdir_panel.populate(patchfiles); + subdir_panel.show_recent_files(); + populate_roller(); + } } void blur_subdir_panel() { @@ -271,6 +275,8 @@ struct PatchSelectorPage : PageBase { case State::TryingToRequestPatchList: if (patch_storage.request_patchlist(gui_state.force_refresh_vol)) { + // Lock patchesfiles: we are not allowed to access it, because M4 has access now + patchfiles_locked = true; state = State::RequestedPatchList; show_spinner(); } @@ -293,10 +299,17 @@ struct PatchSelectorPage : PageBase { patches.mark_patches_no_reload(Volume::SDCard); } state = State::ReloadingPatchList; + + // Unlock patchesfiles: M4 is done with it + patchfiles_locked = false; + } else if (message.message_type == FileStorageProxy::PatchListUnchanged) { gui_state.force_refresh_vol = std::nullopt; hide_spinner(); state = State::Idle; + + // Unlock patchesfiles: M4 is done with it + patchfiles_locked = false; } } break; @@ -434,7 +447,7 @@ private: auto selected_patch = page->get_roller_item_patchloc(idx); if (selected_patch) { page->selected_patch = *selected_patch; - page->state = State::TryingToRequestPatchData; // will load from RAM if open + page->state = State::TryingToRequestPatchData; page->last_selected_idx = idx; } else { //Do nothing? Close/open directory? Focus subdir panel? @@ -470,6 +483,7 @@ private: PatchSelectorSubdirPanel &subdir_panel; PatchDirList &patchfiles; + bool patchfiles_locked = true; bool is_populating_subdir_panel = false;