From: Soeren Apel Date: Sat, 12 Dec 2020 21:23:21 +0000 (+0100) Subject: Fix #1629 more by fixing the notification handling X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=db51130df74d9188d98e1a3b8073d9d482d6fa60;p=pulseview.git Fix #1629 more by fixing the notification handling --- diff --git a/pv/data/analog.cpp b/pv/data/analog.cpp index 9010097c..da9736b5 100644 --- a/pv/data/analog.cpp +++ b/pv/data/analog.cpp @@ -64,9 +64,11 @@ uint32_t Analog::get_segment_count() const void Analog::clear() { - segments_.clear(); + if (!segments_.empty()) { + segments_.clear(); - samples_cleared(); + samples_cleared(); + } } void Analog::set_samplerate(double value) diff --git a/pv/data/decodesignal.cpp b/pv/data/decodesignal.cpp index 640e5814..e076e8c0 100644 --- a/pv/data/decodesignal.cpp +++ b/pv/data/decodesignal.cpp @@ -231,9 +231,6 @@ void DecodeSignal::begin_decode() logic_mux_data_ = make_shared(ch_count); } - // Receive notifications when new sample data is available - connect_input_notifiers(); - if (get_input_segment_count() == 0) set_error_message(tr("No input data")); @@ -274,6 +271,9 @@ void DecodeSignal::auto_assign_signals(const shared_ptr dec) { bool new_assignment = false; + // Disconnect all input signal notifications so we don't have duplicate connections + disconnect_input_notifiers(); + // Try to auto-select channels that don't have signals assigned yet for (decode::DecodeChannel& ch : channels_) { // If a decoder is given, auto-assign only its channels @@ -321,6 +321,9 @@ void DecodeSignal::auto_assign_signals(const shared_ptr dec) } if (new_assignment) { + // Receive notifications when new sample data is available + connect_input_notifiers(); + logic_mux_data_invalid_ = true; stack_config_changed_ = true; commit_decoder_channels(); @@ -330,12 +333,18 @@ void DecodeSignal::auto_assign_signals(const shared_ptr dec) void DecodeSignal::assign_signal(const uint16_t channel_id, shared_ptr signal) { + // Disconnect all input signal notifications so we don't have duplicate connections + disconnect_input_notifiers(); + for (decode::DecodeChannel& ch : channels_) if (ch.id == channel_id) { ch.assigned_signal = signal; logic_mux_data_invalid_ = true; } + // Receive notifications when new sample data is available + connect_input_notifiers(); + stack_config_changed_ = true; commit_decoder_channels(); channels_updated(); @@ -1126,6 +1135,8 @@ void DecodeSignal::logic_mux_proc() // Logic mux data is being updated logic_mux_data_invalid_ = false; + connect_input_segment_notifiers(segment_id); + uint64_t samples_to_process; do { do { @@ -1164,6 +1175,9 @@ void DecodeSignal::logic_mux_proc() output_segment->set_complete(); if (segment_id < get_input_segment_count() - 1) { + + disconnect_input_segment_notifiers(segment_id); + // Process next segment segment_id++; @@ -1173,14 +1187,22 @@ void DecodeSignal::logic_mux_proc() logic_mux_data_->push_segment(output_segment); output_segment->set_samplerate(get_input_samplerate(segment_id)); + + connect_input_segment_notifiers(segment_id); } else { // Wait for more input data if we're processing the currently last segment unique_lock logic_mux_lock(logic_mux_mutex_); logic_mux_cond_.wait(logic_mux_lock); } + } else { + // Input segments aren't all complete yet but samples_to_process is 0, wait for more input data + unique_lock logic_mux_lock(logic_mux_mutex_); + logic_mux_cond_.wait(logic_mux_lock); } } } while (!logic_mux_interrupt_); + + disconnect_input_segment_notifiers(segment_id); } void DecodeSignal::decode_data( @@ -1308,6 +1330,10 @@ void DecodeSignal::decode_proc() unique_lock input_wait_lock(input_mutex_); decode_input_cond_.wait(input_wait_lock); } + } else { + // Input segment isn't complete yet but samples_to_process is 0, wait for more input data + unique_lock input_wait_lock(input_mutex_); + decode_input_cond_.wait(input_wait_lock); } } @@ -1419,23 +1445,72 @@ void DecodeSignal::stop_srd_session() void DecodeSignal::connect_input_notifiers() { - // Disconnect the notification slot from the previous set of signals - disconnect(this, SLOT(on_data_cleared())); - disconnect(this, SLOT(on_data_received())); - // Connect the currently used signals to our slot for (decode::DecodeChannel& ch : channels_) { if (!ch.assigned_signal) continue; + const data::SignalBase *signal = ch.assigned_signal.get(); + connect(signal, SIGNAL(samples_cleared()), + this, SLOT(on_data_cleared())); + connect(signal, SIGNAL(samples_added(uint64_t, uint64_t, uint64_t)), + this, SLOT(on_data_received())); + } +} +void DecodeSignal::disconnect_input_notifiers() +{ + // Disconnect the notification slot from the previous set of signals + for (decode::DecodeChannel& ch : channels_) { + if (!ch.assigned_signal) + continue; const data::SignalBase *signal = ch.assigned_signal.get(); - connect(signal, &data::SignalBase::samples_cleared, - this, &DecodeSignal::on_data_cleared); - connect(signal, &data::SignalBase::samples_added, - this, &DecodeSignal::on_data_received); + disconnect(signal, nullptr, this, SLOT(on_data_cleared())); + disconnect(signal, nullptr, this, SLOT(on_data_received())); } } +void DecodeSignal::connect_input_segment_notifiers(uint32_t segment_id) +{ + for (decode::DecodeChannel& ch : channels_) + if (ch.assigned_signal) { + const shared_ptr logic_data = ch.assigned_signal->logic_data(); + + shared_ptr segment; + if (segment_id < logic_data->logic_segments().size()) { + segment = logic_data->logic_segments().at(segment_id)->get_shared_ptr(); + } else { + qWarning() << "Signal" << name() << ":" << ch.assigned_signal->name() \ + << "has no logic segment, can't connect notifier" << segment_id; + continue; + } + + if (!segment) + continue; + + connect(segment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); + } +} + +void DecodeSignal::disconnect_input_segment_notifiers(uint32_t segment_id) +{ + for (decode::DecodeChannel& ch : channels_) + if (ch.assigned_signal) { + const shared_ptr logic_data = ch.assigned_signal->logic_data(); + + shared_ptr segment; + if (segment_id < logic_data->logic_segments().size()) { + segment = logic_data->logic_segments().at(segment_id)->get_shared_ptr(); + } else { + continue; + } + + if (!segment) + continue; + + disconnect(segment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); + } +} + void DecodeSignal::create_decode_segment() { // Create annotation segment @@ -1643,6 +1718,12 @@ void DecodeSignal::on_data_received() logic_mux_cond_.notify_one(); } +void DecodeSignal::on_input_segment_completed() +{ + if (!logic_mux_thread_.joinable()) + logic_mux_cond_.notify_one(); +} + void DecodeSignal::on_annotation_visibility_changed() { annotation_visibility_changed(); diff --git a/pv/data/decodesignal.hpp b/pv/data/decodesignal.hpp index f4783be2..527b988a 100644 --- a/pv/data/decodesignal.hpp +++ b/pv/data/decodesignal.hpp @@ -210,6 +210,9 @@ private: void stop_srd_session(); void connect_input_notifiers(); + void disconnect_input_notifiers(); + void connect_input_segment_notifiers(uint32_t segment_id); + void disconnect_input_segment_notifiers(uint32_t segment_id); void create_decode_segment(); @@ -230,6 +233,7 @@ private Q_SLOTS: void on_capture_state_changed(int state); void on_data_cleared(); void on_data_received(); + void on_input_segment_completed(); void on_annotation_visibility_changed(); diff --git a/pv/data/logic.cpp b/pv/data/logic.cpp index d2f89e85..ab779228 100644 --- a/pv/data/logic.cpp +++ b/pv/data/logic.cpp @@ -73,9 +73,11 @@ uint32_t Logic::get_segment_count() const void Logic::clear() { - segments_.clear(); + if (!segments_.empty()) { + segments_.clear(); - samples_cleared(); + samples_cleared(); + } } void Logic::set_samplerate(double value) diff --git a/pv/data/signalbase.cpp b/pv/data/signalbase.cpp index 578d908f..46ed8783 100644 --- a/pv/data/signalbase.cpp +++ b/pv/data/signalbase.cpp @@ -127,8 +127,8 @@ SignalBase::SignalBase(shared_ptr channel, ChannelType channel_ error_message_("") { if (channel_) { - internal_name_ = QString::fromStdString(channel_->name()); - index_ = channel_->index(); + set_internal_name(QString::fromStdString(channel_->name())); + set_index(channel_->index()); } connect(&delayed_conversion_starter_, SIGNAL(timeout()), @@ -219,6 +219,9 @@ QString SignalBase::internal_name() const void SignalBase::set_internal_name(QString internal_name) { internal_name_ = internal_name; + + // Use this name also for the QObject instance + setObjectName(internal_name); } QString SignalBase::display_name() const @@ -745,6 +748,9 @@ void SignalBase::convert_single_segment(shared_ptr asegment, // we do another round of sample conversion. } while ((complete_state != old_complete_state) || (end_sample - old_end_sample >= ConversionBlockSize)); + + if (complete_state) + lsegment->set_complete(); } void SignalBase::conversion_thread_proc() @@ -771,6 +777,7 @@ void SignalBase::conversion_thread_proc() shared_ptr asegment = analog_data->analog_segments().front(); assert(asegment); + connect(asegment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); const shared_ptr logic_data = dynamic_pointer_cast(converted_data_); assert(logic_data); @@ -792,11 +799,15 @@ void SignalBase::conversion_thread_proc() if (asegment->is_complete() && analog_data->analog_segments().size() > logic_data->logic_segments().size()) { + disconnect(asegment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); + // There are more segments to process segment_id++; try { asegment = analog_data->analog_segments().at(segment_id); + disconnect(asegment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); + connect(asegment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); } catch (out_of_range&) { qDebug() << "Conversion error for" << name() << ": no analog segment" \ << segment_id << ", segments size is" << analog_data->analog_segments().size(); @@ -808,14 +819,16 @@ void SignalBase::conversion_thread_proc() logic_data->push_segment(new_segment); lsegment = logic_data->logic_segments().back(); - } - - // No more samples/segments to process, wait for data or interrupt - if (!conversion_interrupt_) { - unique_lock input_lock(conversion_input_mutex_); - conversion_input_cond_.wait(input_lock); + } else { + // No more samples/segments to process, wait for data or interrupt + if (!conversion_interrupt_) { + unique_lock input_lock(conversion_input_mutex_); + conversion_input_cond_.wait(input_lock); + } } } while (!conversion_interrupt_); + + disconnect(asegment.get(), SIGNAL(completed()), this, SLOT(on_input_segment_completed())); } void SignalBase::start_conversion(bool delayed_start) @@ -827,10 +840,10 @@ void SignalBase::start_conversion(bool delayed_start) stop_conversion(); - if (converted_data_) + if (converted_data_ && (converted_data_->get_segment_count() > 0)) { converted_data_->clear(); - - samples_cleared(); + samples_cleared(); + } conversion_interrupt_ = false; conversion_thread_ = std::thread(&SignalBase::conversion_thread_proc, this); @@ -856,10 +869,10 @@ void SignalBase::stop_conversion() void SignalBase::on_samples_cleared() { - if (converted_data_) + if (converted_data_ && (converted_data_->get_segment_count() > 0)) { converted_data_->clear(); - - samples_cleared(); + samples_cleared(); + } } void SignalBase::on_samples_added(SharedPtrToSegment segment, uint64_t start_sample, @@ -879,6 +892,15 @@ void SignalBase::on_samples_added(SharedPtrToSegment segment, uint64_t start_sam samples_added(segment->segment_id(), start_sample, end_sample); } +void SignalBase::on_input_segment_completed() +{ + if (conversion_type_ != NoConversion) + if (conversion_thread_.joinable()) { + // Notify the conversion thread since it's running + conversion_input_cond_.notify_one(); + } +} + void SignalBase::on_min_max_changed(float min, float max) { // Restart conversion if one is enabled and uses a calculated threshold diff --git a/pv/data/signalbase.hpp b/pv/data/signalbase.hpp index ee927aae..e50b9e96 100644 --- a/pv/data/signalbase.hpp +++ b/pv/data/signalbase.hpp @@ -389,6 +389,8 @@ private Q_SLOTS: void on_samples_added(SharedPtrToSegment segment, uint64_t start_sample, uint64_t end_sample); + void on_input_segment_completed(); + void on_min_max_changed(float min, float max); void on_capture_state_changed(int state); diff --git a/pv/session.cpp b/pv/session.cpp index d30dbc06..f992d9f1 100644 --- a/pv/session.cpp +++ b/pv/session.cpp @@ -126,6 +126,8 @@ Session::Session(DeviceManager &device_manager, QString name) : cur_samplerate_(0), data_saved_(true) { + // Use this name also for the QObject instance + setObjectName(name_); } Session::~Session() @@ -175,6 +177,9 @@ void Session::set_name(QString name) name_ = name; + // Use this name also for the QObject instance + setObjectName(name_); + name_changed(); }