From: Soeren Apel Date: Tue, 5 May 2020 17:14:18 +0000 (+0200) Subject: Refactor channel/signal handling X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=578d073553fa13c8f6939ad4bd7bd774950a33eb;p=pulseview.git Refactor channel/signal handling --- diff --git a/pv/session.cpp b/pv/session.cpp index d50a9dd6..fb8ee5fe 100644 --- a/pv/session.cpp +++ b/pv/session.cpp @@ -500,7 +500,7 @@ void Session::set_device(shared_ptr device) // Remove all stored data and reset all views for (shared_ptr 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& 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& 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) { 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 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& viewbase : views_) { - views::trace::View *trace_view = - qobject_cast(viewbase.get()); - - if (trace_view) { - vector< shared_ptr > prev_sigs(trace_view->signals()); - trace_view->clear_signals(); - - for (auto channel : sr_dev->channels()) { - shared_ptr signalbase; - shared_ptr signal; - - // Find the channel in the old signals - const auto iter = find_if( - prev_sigs.cbegin(), prev_sigs.cend(), - [&](const shared_ptr &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& b : signalbases_) - if (b->channel() == channel) - signalbase = b; - - shared_ptr signal; - - switch(channel->type()->id()) { - case SR_CHANNEL_LOGIC: - if (!signalbase) { - signalbase = make_shared(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(new LogicSignal(*this, device_, signalbase)); - break; - - case SR_CHANNEL_ANALOG: - { - if (!signalbase) { - signalbase = make_shared(channel, - data::SignalBase::AnalogChannel); - signalbases_.push_back(signalbase); - - shared_ptr 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(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 main_tv = - dynamic_pointer_cast(main_view_); - shared_ptr 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 &sb) { return sb->channel() == channel; }); + + // Not found, let's make a signalbase for it + if (iter == signalbases_.cend()) { + shared_ptr signalbase; + switch(channel->type()->id()) { + case SR_CHANNEL_LOGIC: + signalbase = make_shared(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(channel, data::SignalBase::AnalogChannel); + signalbases_.push_back(signalbase); + + shared_ptr 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& viewbase : views_) { + vector< shared_ptr > view_signalbases = + viewbase->signalbases(); + + // Add all non-decode signalbases that don't yet exist in the view + for (shared_ptr& session_sb : signalbases_) { + if (session_sb->type() == SignalBase::DecodeChannel) + continue; + + const auto iter = find_if(view_signalbases.cbegin(), view_signalbases.cend(), + [&](const shared_ptr &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& view_sb : view_signalbases) { + if (view_sb->type() == SignalBase::DecodeChannel) + continue; + + const auto iter = find_if(signalbases_.cbegin(), signalbases_.cend(), + [&](const shared_ptr &sb) { return sb == view_sb; }); + + if (iter == signalbases_.cend()) + viewbase->remove_signalbase(view_sb); + } + } + signals_changed(); } diff --git a/pv/session.hpp b/pv/session.hpp index ac3a076d..eefdd12b 100644 --- a/pv/session.hpp +++ b/pv/session.hpp @@ -32,7 +32,6 @@ #include #include #include -#include #include #include @@ -270,13 +269,13 @@ private: shared_ptr 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 > signalbases_; unordered_set< shared_ptr > all_signal_data_; - /// trigger_list_ contains pairs of values. + /// trigger_list_ contains pairs of values vector< std::pair > trigger_list_; mutable recursive_mutex data_mutex_; diff --git a/pv/views/trace/view.cpp b/pv/views/trace/view.cpp index 93bcb520..46bae5d8 100644 --- a/pv/views/trace/view.cpp +++ b/pv/views/trace/view.cpp @@ -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 View::get_signal_by_signalbase(shared_ptr 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) +void View::add_signalbase(const shared_ptr signalbase) { - ViewBase::add_signalbase(signal->base()); + ViewBase::add_signalbase(signalbase); + + shared_ptr signal; + + switch (signalbase->type()) { + case SignalBase::LogicChannel: + signal = shared_ptr(new LogicSignal(session_, session_.device(), signalbase)); + break; + + case SignalBase::AnalogChannel: + signal = shared_ptr(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 main_tv = dynamic_pointer_cast(session_.main_view()); + shared_ptr 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 signalbase) +{ + ViewBase::remove_signalbase(signalbase); +} + #ifdef ENABLE_DECODE void View::clear_decode_signals() { @@ -892,7 +924,7 @@ pair 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 lock(signal_mutex_); + vector< shared_ptr > channels; shared_ptr sr_dev; bool signals_added_or_removed = false; @@ -1727,7 +1761,9 @@ void View::signals_changed() vector< shared_ptr > 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> prev_trace_list = list_by_type(); const set> prev_traces( prev_trace_list.begin(), prev_trace_list.end()); diff --git a/pv/views/trace/view.hpp b/pv/views/trace/view.hpp index 8183fefc..0adab674 100644 --- a/pv/views/trace/view.hpp +++ b/pv/views/trace/view.hpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -121,9 +122,9 @@ public: shared_ptr get_signal_by_signalbase(shared_ptr base) const; - virtual void clear_signals(); - - void add_signal(const shared_ptr signal); + virtual void clear_signalbases(); + virtual void add_signalbase(const shared_ptr signalbase); + virtual void remove_signalbase(const shared_ptr 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 > signals_; #ifdef ENABLE_DECODE diff --git a/pv/views/viewbase.cpp b/pv/views/viewbase.cpp index 7be88222..6dd83c67 100644 --- a/pv/views/viewbase.cpp +++ b/pv/views/viewbase.cpp @@ -84,11 +84,6 @@ const Session& ViewBase::session() const return session_; } -void ViewBase::clear_signals() -{ - clear_signalbases(); -} - vector< shared_ptr > ViewBase::signalbases() const { return signalbases_; diff --git a/pv/views/viewbase.hpp b/pv/views/viewbase.hpp index 73d7a179..5e9f8fdc 100644 --- a/pv/views/viewbase.hpp +++ b/pv/views/viewbase.hpp @@ -85,8 +85,6 @@ public: Session& session(); const Session& session() const; - virtual void clear_signals(); - /** * Returns the signal bases contained in this view. */