Bookmarks: fix for #525 item 2, other cleanups.

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) {
+const BookmarkList& BookmarkMgr::getBookmarks(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 results;
+        return emptyResults;
-        return results;
     }
     
     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() {
+const BookmarkList& BookmarkMgr::getRecents() {
-    std::lock_guard < std::mutex > lockData(busy_lock);
+    std::lock_guard < std::recursive_mutex > lockData(busy_lock);
-    return BookmarkList(recents.rbegin(), recents.rend());
+    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() {
+const BookmarkRangeList& BookmarkMgr::getRanges() {
-    std::lock_guard < std::mutex > lock(busy_lock);
+    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);
+}
+

src/BookmarkMgr.h

@@ -19,8 +19,8 @@ public:
     std::mutex busy_lock;
 
     std::string type;
+	//maps on the Demod user label.
     std::wstring label; 
-    std::wstring userLabel;
     
     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,12 +108,14 @@ 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);
@@ -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;
 };

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<DemodulatorInstance *>::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,6 +435,9 @@ 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();
@@ -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<int, RtAudio::DeviceInfo>::iterator i;
     for (i = outputDevices.begin(); i != outputDevices.end(); i++) {
         if (i->second.name == output_device) {
             newDemod->setOutputDevice(i->first);
             found_device = true;
+			break;
         }
     }
     

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<int,RtAudio::DeviceInfo> devs);
     void saveInstance(DataNode *node, DemodulatorInstance *inst);
+	
     DemodulatorInstance *loadInstance(DataNode *node);
     
 private:

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().getBookmarkMgr().removeActive(demod);
-    wxGetApp().removeDemodulator(demod);
+	wxGetApp().getBookmarkMgr().updateActiveList();
-    wxGetApp().getDemodMgr().deleteThread(demod);
 }
 
 
@@ -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);
+	}
 
-    wxTreeItemId selItem = m_treeView->GetSelection();
+	//if a matching DemodulatorInstance do not exist yet, create it and activate it, else use
-    if (selItem) {
+	//the already existing one:
-        m_treeView->SelectItem(selItem, false);
+	// 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) {
 
-    long long freq = newDemod->getFrequency();
+		matchingDemod = wxGetApp().getDemodMgr().loadInstance(bmEnt->node);
-    long long currentFreq = wxGetApp().getFrequency();
+		matchingDemod->run();
-    long long currentRate = wxGetApp().getSampleRate();
+		wxGetApp().bindDemodulator(matchingDemod);
+	}
 
-    if ( ( abs(freq - currentFreq) > currentRate / 2 ) || ( abs( currentFreq - freq) > currentRate / 2 ) ) {
+	matchingDemod->setActive(true);
-        wxGetApp().setFrequency(freq);
-    }
 
-    newDemod->run();
+	long long freq = matchingDemod->getFrequency();
-    newDemod->setActive(true);
+	long long currentFreq = wxGetApp().getFrequency();
-    wxGetApp().bindDemodulator(newDemod);
+	long long currentRate = wxGetApp().getSampleRate();
 
-    //order immediate refresh of the whole tree.
+	if ((abs(freq - currentFreq) > currentRate / 2) || (abs(currentFreq - freq) > currentRate / 2)) {
-    doUpdateActiveList();
+		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());
     }
 }