From c8cca67fc749242081f4c36b54f5ef40a6125c7e Mon Sep 17 00:00:00 2001 From: vsonnier Date: Sat, 1 Apr 2017 19:38:08 +0200 Subject: [PATCH] Bookmarks: fix for #525 item 2, other cleanups. --- src/BookmarkMgr.cpp | 84 +++++++++++++++++------------ src/BookmarkMgr.h | 19 ++++--- src/demod/DemodulatorMgr.cpp | 30 ++++++++++- src/demod/DemodulatorMgr.h | 5 ++ src/forms/Bookmark/BookmarkView.cpp | 79 +++++++++++++++------------ 5 files changed, 142 insertions(+), 75 deletions(-) diff --git a/src/BookmarkMgr.cpp b/src/BookmarkMgr.cpp index b314450..7f0770e 100644 --- a/src/BookmarkMgr.cpp +++ b/src/BookmarkMgr.cpp @@ -15,6 +15,9 @@ BookmarkMgr::BookmarkMgr() { rangesSorted = false; } +//represents an empty BookMarkList that is returned by reference by some functions. +const BookmarkList BookmarkMgr::emptyResults; + void BookmarkMgr::saveToFile(std::string bookmarkFn, bool backup) { DataTree s("cubicsdr_bookmarks"); DataNode *header = s.rootNode()->newChild("header"); @@ -203,19 +206,18 @@ bool BookmarkMgr::hasBackup(std::string bookmarkFn) { } void BookmarkMgr::addBookmark(std::string group, DemodulatorInstance *demod) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); + //Create a BookmarkEntry from demod data, saving its + //characteristics in be->node. BookmarkEntryPtr be = demodToBookmarkEntry(demod); - - //copy settings of demod into be->node - wxGetApp().getDemodMgr().saveInstance(be->node, demod); - + bmData[group].push_back(be); bmDataSorted[group] = false; } void BookmarkMgr::addBookmark(std::string group, BookmarkEntryPtr be) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); bmData[group].push_back(be); bmDataSorted[group] = false; @@ -223,7 +225,7 @@ void BookmarkMgr::addBookmark(std::string group, BookmarkEntryPtr be) { void BookmarkMgr::removeBookmark(std::string group, BookmarkEntryPtr be) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); std::lock_guard < std::mutex > lockEnt(be->busy_lock); if (bmData.find(group) == bmData.end()) { @@ -239,7 +241,7 @@ void BookmarkMgr::removeBookmark(std::string group, BookmarkEntryPtr be) { } void BookmarkMgr::removeBookmark(BookmarkEntryPtr be) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); std::lock_guard < std::mutex > lockEnt(be->busy_lock); for (auto &bmd_i : bmData) { @@ -251,7 +253,7 @@ void BookmarkMgr::removeBookmark(BookmarkEntryPtr be) { } void BookmarkMgr::moveBookmark(BookmarkEntryPtr be, std::string group) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); std::lock_guard < std::mutex > lockEnt(be->busy_lock); for (auto &bmd_i : bmData) { @@ -271,7 +273,7 @@ void BookmarkMgr::moveBookmark(BookmarkEntryPtr be, std::string group) { void BookmarkMgr::addGroup(std::string group) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); if (bmData.find(group) == bmData.end()) { BookmarkList dummy = bmData[group]; @@ -279,7 +281,7 @@ void BookmarkMgr::addGroup(std::string group) { } void BookmarkMgr::removeGroup(std::string group) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); BookmarkMap::iterator i = bmData.find(group); @@ -294,7 +296,7 @@ void BookmarkMgr::renameGroup(std::string group, std::string ngroup) { return; } - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); BookmarkMap::iterator i = bmData.find(group); BookmarkMap::iterator it = bmData.find(ngroup); @@ -310,12 +312,11 @@ void BookmarkMgr::renameGroup(std::string group, std::string ngroup) { } } -BookmarkList BookmarkMgr::getBookmarks(std::string group) { - std::lock_guard < std::mutex > lock(busy_lock); +const BookmarkList& BookmarkMgr::getBookmarks(std::string group) { + std::lock_guard < std::recursive_mutex > lock(busy_lock); if (bmData.find(group) == bmData.end()) { - BookmarkList results; - return results; + return emptyResults; } if (!bmDataSorted[group]) { @@ -328,7 +329,7 @@ BookmarkList BookmarkMgr::getBookmarks(std::string group) { void BookmarkMgr::getGroups(BookmarkNames &arr) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); for (BookmarkMap::iterator i = bmData.begin(); i!= bmData.end(); ++i) { arr.push_back(i->first); @@ -336,7 +337,7 @@ void BookmarkMgr::getGroups(BookmarkNames &arr) { } void BookmarkMgr::getGroups(wxArrayString &arr) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); for (BookmarkMap::iterator i = bmData.begin(); i!= bmData.end(); ++i) { arr.push_back(i->first); @@ -359,7 +360,9 @@ bool BookmarkMgr::getExpandState(std::string groupName) { void BookmarkMgr::updateActiveList() { - BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); + + BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); if (bmv) { bmv->updateActiveList(); @@ -368,7 +371,7 @@ void BookmarkMgr::updateActiveList() { void BookmarkMgr::updateBookmarks() { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); @@ -379,7 +382,7 @@ void BookmarkMgr::updateBookmarks() { void BookmarkMgr::updateBookmarks(std::string group) { - std::lock_guard < std::mutex > lockData(busy_lock); + std::lock_guard < std::recursive_mutex > lockData(busy_lock); BookmarkView *bmv = wxGetApp().getAppFrame()->getBookmarkView(); @@ -390,7 +393,7 @@ void BookmarkMgr::updateBookmarks(std::string group) { void BookmarkMgr::addRecent(DemodulatorInstance *demod) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); recents.push_back(demodToBookmarkEntry(demod)); @@ -398,7 +401,7 @@ void BookmarkMgr::addRecent(DemodulatorInstance *demod) { } void BookmarkMgr::addRecent(BookmarkEntryPtr be) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); recents.push_back(be); @@ -408,7 +411,7 @@ void BookmarkMgr::addRecent(BookmarkEntryPtr be) { void BookmarkMgr::removeRecent(BookmarkEntryPtr be) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); BookmarkList::iterator bm_i = std::find(recents.begin(),recents.end(), be); @@ -418,14 +421,14 @@ void BookmarkMgr::removeRecent(BookmarkEntryPtr be) { } -BookmarkList BookmarkMgr::getRecents() { - std::lock_guard < std::mutex > lockData(busy_lock); - return BookmarkList(recents.rbegin(), recents.rend()); +const BookmarkList& BookmarkMgr::getRecents() { + std::lock_guard < std::recursive_mutex > lockData(busy_lock); + return recents; } void BookmarkMgr::clearRecents() { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); recents.clear(); } @@ -440,7 +443,7 @@ void BookmarkMgr::trimRecents() { void BookmarkMgr::addRange(BookmarkRangeEntryPtr re) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); ranges.push_back(re); rangesSorted = false; @@ -449,7 +452,7 @@ void BookmarkMgr::addRange(BookmarkRangeEntryPtr re) { void BookmarkMgr::removeRange(BookmarkRangeEntryPtr re) { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); BookmarkRangeList::iterator re_i = std::find(ranges.begin(), ranges.end(), re); @@ -460,8 +463,8 @@ void BookmarkMgr::removeRange(BookmarkRangeEntryPtr re) { } -BookmarkRangeList BookmarkMgr::getRanges() { - std::lock_guard < std::mutex > lock(busy_lock); +const BookmarkRangeList& BookmarkMgr::getRanges() { + std::lock_guard < std::recursive_mutex > lock(busy_lock); if (!rangesSorted) { std::sort(ranges.begin(), ranges.end(), BookmarkRangeEntryCompare()); @@ -473,7 +476,7 @@ BookmarkRangeList BookmarkMgr::getRanges() { void BookmarkMgr::clearRanges() { - std::lock_guard < std::mutex > lock(busy_lock); + std::lock_guard < std::recursive_mutex > lock(busy_lock); ranges.clear(); } @@ -544,3 +547,18 @@ std::wstring BookmarkMgr::getActiveDisplayName(DemodulatorInstance *demod) { return activeName; } +void BookmarkMgr::removeActive(DemodulatorInstance *demod) { + + std::lock_guard < std::recursive_mutex > lock(busy_lock); + + if (demod == nullptr) { + return; + } + + //Delete demodulator + wxGetApp().getDemodMgr().setActiveDemodulator(nullptr, true); + wxGetApp().getDemodMgr().setActiveDemodulator(nullptr, false); + wxGetApp().removeDemodulator(demod); + wxGetApp().getDemodMgr().deleteThread(demod); +} + diff --git a/src/BookmarkMgr.h b/src/BookmarkMgr.h index b688799..5f18b69 100644 --- a/src/BookmarkMgr.h +++ b/src/BookmarkMgr.h @@ -19,8 +19,8 @@ public: std::mutex busy_lock; std::string type; - std::wstring label; - std::wstring userLabel; + //maps on the Demod user label. + std::wstring label; long long frequency; int bandwidth; @@ -94,7 +94,7 @@ public: void addGroup(std::string group); void removeGroup(std::string group); void renameGroup(std::string group, std::string ngroup); - BookmarkList getBookmarks(std::string group); + const BookmarkList& getBookmarks(std::string group); void getGroups(BookmarkNames &arr); void getGroups(wxArrayString &arr); @@ -108,14 +108,16 @@ public: void addRecent(DemodulatorInstance *demod); void addRecent(BookmarkEntryPtr be); void removeRecent(BookmarkEntryPtr be); - BookmarkList getRecents(); + const BookmarkList& getRecents(); void clearRecents(); + void removeActive(DemodulatorInstance *demod); + void addRange(BookmarkRangeEntryPtr re); void removeRange(BookmarkRangeEntryPtr re); - BookmarkRangeList getRanges(); + const BookmarkRangeList& getRanges(); void clearRanges(); - + static std::wstring getBookmarkEntryDisplayName(BookmarkEntryPtr bmEnt); static std::wstring getActiveDisplayName(DemodulatorInstance *demod); @@ -131,7 +133,10 @@ protected: BookmarkList recents; BookmarkRangeList ranges; bool rangesSorted; - std::mutex busy_lock; + std::recursive_mutex busy_lock; BookmarkExpandState expandState; + + //represents an empty BookMarkList that is returned by reference by some functions. + static const BookmarkList emptyResults; }; diff --git a/src/demod/DemodulatorMgr.cpp b/src/demod/DemodulatorMgr.cpp index a2b02d1..4fa9e85 100644 --- a/src/demod/DemodulatorMgr.cpp +++ b/src/demod/DemodulatorMgr.cpp @@ -260,6 +260,27 @@ DemodulatorInstance *DemodulatorMgr::getLastActiveDemodulator() { return lastActiveDemodulator; } +DemodulatorInstance *DemodulatorMgr::getLastDemodulatorWith(const std::string& type, + const std::wstring& userLabel, + long long frequency, + int bandwidth) { + std::lock_guard < std::recursive_mutex > lock(demods_busy); + + //backwards search: + for (std::vector::reverse_iterator it = demods.rbegin(); it != demods.rend(); it++) { + + if ((*it)->getDemodulatorType() == type && + (*it)->getDemodulatorUserLabel() == userLabel && + (*it)->getFrequency() == frequency && + (*it)->getBandwidth() == bandwidth) { + + return (*it); + } + } + + return nullptr; +} + //Private internal method, no need to protect it with demods_busy void DemodulatorMgr::garbageCollect() { if (demods_deleted.size()) { @@ -414,8 +435,11 @@ void DemodulatorMgr::saveInstance(DataNode *node, DemodulatorInstance *inst) { } DemodulatorInstance *DemodulatorMgr::loadInstance(DataNode *node) { + + std::lock_guard < std::recursive_mutex > lock(demods_busy); + DemodulatorInstance *newDemod = nullptr; - + node->rewindAll(); long bandwidth = *node->getNext("bandwidth"); @@ -482,7 +506,7 @@ DemodulatorInstance *DemodulatorMgr::loadInstance(DataNode *node) { } } - newDemod = wxGetApp().getDemodMgr().newThread(); + newDemod = newThread(); newDemod->setDemodulatorType(type); newDemod->setDemodulatorUserLabel(user_label); @@ -501,12 +525,14 @@ DemodulatorInstance *DemodulatorMgr::loadInstance(DataNode *node) { newDemod->setSquelchLevel(squelch_level); } + //Attach to sound output: bool found_device = false; std::map::iterator i; for (i = outputDevices.begin(); i != outputDevices.end(); i++) { if (i->second.name == output_device) { newDemod->setOutputDevice(i->first); found_device = true; + break; } } diff --git a/src/demod/DemodulatorMgr.h b/src/demod/DemodulatorMgr.h index 41b0945..ca65c22 100644 --- a/src/demod/DemodulatorMgr.h +++ b/src/demod/DemodulatorMgr.h @@ -32,6 +32,10 @@ public: void setActiveDemodulator(DemodulatorInstance *demod, bool temporary = true); DemodulatorInstance *getActiveDemodulator(); DemodulatorInstance *getLastActiveDemodulator(); + DemodulatorInstance *getLastDemodulatorWith(const std::string& type, + const std::wstring& userLabel, + long long frequency, + int bandwidth); int getLastBandwidth() const; void setLastBandwidth(int lastBandwidth); @@ -61,6 +65,7 @@ public: void setOutputDevices(std::map devs); void saveInstance(DataNode *node, DemodulatorInstance *inst); + DemodulatorInstance *loadInstance(DataNode *node); private: diff --git a/src/forms/Bookmark/BookmarkView.cpp b/src/forms/Bookmark/BookmarkView.cpp index ecca424..883b813 100644 --- a/src/forms/Bookmark/BookmarkView.cpp +++ b/src/forms/Bookmark/BookmarkView.cpp @@ -299,7 +299,8 @@ wxTreeItemId BookmarkView::refreshBookmarks() { bool groupExpanded = searchState || wxGetApp().getBookmarkMgr().getExpandState(gn_i); - BookmarkList bmList = wxGetApp().getBookmarkMgr().getBookmarks(gn_i); + const BookmarkList& bmList = wxGetApp().getBookmarkMgr().getBookmarks(gn_i); + for (auto &bmEnt : bmList) { std::wstring labelVal = BookmarkMgr::getBookmarkEntryDisplayName(bmEnt); @@ -308,7 +309,7 @@ wxTreeItemId BookmarkView::refreshBookmarks() { std::string bwStr = frequencyToStr(bmEnt->bandwidth); std::wstring fullText = labelVal + - L" " + bmEnt->userLabel + + L" " + bmEnt->label + L" " + std::to_wstring(bmEnt->frequency) + L" " + std::wstring(freqStr.begin(),freqStr.end()) + L" " + std::wstring(bwStr.begin(),bwStr.end()) + @@ -406,7 +407,9 @@ void BookmarkView::doUpdateActiveList() { bool rangeExpandState = searchState?false:expandState["range"]; - BookmarkRangeList bmRanges = wxGetApp().getBookmarkMgr().getRanges(); + //Ranges + const BookmarkRangeList& bmRanges = wxGetApp().getBookmarkMgr().getRanges(); + m_treeView->DeleteChildren(rangeBranch); for (auto &re_i: bmRanges) { @@ -435,7 +438,7 @@ void BookmarkView::doUpdateActiveList() { bool recentExpandState = searchState || expandState["recent"]; // Recents - BookmarkList bmRecents = wxGetApp().getBookmarkMgr().getRecents(); + const BookmarkList& bmRecents = wxGetApp().getBookmarkMgr().getRecents(); m_treeView->DeleteChildren(recentBranch); for (auto &bmr_i: bmRecents) { @@ -457,7 +460,6 @@ void BookmarkView::doUpdateActiveList() { std::string bwStr = frequencyToStr(bmr_i->bandwidth); std::wstring fullText = labelVal + - L" " + bmr_i->userLabel + L" " + std::to_wstring(bmr_i->frequency) + L" " + std::wstring(freqStr.begin(),freqStr.end()) + L" " + std::wstring(bwStr.begin(),bwStr.end()) + @@ -716,10 +718,9 @@ void BookmarkView::doMoveBookmark(BookmarkEntryPtr be, std::string group) { void BookmarkView::doRemoveActive(DemodulatorInstance *demod) { - wxGetApp().getDemodMgr().setActiveDemodulator(nullptr, true); - wxGetApp().getDemodMgr().setActiveDemodulator(nullptr, false); - wxGetApp().removeDemodulator(demod); - wxGetApp().getDemodMgr().deleteThread(demod); + + wxGetApp().getBookmarkMgr().removeActive(demod); + wxGetApp().getBookmarkMgr().updateActiveList(); } @@ -824,29 +825,42 @@ void BookmarkView::activeSelection(DemodulatorInstance *dsel) { void BookmarkView::activateBookmark(BookmarkEntryPtr bmEnt) { - DemodulatorInstance *newDemod = wxGetApp().getDemodMgr().loadInstance(bmEnt->node); - - nextDemod = newDemod; - - wxTreeItemId selItem = m_treeView->GetSelection(); - if (selItem) { - m_treeView->SelectItem(selItem, false); - } - - long long freq = newDemod->getFrequency(); - long long currentFreq = wxGetApp().getFrequency(); - long long currentRate = wxGetApp().getSampleRate(); - - if ( ( abs(freq - currentFreq) > currentRate / 2 ) || ( abs( currentFreq - freq) > currentRate / 2 ) ) { - wxGetApp().setFrequency(freq); - } - - newDemod->run(); - newDemod->setActive(true); - wxGetApp().bindDemodulator(newDemod); - - //order immediate refresh of the whole tree. - doUpdateActiveList(); + + wxTreeItemId selItem = m_treeView->GetSelection(); + if (selItem) { + m_treeView->SelectItem(selItem, false); + } + + //if a matching DemodulatorInstance do not exist yet, create it and activate it, else use + //the already existing one: + // we search among the list of existing demodulators the one matching + //bmEnt and activate it. The search is made backwards, to select the most recently created one. + DemodulatorInstance *matchingDemod = wxGetApp().getDemodMgr().getLastDemodulatorWith( + bmEnt->type, + bmEnt->label, + bmEnt->frequency, + bmEnt->bandwidth); + //not found, create a new demod instance: + if (matchingDemod == nullptr) { + + matchingDemod = wxGetApp().getDemodMgr().loadInstance(bmEnt->node); + matchingDemod->run(); + wxGetApp().bindDemodulator(matchingDemod); + } + + matchingDemod->setActive(true); + + long long freq = matchingDemod->getFrequency(); + long long currentFreq = wxGetApp().getFrequency(); + long long currentRate = wxGetApp().getSampleRate(); + + if ((abs(freq - currentFreq) > currentRate / 2) || (abs(currentFreq - freq) > currentRate / 2)) { + wxGetApp().setFrequency(freq); + } + + nextDemod = matchingDemod; + + wxGetApp().getBookmarkMgr().updateActiveList(); } @@ -1132,7 +1146,6 @@ void BookmarkView::onRemoveActive( wxCommandEvent& /* event */ ) { return; } doRemoveActive(curSel->demod); - m_treeView->Delete(m_treeView->GetSelection()); } }