Session: Fix issue #67 by improving error handling

[pulseview.git] / pv / data / mathsignal.cpp

diff --git a/pv/data/mathsignal.cpp b/pv/data/mathsignal.cpp
index 2ad90212fe01eb5c8cd5cef764aaed6f1b3150b5..adb27131e6d43bc00a89e788cf398c07ed57229a 100644 (file)
--- a/pv/data/mathsignal.cpp
+++ b/pv/data/mathsignal.cpp
@@ -37,18 +37,24 @@ using std::unique_lock;
 namespace pv {
 namespace data {
 
 namespace pv {
 namespace data {
 

+#define MATH_ERR_NONE           0
+#define MATH_ERR_EMPTY_EXPR     1
+#define MATH_ERR_EXPRESSION     2
+#define MATH_ERR_INVALID_SIGNAL 3
+#define MATH_ERR_ENABLE         4
+

 const int64_t MathSignal::ChunkLength = 256 * 1024;
 
 
 template<typename T>
 const int64_t MathSignal::ChunkLength = 256 * 1024;
 
 
 template<typename T>

-struct sig_sample : public exprtk::igeneric_function<T>
+struct fnc_sample : public exprtk::igeneric_function<T>

 {
        typedef typename exprtk::igeneric_function<T>::parameter_list_t parameter_list_t;
        typedef typename exprtk::igeneric_function<T>::generic_type generic_type;
        typedef typename generic_type::scalar_view scalar_t;
        typedef typename generic_type::string_view string_t;
 
 {
        typedef typename exprtk::igeneric_function<T>::parameter_list_t parameter_list_t;
        typedef typename exprtk::igeneric_function<T>::generic_type generic_type;
        typedef typename generic_type::scalar_view scalar_t;
        typedef typename generic_type::string_view string_t;
 

-       sig_sample(MathSignal& owner) :
+       fnc_sample(MathSignal& owner) :

                exprtk::igeneric_function<T>("ST"),  // Require channel name and sample number
                owner_(owner),
                sig_data(nullptr)
                exprtk::igeneric_function<T>("ST"),  // Require channel name and sample number
                owner_(owner),
                sig_data(nullptr)


@@ -63,10 +69,16 @@ struct sig_sample : public exprtk::igeneric_function<T>
                const std::string str_sig_name = to_str(exprtk_sig_name);
                const double sample_num = exprtk_sample_num();
 
                const std::string str_sig_name = to_str(exprtk_sig_name);
                const double sample_num = exprtk_sample_num();
 

+               if (sample_num < 0)
+                       return 0;
+

                if (!sig_data)
                        sig_data = owner_.signal_from_name(str_sig_name);
 
                if (!sig_data)
                        sig_data = owner_.signal_from_name(str_sig_name);
 

-               assert(sig_data);
+               if (!sig_data)
+                       // There doesn't actually exist a signal with that name
+                       return 0;
+

                owner_.update_signal_sample(sig_data, current_segment, sample_num);
 
                return T(sig_data->sample_value);
                owner_.update_signal_sample(sig_data, current_segment, sample_num);
 
                return T(sig_data->sample_value);


@@ -84,12 +96,12 @@ MathSignal::MathSignal(pv::Session &session) :
        use_custom_sample_rate_(false),
        use_custom_sample_count_(false),
        expression_(""),
        use_custom_sample_rate_(false),
        use_custom_sample_count_(false),
        expression_(""),

-       error_message_(""),
+       error_type_(MATH_ERR_NONE),

        exprtk_unknown_symbol_table_(nullptr),
        exprtk_symbol_table_(nullptr),
        exprtk_expression_(nullptr),
        exprtk_parser_(nullptr),
        exprtk_unknown_symbol_table_(nullptr),
        exprtk_symbol_table_(nullptr),
        exprtk_expression_(nullptr),
        exprtk_parser_(nullptr),

-       fnc_sig_sample_(nullptr)
+       fnc_sample_(nullptr)

 {
        uint32_t sig_idx = session_.get_next_signal_index(MathChannel);
        set_name(QString(tr("Math%1")).arg(sig_idx));
 {
        uint32_t sig_idx = session_.get_next_signal_index(MathChannel);
        set_name(QString(tr("Math%1")).arg(sig_idx));


@@ -104,13 +116,12 @@ MathSignal::MathSignal(pv::Session &session) :
 MathSignal::~MathSignal()
 {
        reset_generation();
 MathSignal::~MathSignal()
 {
        reset_generation();

-
-       if (fnc_sig_sample_)
-               delete fnc_sig_sample_;

 }
 
 void MathSignal::save_settings(QSettings &settings) const
 {
 }
 
 void MathSignal::save_settings(QSettings &settings) const
 {

+       SignalBase::save_settings(settings);
+

        settings.setValue("expression", expression_);
 
        settings.setValue("custom_sample_rate", (qulonglong)custom_sample_rate_);
        settings.setValue("expression", expression_);
 
        settings.setValue("custom_sample_rate", (qulonglong)custom_sample_rate_);


@@ -121,6 +132,8 @@ void MathSignal::save_settings(QSettings &settings) const
 
 void MathSignal::restore_settings(QSettings &settings)
 {
 
 void MathSignal::restore_settings(QSettings &settings)
 {

+       SignalBase::restore_settings(settings);
+

        if (settings.contains("expression"))
                expression_ = settings.value("expression").toString();
 
        if (settings.contains("expression"))
                expression_ = settings.value("expression").toString();
 


@@ -137,11 +150,6 @@ void MathSignal::restore_settings(QSettings &settings)
                use_custom_sample_count_ = settings.value("use_custom_sample_count").toBool();
 }
 
                use_custom_sample_count_ = settings.value("use_custom_sample_count").toBool();
 }
 

-QString MathSignal::error_message() const
-{
-       return error_message_;
-}
-

 QString MathSignal::get_expression() const
 {
        return expression_;
 QString MathSignal::get_expression() const
 {
        return expression_;


@@ -154,11 +162,14 @@ void MathSignal::set_expression(QString expression)
        begin_generation();
 }
 
        begin_generation();
 }
 

-void MathSignal::set_error_message(QString msg)
+void MathSignal::set_error(uint8_t type, QString msg)

 {
 {

+       error_type_ = type;

        error_message_ = msg;
        // TODO Emulate noquote()
        error_message_ = msg;
        // TODO Emulate noquote()

-       qDebug().nospace() << name() << ": " << msg << "(Expression: '" << expression_ << "')";
+       qDebug().nospace() << name() << ": " << msg << "(Expression: '" + expression_ + "')";
+
+       error_message_changed(msg);

 }
 
 uint64_t MathSignal::get_working_sample_count(uint32_t segment_id) const
 }
 
 uint64_t MathSignal::get_working_sample_count(uint32_t segment_id) const


@@ -272,16 +283,19 @@ void MathSignal::reset_generation()
                exprtk_unknown_symbol_table_ = nullptr;
        }
 
                exprtk_unknown_symbol_table_ = nullptr;
        }
 

-       if (fnc_sig_sample_) {
-               delete fnc_sig_sample_;
-               fnc_sig_sample_ = nullptr;
+       if (fnc_sample_) {
+               delete fnc_sample_;
+               fnc_sample_ = nullptr;

        }
 
        if (!error_message_.isEmpty()) {
        }
 
        if (!error_message_.isEmpty()) {

-               error_message_ = QString();
+               error_message_.clear();
+               error_type_ = MATH_ERR_NONE;

                // TODO Emulate noquote()
                qDebug().nospace() << name() << ": Error cleared";
        }
                // TODO Emulate noquote()
                qDebug().nospace() << name() << ": Error cleared";
        }

+
+       generation_chunk_size_ = ChunkLength;

 }
 
 void MathSignal::begin_generation()
 }
 
 void MathSignal::begin_generation()


@@ -289,18 +303,25 @@ void MathSignal::begin_generation()
        reset_generation();
 
        if (expression_.isEmpty()) {
        reset_generation();
 
        if (expression_.isEmpty()) {

-               set_error_message(tr("No expression defined, nothing to do"));
+               set_error(MATH_ERR_EMPTY_EXPR, tr("No expression defined, nothing to do"));

                return;
        }
 
                return;
        }
 

-       disconnect(this, SLOT(on_data_received()));
+       disconnect(&session_, SIGNAL(data_received()), this, SLOT(on_data_received()));

 
 

-       fnc_sig_sample_ = new sig_sample<double>(*this);
+       for (const shared_ptr<SignalBase>& sb : session_.signalbases()) {
+               if (sb->analog_data())
+                       disconnect(sb->analog_data().get(), nullptr, this, SLOT(on_data_received()));
+               disconnect(sb.get(), nullptr, this, SLOT(on_enabled_changed()));
+       }
+
+       fnc_sample_ = new fnc_sample<double>(*this);

 
        exprtk_unknown_symbol_table_ = new exprtk::symbol_table<double>();
 
        exprtk_symbol_table_ = new exprtk::symbol_table<double>();
 
        exprtk_unknown_symbol_table_ = new exprtk::symbol_table<double>();
 
        exprtk_symbol_table_ = new exprtk::symbol_table<double>();

-       exprtk_symbol_table_->add_function("sig_sample", *fnc_sig_sample_);
+       exprtk_symbol_table_->add_constant("T", 1 / session_.get_samplerate());
+       exprtk_symbol_table_->add_function("sample", *fnc_sample_);

        exprtk_symbol_table_->add_variable("t", exprtk_current_time_);
        exprtk_symbol_table_->add_variable("s", exprtk_current_sample_);
        exprtk_symbol_table_->add_constants();
        exprtk_symbol_table_->add_variable("t", exprtk_current_time_);
        exprtk_symbol_table_->add_variable("s", exprtk_current_sample_);
        exprtk_symbol_table_->add_constants();


@@ -313,7 +334,25 @@ void MathSignal::begin_generation()
        exprtk_parser_->enable_unknown_symbol_resolver();
 
        if (!exprtk_parser_->compile(expression_.toStdString(), *exprtk_expression_)) {
        exprtk_parser_->enable_unknown_symbol_resolver();
 
        if (!exprtk_parser_->compile(expression_.toStdString(), *exprtk_expression_)) {

-               set_error_message(tr("Error in expression"));
+               QString error_details;
+               size_t error_count = exprtk_parser_->error_count();
+
+               for (size_t i = 0; i < error_count; i++) {
+                       typedef exprtk::parser_error::type error_t;
+                       error_t error = exprtk_parser_->get_error(i);
+                       exprtk::parser_error::update_error(error, expression_.toStdString());
+
+                       QString error_detail = tr("%1 at line %2, column %3: %4");
+                       if ((error_count > 1) && (i < (error_count - 1)))
+                               error_detail += "\n";
+
+                       error_details += error_detail \
+                               .arg(exprtk::parser_error::to_str(error.mode).c_str()) \
+                               .arg(error.line_no) \
+                               .arg(error.column_no) \
+                               .arg(error.diagnostic.c_str());
+               }
+               set_error(MATH_ERR_EXPRESSION, error_details);

        } else {
                // Resolve unknown scalars to signals and add them to the input signal list
                vector<string> unknowns;
        } else {
                // Resolve unknown scalars to signals and add them to the input signal list
                vector<string> unknowns;


@@ -322,32 +361,38 @@ void MathSignal::begin_generation()
                        signal_data* sig_data = signal_from_name(unknown);
                        const shared_ptr<SignalBase> signal = (sig_data) ? (sig_data->sb) : nullptr;
                        if (!signal || (!signal->analog_data())) {
                        signal_data* sig_data = signal_from_name(unknown);
                        const shared_ptr<SignalBase> signal = (sig_data) ? (sig_data->sb) : nullptr;
                        if (!signal || (!signal->analog_data())) {

-                               set_error_message(QString(tr("%1 isn't a valid analog signal")).arg(
-                                       QString::fromStdString(unknown)));
+                               set_error(MATH_ERR_INVALID_SIGNAL, QString(tr("\"%1\" isn't a valid analog signal")) \
+                                       .arg(QString::fromStdString(unknown)));

                        } else
                                sig_data->ref = &(exprtk_unknown_symbol_table_->variable_ref(unknown));
                }
        }
 
                        } else
                                sig_data->ref = &(exprtk_unknown_symbol_table_->variable_ref(unknown));
                }
        }
 

+       QString disabled_signals;
+       if (!all_input_signals_enabled(disabled_signals) && error_message_.isEmpty())
+               set_error(MATH_ERR_ENABLE,
+                       tr("No data will be generated as %1 must be enabled").arg(disabled_signals));
+

        if (error_message_.isEmpty()) {
                // Connect to the session data notification if we have no input signals
                if (input_signals_.empty())
        if (error_message_.isEmpty()) {
                // Connect to the session data notification if we have no input signals
                if (input_signals_.empty())

-                       connect(&session_, SIGNAL(data_received()),
-                               this, SLOT(on_data_received()));
+                       connect(&session_, SIGNAL(data_received()),     this, SLOT(on_data_received()));

 
                gen_interrupt_ = false;
                gen_thread_ = std::thread(&MathSignal::generation_proc, this);
        }
 }
 
 
                gen_interrupt_ = false;
                gen_thread_ = std::thread(&MathSignal::generation_proc, this);
        }
 }
 

-void MathSignal::generate_samples(uint32_t segment_id, const uint64_t start_sample,
+uint64_t MathSignal::generate_samples(uint32_t segment_id, const uint64_t start_sample,

        const int64_t sample_count)
 {
        const int64_t sample_count)
 {

+       uint64_t count = 0;
+

        shared_ptr<Analog> analog = dynamic_pointer_cast<Analog>(data_);
        shared_ptr<AnalogSegment> segment = analog->analog_segments().at(segment_id);
 
        // Keep the math functions segment IDs in sync
        shared_ptr<Analog> analog = dynamic_pointer_cast<Analog>(data_);
        shared_ptr<AnalogSegment> segment = analog->analog_segments().at(segment_id);
 
        // Keep the math functions segment IDs in sync

-       fnc_sig_sample_->current_segment = segment_id;
+       fnc_sample_->current_segment = segment_id;

 
        const double sample_rate = data_->get_samplerate();
 
 
        const double sample_rate = data_->get_samplerate();
 


@@ -366,11 +411,20 @@ void MathSignal::generate_samples(uint32_t segment_id, const uint64_t start_samp
                double value = exprtk_expression_->value();
                sample_data[i] = value;
                exprtk_current_sample_ += 1;
                double value = exprtk_expression_->value();
                sample_data[i] = value;
                exprtk_current_sample_ += 1;

+               count++;
+
+               // If during the evaluation of the expression it was found that this
+               // math signal itself is being accessed, the chunk size was reduced
+               // to 1, which means we must stop after this sample we just generated
+               if (generation_chunk_size_ == 1)
+                       break;

        }
 
        }
 

-       segment->append_interleaved_samples(sample_data, sample_count, 1);
+       segment->append_interleaved_samples(sample_data, count, 1);

 
        delete[] sample_data;
 
        delete[] sample_data;

+
+       return count;

 }
 
 void MathSignal::generation_proc()
 }
 
 void MathSignal::generation_proc()


@@ -410,15 +464,13 @@ void MathSignal::generation_proc()
 
                // Process the samples if necessary...
                if (samples_to_process > 0) {
 
                // Process the samples if necessary...
                if (samples_to_process > 0) {

-                       const uint64_t chunk_sample_count = ChunkLength;
-

                        uint64_t processed_samples = 0;
                        do {
                                const uint64_t start_sample = output_sample_count + processed_samples;
                        uint64_t processed_samples = 0;
                        do {
                                const uint64_t start_sample = output_sample_count + processed_samples;

-                               const uint64_t sample_count =
-                                       min(samples_to_process - processed_samples,     chunk_sample_count);
+                               uint64_t sample_count =
+                                       min(samples_to_process - processed_samples,     generation_chunk_size_);

 
 

-                               generate_samples(segment_id, start_sample, sample_count);
+                               sample_count = generate_samples(segment_id, start_sample, sample_count);

                                processed_samples += sample_count;
 
                                // Notify consumers of this signal's data
                                processed_samples += sample_count;
 
                                // Notify consumers of this signal's data


@@ -447,6 +499,12 @@ void MathSignal::generation_proc()
 
 signal_data* MathSignal::signal_from_name(const std::string& name)
 {
 
 signal_data* MathSignal::signal_from_name(const std::string& name)
 {

+       // If the expression contains the math signal itself, we must add every sample to
+       // the output segment immediately so that it can be accessed
+       const QString sig_name = QString::fromStdString(name);
+       if (sig_name == this->name())
+               generation_chunk_size_ = 1;
+

        // Look up signal in the map and if it doesn't exist yet, add it for future use
 
        auto element = input_signals_.find(name);
        // Look up signal in the map and if it doesn't exist yet, add it for future use
 
        auto element = input_signals_.find(name);


@@ -455,7 +513,6 @@ signal_data* MathSignal::signal_from_name(const std::string& name)
                return &(element->second);
        } else {
                const vector< shared_ptr<SignalBase> > signalbases = session_.signalbases();
                return &(element->second);
        } else {
                const vector< shared_ptr<SignalBase> > signalbases = session_.signalbases();

-               const QString sig_name = QString::fromStdString(name);

 
                for (const shared_ptr<SignalBase>& sb : signalbases)
                        if (sb->name() == sig_name) {
 
                for (const shared_ptr<SignalBase>& sb : signalbases)
                        if (sb->name() == sig_name) {


@@ -467,10 +524,18 @@ signal_data* MathSignal::signal_from_name(const std::string& name)
                                connect(sb->analog_data().get(), SIGNAL(segment_completed()),
                                        this, SLOT(on_data_received()));
 
                                connect(sb->analog_data().get(), SIGNAL(segment_completed()),
                                        this, SLOT(on_data_received()));
 

+                               connect(sb.get(), SIGNAL(enabled_changed(bool)),
+                                       this, SLOT(on_enabled_changed()));
+

                                return &(input_signals_.insert({name, signal_data(sb)}).first->second);
                        }
        }
 
                                return &(input_signals_.insert({name, signal_data(sb)}).first->second);
                        }
        }
 

+       // If we reach this point, no valid signal was found with the supplied name
+       if (error_type_ == MATH_ERR_NONE)
+               set_error(MATH_ERR_INVALID_SIGNAL, QString(tr("\"%1\" isn't a valid analog signal")) \
+                       .arg(QString::fromStdString(name)));
+

        return nullptr;
 }
 
        return nullptr;
 }
 


@@ -491,7 +556,11 @@ void MathSignal::update_signal_sample(signal_data* sig_data, uint32_t segment_id
        const shared_ptr<AnalogSegment> segment = analog->analog_segments().at(segment_id);
 
        sig_data->sample_num = sample_num;
        const shared_ptr<AnalogSegment> segment = analog->analog_segments().at(segment_id);
 
        sig_data->sample_num = sample_num;

-       sig_data->sample_value = segment->get_sample(sample_num);
+
+       if (sample_num < segment->get_sample_count())
+               sig_data->sample_value = segment->get_sample(sample_num);
+       else
+               sig_data->sample_value = 0;

 
        // We only have a reference if this signal is used as a scalar;
        // if it's used by a function, it's null
 
        // We only have a reference if this signal is used as a scalar;
        // if it's used by a function, it's null


@@ -499,6 +568,25 @@ void MathSignal::update_signal_sample(signal_data* sig_data, uint32_t segment_id
                *(sig_data->ref) = sig_data->sample_value;
 }
 
                *(sig_data->ref) = sig_data->sample_value;
 }
 

+bool MathSignal::all_input_signals_enabled(QString &disabled_signals) const
+{
+       bool all_enabled = true;
+
+       disabled_signals.clear();
+
+       for (auto input_signal : input_signals_) {
+               const shared_ptr<SignalBase>& sb = input_signal.second.sb;
+
+               if (!sb->enabled()) {
+                       all_enabled = false;
+                       disabled_signals += disabled_signals.isEmpty() ?
+                               sb->name() : ", " + sb->name();
+               }
+       }
+
+       return all_enabled;
+}
+

 void MathSignal::on_capture_state_changed(int state)
 {
        if (state == Session::Running)
 void MathSignal::on_capture_state_changed(int state)
 {
        if (state == Session::Running)


@@ -514,5 +602,18 @@ void MathSignal::on_data_received()
        gen_input_cond_.notify_one();
 }
 
        gen_input_cond_.notify_one();
 }
 

+void MathSignal::on_enabled_changed()
+{
+       QString disabled_signals;
+       if (!all_input_signals_enabled(disabled_signals) &&
+               ((error_type_ == MATH_ERR_NONE) || (error_type_ == MATH_ERR_ENABLE)))
+               set_error(MATH_ERR_ENABLE,
+                       tr("No data will be generated as %1 must be enabled").arg(disabled_signals));
+       else if (disabled_signals.isEmpty() && (error_type_ == MATH_ERR_ENABLE)) {
+               error_type_ = MATH_ERR_NONE;
+               error_message_.clear();
+       }
+}
+

 } // namespace data
 } // namespace pv
 } // namespace data
 } // namespace pv