Refactor channel/signal handling
authorSoeren Apel <soeren@apelpie.net>
Tue, 5 May 2020 17:14:18 +0000 (19:14 +0200)
committerSoeren Apel <soeren@apelpie.net>
Mon, 11 May 2020 19:24:22 +0000 (21:24 +0200)
pv/session.cpp
pv/session.hpp
pv/views/trace/view.cpp
pv/views/trace/view.hpp
pv/views/viewbase.cpp
pv/views/viewbase.hpp

index d50a9dd6a41420c16a92d70c3898104657004064..fb8ee5fe9a3456dc2533078cdcbdad59b040e36a 100644 (file)
@@ -500,7 +500,7 @@ void Session::set_device(shared_ptr<devices::Device> device)
 
        // Remove all stored data and reset all views
        for (shared_ptr<views::ViewBase> view : views_) {
-               view->clear_signals();
+               view->clear_signalbases();
 #ifdef ENABLE_DECODE
                view->clear_decode_signals();
 #endif
@@ -935,7 +935,7 @@ void Session::update_signals()
                signalbases_.clear();
                logic_data_.reset();
                for (shared_ptr<views::ViewBase>& view : views_) {
-                       view->clear_signals();
+                       view->clear_signalbases();
 #ifdef ENABLE_DECODE
                        view->clear_decode_signals();
 #endif
@@ -950,7 +950,7 @@ void Session::update_signals()
                signalbases_.clear();
                logic_data_.reset();
                for (shared_ptr<views::ViewBase>& view : views_) {
-                       view->clear_signals();
+                       view->clear_signalbases();
 #ifdef ENABLE_DECODE
                        view->clear_decode_signals();
 #endif
@@ -965,7 +965,7 @@ void Session::update_signals()
                [] (shared_ptr<Channel> channel) {
                        return channel->type() == sigrok::ChannelType::LOGIC; });
 
-       // Create data containers for the logic data segments
+       // Create a common data container for the logic signalbases
        {
                lock_guard<recursive_mutex> data_lock(data_mutex_);
 
@@ -973,100 +973,77 @@ void Session::update_signals()
                        logic_data_.reset();
                } else if (!logic_data_ ||
                        logic_data_->num_channels() != logic_channel_count) {
-                       logic_data_.reset(new data::Logic(
-                               logic_channel_count));
+                       logic_data_.reset(new data::Logic(logic_channel_count));
                        assert(logic_data_);
                }
        }
 
-       // Make the signals list
-       for (shared_ptr<views::ViewBase>& viewbase : views_) {
-               views::trace::View *trace_view =
-                       qobject_cast<views::trace::View*>(viewbase.get());
-
-               if (trace_view) {
-                       vector< shared_ptr<Signal> > prev_sigs(trace_view->signals());
-                       trace_view->clear_signals();
-
-                       for (auto channel : sr_dev->channels()) {
-                               shared_ptr<data::SignalBase> signalbase;
-                               shared_ptr<Signal> signal;
-
-                               // Find the channel in the old signals
-                               const auto iter = find_if(
-                                       prev_sigs.cbegin(), prev_sigs.cend(),
-                                       [&](const shared_ptr<Signal> &s) {
-                                               return s->base()->channel() == channel;
-                                       });
-                               if (iter != prev_sigs.end()) {
-                                       // Copy the signal from the old set to the new
-                                       signal = *iter;
-                                       trace_view->add_signal(signal);
-                               } else {
-                                       // Find the signalbase for this channel if possible
-                                       signalbase.reset();
-                                       for (const shared_ptr<data::SignalBase>& b : signalbases_)
-                                               if (b->channel() == channel)
-                                                       signalbase = b;
-
-                                       shared_ptr<Signal> signal;
-
-                                       switch(channel->type()->id()) {
-                                       case SR_CHANNEL_LOGIC:
-                                               if (!signalbase) {
-                                                       signalbase = make_shared<data::SignalBase>(channel,
-                                                               data::SignalBase::LogicChannel);
-                                                       signalbases_.push_back(signalbase);
-
-                                                       all_signal_data_.insert(logic_data_);
-                                                       signalbase->set_data(logic_data_);
-
-                                                       connect(this, SIGNAL(capture_state_changed(int)),
-                                                               signalbase.get(), SLOT(on_capture_state_changed(int)));
-                                               }
-
-                                               signal = shared_ptr<Signal>(new LogicSignal(*this, device_, signalbase));
-                                               break;
-
-                                       case SR_CHANNEL_ANALOG:
-                                       {
-                                               if (!signalbase) {
-                                                       signalbase = make_shared<data::SignalBase>(channel,
-                                                               data::SignalBase::AnalogChannel);
-                                                       signalbases_.push_back(signalbase);
-
-                                                       shared_ptr<data::Analog> data(new data::Analog());
-                                                       all_signal_data_.insert(data);
-                                                       signalbase->set_data(data);
-
-                                                       connect(this, SIGNAL(capture_state_changed(int)),
-                                                               signalbase.get(), SLOT(on_capture_state_changed(int)));
-                                               }
-
-                                               signal = shared_ptr<Signal>(new AnalogSignal(*this, signalbase));
-                                               break;
-                                       }
-
-                                       default:
-                                               assert(false);
-                                               break;
-                                       }
-
-                                       // New views take their signal settings from the main view
-                                       if (!viewbase->is_main_view()) {
-                                               shared_ptr<pv::views::trace::View> main_tv =
-                                                       dynamic_pointer_cast<pv::views::trace::View>(main_view_);
-                                               shared_ptr<Signal> main_signal =
-                                                       main_tv->get_signal_by_signalbase(signalbase);
-                                               signal->restore_settings(main_signal->save_settings());
-                                       }
-
-                                       trace_view->add_signal(signal);
-                               }
+       // Create signalbases if necessary
+       for (auto channel : sr_dev->channels()) {
+
+               // Try to find the channel in the list of existing signalbases
+               const auto iter = find_if(signalbases_.cbegin(), signalbases_.cend(),
+                       [&](const shared_ptr<SignalBase> &sb) { return sb->channel() == channel; });
+
+               // Not found, let's make a signalbase for it
+               if (iter == signalbases_.cend()) {
+                       shared_ptr<SignalBase> signalbase;
+                       switch(channel->type()->id()) {
+                       case SR_CHANNEL_LOGIC:
+                               signalbase = make_shared<data::SignalBase>(channel, data::SignalBase::LogicChannel);
+                               signalbases_.push_back(signalbase);
+
+                               all_signal_data_.insert(logic_data_);
+                               signalbase->set_data(logic_data_);
+
+                               connect(this, SIGNAL(capture_state_changed(int)),
+                                               signalbase.get(), SLOT(on_capture_state_changed(int)));
+                               break;
+
+                       case SR_CHANNEL_ANALOG:
+                               signalbase = make_shared<data::SignalBase>(channel, data::SignalBase::AnalogChannel);
+                               signalbases_.push_back(signalbase);
+
+                               shared_ptr<data::Analog> data(new data::Analog());
+                               all_signal_data_.insert(data);
+                               signalbase->set_data(data);
+
+                               connect(this, SIGNAL(capture_state_changed(int)),
+                                               signalbase.get(), SLOT(on_capture_state_changed(int)));
+                               break;
                        }
                }
        }
 
+       for (shared_ptr<views::ViewBase>& viewbase : views_) {
+               vector< shared_ptr<SignalBase> > view_signalbases =
+                               viewbase->signalbases();
+
+               // Add all non-decode signalbases that don't yet exist in the view
+               for (shared_ptr<SignalBase>& session_sb : signalbases_) {
+                       if (session_sb->type() == SignalBase::DecodeChannel)
+                               continue;
+
+                       const auto iter = find_if(view_signalbases.cbegin(), view_signalbases.cend(),
+                               [&](const shared_ptr<SignalBase> &sb) { return sb == session_sb; });
+
+                       if (iter == view_signalbases.cend())
+                               viewbase->add_signalbase(session_sb);
+               }
+
+               // Remove all non-decode signalbases that no longer exist
+               for (shared_ptr<SignalBase>& view_sb : view_signalbases) {
+                       if (view_sb->type() == SignalBase::DecodeChannel)
+                               continue;
+
+                       const auto iter = find_if(signalbases_.cbegin(), signalbases_.cend(),
+                               [&](const shared_ptr<SignalBase> &sb) { return sb == view_sb; });
+
+                       if (iter == signalbases_.cend())
+                               viewbase->remove_signalbase(view_sb);
+               }
+       }
+
        signals_changed();
 }
 
index ac3a076d2fb8d5e0edda93c86a849b5e16310847..eefdd12bd20bf67262b17f5e2948c47d77d63a35 100644 (file)
@@ -32,7 +32,6 @@
 #include <set>
 #include <string>
 #include <thread>
-#include <unordered_set>
 #include <vector>
 
 #include <QObject>
@@ -270,13 +269,13 @@ private:
 
        shared_ptr<pv::toolbars::MainBar> main_bar_;
 
-       mutable mutex sampling_mutex_; //!< Protects access to capture_state_.
+       mutable mutex sampling_mutex_; //!< Protects access to capture_state_
        capture_state capture_state_;
 
        vector< shared_ptr<data::SignalBase> > signalbases_;
        unordered_set< shared_ptr<data::SignalData> > all_signal_data_;
 
-       /// trigger_list_ contains pairs of <segment_id, timestamp> values.
+       /// trigger_list_ contains pairs of <segment_id, timestamp> values
        vector< std::pair<uint32_t, util::Timestamp> > trigger_list_;
 
        mutable recursive_mutex data_mutex_;
index 93bcb52072f01ea2e7fe61b433e5849606013323..46bae5d8b27755287fe17a187ded3ea31e79cb59 100644 (file)
@@ -72,6 +72,7 @@ using std::back_inserter;
 using std::copy_if;
 using std::count_if;
 using std::inserter;
+using std::lock_guard;
 using std::max;
 using std::make_pair;
 using std::make_shared;
@@ -341,24 +342,55 @@ shared_ptr<Signal> View::get_signal_by_signalbase(shared_ptr<data::SignalBase> b
        return ret_val;
 }
 
-void View::clear_signals()
+void View::clear_signalbases()
 {
-       ViewBase::clear_signals();
+       ViewBase::clear_signalbases();
        signals_.clear();
 }
 
-void View::add_signal(const shared_ptr<Signal> signal)
+void View::add_signalbase(const shared_ptr<data::SignalBase> signalbase)
 {
-       ViewBase::add_signalbase(signal->base());
+       ViewBase::add_signalbase(signalbase);
+
+       shared_ptr<Signal> signal;
+
+       switch (signalbase->type()) {
+       case SignalBase::LogicChannel:
+               signal = shared_ptr<Signal>(new LogicSignal(session_, session_.device(), signalbase));
+               break;
+
+       case SignalBase::AnalogChannel:
+               signal = shared_ptr<Signal>(new AnalogSignal(session_, signalbase));
+               break;
+
+       default:
+               qDebug() << "Unknown signalbase type:" << signalbase->type();
+               assert(false);
+               break;
+       }
+
        signals_.push_back(signal);
 
        signal->set_segment_display_mode(segment_display_mode_);
        signal->set_current_segment(current_segment_);
 
+       // Secondary views use the signal's settings in the main view
+       if (!is_main_view()) {
+               shared_ptr<View> main_tv = dynamic_pointer_cast<View>(session_.main_view());
+               shared_ptr<Signal> main_signal = main_tv->get_signal_by_signalbase(signalbase);
+               if (main_signal)
+                       signal->restore_settings(main_signal->save_settings());
+       }
+
        connect(signal->base().get(), SIGNAL(name_changed(const QString&)),
                this, SLOT(on_signal_name_changed()));
 }
 
+void View::remove_signalbase(const shared_ptr<data::SignalBase> signalbase)
+{
+       ViewBase::remove_signalbase(signalbase);
+}
+
 #ifdef ENABLE_DECODE
 void View::clear_decode_signals()
 {
@@ -892,7 +924,7 @@ pair<Timestamp, Timestamp> View::get_time_extents() const
        if (!left_time || !right_time)
                return make_pair(0, 0);
 
-       assert(*left_time < *right_time);
+       assert(*left_time <= *right_time);
        return make_pair(*left_time, *right_time);
 }
 
@@ -1705,6 +1737,8 @@ void View::signals_changed()
 {
        using sigrok::Channel;
 
+       lock_guard<mutex> lock(signal_mutex_);
+
        vector< shared_ptr<Channel> > channels;
        shared_ptr<sigrok::Device> sr_dev;
        bool signals_added_or_removed = false;
@@ -1727,7 +1761,9 @@ void View::signals_changed()
        vector< shared_ptr<TraceTreeItem> > new_top_level_items;
 
        // Make a list of traces that are being added, and a list of traces
-       // that are being removed
+       // that are being removed. The set_difference() algorithms require
+       // both sets to be in the exact same order, which means that PD signals
+       // must always be last as they interrupt the sort order otherwise
        const vector<shared_ptr<Trace>> prev_trace_list = list_by_type<Trace>();
        const set<shared_ptr<Trace>> prev_traces(
                prev_trace_list.begin(), prev_trace_list.end());
index 8183fefc6c0b814a0cf96351e3a0d047fded990c..0adab674b10cfdeaa513bce11d7b6d871aa8ee1a 100644 (file)
@@ -23,6 +23,7 @@
 #include <cstdint>
 #include <list>
 #include <memory>
+#include <mutex>
 #include <set>
 #include <vector>
 
@@ -121,9 +122,9 @@ public:
 
        shared_ptr<Signal> get_signal_by_signalbase(shared_ptr<data::SignalBase> base) const;
 
-       virtual void clear_signals();
-
-       void add_signal(const shared_ptr<Signal> signal);
+       virtual void clear_signalbases();
+       virtual void add_signalbase(const shared_ptr<data::SignalBase> signalbase);
+       virtual void remove_signalbase(const shared_ptr<data::SignalBase> signalbase);
 
 #ifdef ENABLE_DECODE
        virtual void clear_decode_signals();
@@ -508,6 +509,7 @@ private:
        QShortcut *grab_ruler_left_shortcut_, *grab_ruler_right_shortcut_;
        QShortcut *cancel_grab_shortcut_;
 
+       mutable mutex signal_mutex_;
        vector< shared_ptr<Signal> > signals_;
 
 #ifdef ENABLE_DECODE
index 7be8822263c12111ec1bb0a6bb2e78ef7c88e607..6dd83c675c9b932e5ceb7d0a0e3fafac0f5ec9ac 100644 (file)
@@ -84,11 +84,6 @@ const Session& ViewBase::session() const
        return session_;
 }
 
-void ViewBase::clear_signals()
-{
-       clear_signalbases();
-}
-
 vector< shared_ptr<data::SignalBase> > ViewBase::signalbases() const
 {
        return signalbases_;
index 73d7a179c5a4e78fa446b1ec8a040fed98e6820b..5e9f8fdc48776fa3e8e8a9771e82f87fe6ea2ef2 100644 (file)
@@ -85,8 +85,6 @@ public:
        Session& session();
        const Session& session() const;
 
-       virtual void clear_signals();
-
        /**
         * Returns the signal bases contained in this view.
         */