Make error handling generic improve math error detail

diff --git a/pv/data/decodesignal.cpp b/pv/data/decodesignal.cpp
index 256c29b8c9f34d65d090ab8a77ca1e4d563d32f3..b665ba8934fafde460c762922c354890836c7716 100644 (file)
--- a/pv/data/decodesignal.cpp
+++ b/pv/data/decodesignal.cpp
@@ -56,8 +56,7 @@ DecodeSignal::DecodeSignal(pv::Session &session) :
        srd_session_(nullptr),
        logic_mux_data_invalid_(false),
        stack_config_changed_(true),
-       current_segment_id_(0),
-       error_message_("")
+       current_segment_id_(0)
 {
        connect(&session_, SIGNAL(capture_state_changed(int)),
                this, SLOT(on_capture_state_changed(int)));
@@ -266,12 +265,6 @@ bool DecodeSignal::is_paused() const
        return decode_paused_;
 }
 
-QString DecodeSignal::error_message() const
-{
-       lock_guard<mutex> lock(output_mutex_);
-       return error_message_;
-}
-
 const vector<decode::DecodeChannel> DecodeSignal::get_channels() const
 {
        return channels_;
@@ -829,13 +822,6 @@ void DecodeSignal::restore_settings(QSettings &settings)
        begin_decode();
 }
 
-void DecodeSignal::set_error_message(QString msg)
-{
-       error_message_ = msg;
-       // TODO Emulate noquote()
-       qDebug().nospace() << name() << ": " << msg;
-}
-
 bool DecodeSignal::all_input_segments_complete(uint32_t segment_id) const
 {
        bool all_complete = true;

diff --git a/pv/data/decodesignal.hpp b/pv/data/decodesignal.hpp
index bfbf5ee0644868856c9fb9cd9c86ec410e2cf42e..c9ebb2a69c4d00adea4e099e0c4df8de5c6a7d87 100644 (file)
--- a/pv/data/decodesignal.hpp
+++ b/pv/data/decodesignal.hpp
@@ -27,7 +27,6 @@
 #include <vector>
 
 #include <QSettings>
-#include <QString>
 
 #include <libsigrokdecode/libsigrokdecode.h>
 
@@ -115,7 +114,6 @@ public:
        void pause_decode();
        void resume_decode();
        bool is_paused() const;
-       QString error_message() const;
 
        const vector<decode::DecodeChannel> get_channels() const;
        void auto_assign_signals(const shared_ptr<Decoder> dec);
@@ -189,8 +187,6 @@ public:
        virtual void restore_settings(QSettings &settings);
 
 private:
-       void set_error_message(QString msg);
-
        bool all_input_segments_complete(uint32_t segment_id) const;
        uint32_t get_input_segment_count() const;
        double get_input_samplerate(uint32_t segment_id) const;
@@ -261,8 +257,6 @@ private:
        atomic<bool> decode_interrupt_, logic_mux_interrupt_;
 
        bool decode_paused_;
-
-       QString error_message_;
 };
 
 } // namespace data

diff --git a/pv/data/mathsignal.cpp b/pv/data/mathsignal.cpp
index 2ad90212fe01eb5c8cd5cef764aaed6f1b3150b5..e560a2e7b1a3c9a99b7aaa94ed65b4d04165252f 100644 (file)
--- a/pv/data/mathsignal.cpp
+++ b/pv/data/mathsignal.cpp
@@ -84,7 +84,6 @@ MathSignal::MathSignal(pv::Session &session) :
        use_custom_sample_rate_(false),
        use_custom_sample_count_(false),
        expression_(""),
-       error_message_(""),
        exprtk_unknown_symbol_table_(nullptr),
        exprtk_symbol_table_(nullptr),
        exprtk_expression_(nullptr),
@@ -137,11 +136,6 @@ void MathSignal::restore_settings(QSettings &settings)
                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_;
@@ -159,6 +153,8 @@ void MathSignal::set_error_message(QString msg)
        error_message_ = msg;
        // TODO Emulate noquote()
        qDebug().nospace() << name() << ": " << msg << "(Expression: '" << expression_ << "')";
+
+       error_message_changed(msg);
 }
 
 uint64_t MathSignal::get_working_sample_count(uint32_t segment_id) const
@@ -313,7 +309,25 @@ void MathSignal::begin_generation()
        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_message(error_details);
        } else {
                // Resolve unknown scalars to signals and add them to the input signal list
                vector<string> unknowns;

diff --git a/pv/data/mathsignal.hpp b/pv/data/mathsignal.hpp
index 6127b177e025175f1a7cf5621bb48a0ed089a2c1..67cc506849db476e96c4bfdf4b99ed8e073b0031 100644 (file)
--- a/pv/data/mathsignal.hpp
+++ b/pv/data/mathsignal.hpp
@@ -60,7 +60,6 @@ class MathSignal : public SignalBase
 {
        Q_OBJECT
        Q_PROPERTY(QString expression READ get_expression WRITE set_expression NOTIFY expression_changed)
-       Q_PROPERTY(QString error_message READ error_message)
 
 private:
        static const int64_t ChunkLength;
@@ -72,13 +71,11 @@ public:
        virtual void save_settings(QSettings &settings) const;
        virtual void restore_settings(QSettings &settings);
 
-       QString error_message() const;
-
        QString get_expression() const;
        void set_expression(QString expression);
 
 private:
-       void set_error_message(QString msg);
+       virtual void set_error_message(QString msg);
 
        /**
         * Returns the number of samples that can be worked on,
@@ -123,7 +120,7 @@ private:
        bool use_custom_sample_rate_, use_custom_sample_count_;
        map<std::string, signal_data> input_signals_;
 
-       QString expression_, error_message_;
+       QString expression_;
 
        mutable mutex input_mutex_;
        mutable condition_variable gen_input_cond_;

diff --git a/pv/data/signalbase.cpp b/pv/data/signalbase.cpp
index 104243b11db8a168ef24d1e9ed021f4c949958d7..94e1f6a6c03a8f86318bfaf35d9c14fb572bb290 100644 (file)
--- a/pv/data/signalbase.cpp
+++ b/pv/data/signalbase.cpp
@@ -123,7 +123,8 @@ SignalBase::SignalBase(shared_ptr<sigrok::Channel> channel, ChannelType channel_
        group_(nullptr),
        conversion_type_(NoConversion),
        min_value_(0),
-       max_value_(0)
+       max_value_(0),
+       error_message_("")
 {
        if (channel_) {
                internal_name_ = QString::fromStdString(channel_->name());
@@ -258,6 +259,11 @@ QColor SignalBase::bgcolor() const
        return bgcolor_;
 }
 
+QString SignalBase::get_error_message() const
+{
+       return error_message_;
+}
+
 void SignalBase::set_data(shared_ptr<pv::data::SignalData> data)
 {
        if (data_) {
@@ -824,6 +830,15 @@ void SignalBase::start_conversion(bool delayed_start)
        conversion_thread_ = std::thread(&SignalBase::conversion_thread_proc, this);
 }
 
+void SignalBase::set_error_message(QString msg)
+{
+       error_message_ = msg;
+       // TODO Emulate noquote()
+       qDebug().nospace() << name() << ": " << msg;
+
+       error_message_changed(msg);
+}
+
 void SignalBase::stop_conversion()
 {
        // Stop conversion so we can restart it from the beginning

diff --git a/pv/data/signalbase.hpp b/pv/data/signalbase.hpp
index 89bfc25298369bf1431e3fa05f325bba1abd0e74..a55b3fc2d3fa458c7d97f522d2e503674d421ecc 100644 (file)
--- a/pv/data/signalbase.hpp
+++ b/pv/data/signalbase.hpp
@@ -87,6 +87,7 @@ private:
 class SignalBase : public QObject, public enable_shared_from_this<SignalBase>
 {
        Q_OBJECT
+       Q_PROPERTY(QString error_message READ get_error_message)
 
 public:
        enum ChannelType {
@@ -225,6 +226,11 @@ public:
         */
        QColor bgcolor() const;
 
+       /**
+        * Returns the current error message text.
+        */
+       virtual QString get_error_message() const;
+
        /**
         * Sets the internal data object.
         */
@@ -346,7 +352,12 @@ public:
 
        void start_conversion(bool delayed_start=false);
 
+protected:
+       virtual void set_error_message(QString msg);
+
 private:
+       void stop_conversion();
+
        bool conversion_is_a2l() const;
 
        uint8_t convert_a2l_threshold(float threshold, float value);
@@ -359,19 +370,14 @@ private:
                shared_ptr<LogicSegment> lsegment);
        void conversion_thread_proc();
 
-       void stop_conversion();
-
 Q_SIGNALS:
        void enabled_changed(const bool &value);
-
        void name_changed(const QString &name);
-
        void color_changed(const QColor &color);
-
+       void error_message_changed(const QString &msg);
        void conversion_type_changed(const ConversionType t);
 
        void samples_cleared();
-
        void samples_added(uint64_t segment_id, uint64_t start_sample,
                uint64_t end_sample);
 
@@ -409,6 +415,8 @@ protected:
        QString internal_name_, name_;
        QColor color_, bgcolor_;
        unsigned int index_;
+
+       QString error_message_;
 };
 
 } // namespace data

diff --git a/pv/views/trace/analogsignal.cpp b/pv/views/trace/analogsignal.cpp
index ea33823d26fcd63031ac3fffb567381fd1f793c4..e6b6d4966b710b0b6043926439083908f6233152 100644 (file)
--- a/pv/views/trace/analogsignal.cpp
+++ b/pv/views/trace/analogsignal.cpp
@@ -257,32 +257,36 @@ void AnalogSignal::paint_mid(QPainter &p, ViewItemPaintParams &pp)
                paint_grid(p, y, pp.left(), pp.right());
 
                shared_ptr<pv::data::AnalogSegment> segment = get_analog_segment_to_paint();
-               if (!segment || (segment->get_sample_count() == 0))
-                       return;
-
-               const double pixels_offset = pp.pixels_offset();
-               const double samplerate = max(1.0, segment->samplerate());
-               const pv::util::Timestamp& start_time = segment->start_time();
-               const int64_t last_sample = (int64_t)segment->get_sample_count() - 1;
-               const double samples_per_pixel = samplerate * pp.scale();
-               const pv::util::Timestamp start = samplerate * (pp.offset() - start_time);
-               const pv::util::Timestamp end = start + samples_per_pixel * pp.width();
-
-               const int64_t start_sample = min(max(floor(start).convert_to<int64_t>(),
-                       (int64_t)0), last_sample);
-               const int64_t end_sample = min(max((ceil(end) + 1).convert_to<int64_t>(),
-                       (int64_t)0), last_sample);
-
-               if (samples_per_pixel < EnvelopeThreshold)
-                       paint_trace(p, segment, y, pp.left(), start_sample, end_sample,
-                               pixels_offset, samples_per_pixel);
-               else
-                       paint_envelope(p, segment, y, pp.left(), start_sample, end_sample,
-                               pixels_offset, samples_per_pixel);
+
+               if (segment && (segment->get_sample_count() > 0)) {
+                       const double pixels_offset = pp.pixels_offset();
+                       const double samplerate = max(1.0, segment->samplerate());
+                       const pv::util::Timestamp& start_time = segment->start_time();
+                       const int64_t last_sample = (int64_t)segment->get_sample_count() - 1;
+                       const double samples_per_pixel = samplerate * pp.scale();
+                       const pv::util::Timestamp start = samplerate * (pp.offset() - start_time);
+                       const pv::util::Timestamp end = start + samples_per_pixel * pp.width();
+
+                       const int64_t start_sample = min(max(floor(start).convert_to<int64_t>(),
+                               (int64_t)0), last_sample);
+                       const int64_t end_sample = min(max((ceil(end) + 1).convert_to<int64_t>(),
+                               (int64_t)0), last_sample);
+
+                       if (samples_per_pixel < EnvelopeThreshold)
+                               paint_trace(p, segment, y, pp.left(), start_sample, end_sample,
+                                       pixels_offset, samples_per_pixel);
+                       else
+                               paint_envelope(p, segment, y, pp.left(), start_sample, end_sample,
+                                       pixels_offset, samples_per_pixel);
+               }
        }
 
        if ((display_type_ == DisplayConverted) || (display_type_ == DisplayBoth))
                paint_logic_mid(p, pp);
+
+       const QString err = base_->get_error_message();
+       if (!err.isEmpty())
+               paint_error(p, pp);
 }
 
 void AnalogSignal::paint_fore(QPainter &p, ViewItemPaintParams &pp)

diff --git a/pv/views/trace/decodetrace.cpp b/pv/views/trace/decodetrace.cpp
index 93c7c5a93cdd3bae6af7db425b612e12618ab200..2902c936552757ee1c88160a32d0f2b8d24ca82a 100644 (file)
--- a/pv/views/trace/decodetrace.cpp
+++ b/pv/views/trace/decodetrace.cpp
@@ -82,7 +82,6 @@ namespace pv {
 namespace views {
 namespace trace {
 
-const QColor DecodeTrace::ErrorBgColor = QColor(0xEF, 0x29, 0x29);
 const QColor DecodeTrace::NoDecodeColor = QColor(0x88, 0x8A, 0x85);
 const QColor DecodeTrace::ExpandMarkerWarnColor = QColor(0xFF, 0xA5, 0x00); // QColorConstants::Svg::orange
 const QColor DecodeTrace::ExpandMarkerHiddenColor = QColor(0x69, 0x69, 0x69); // QColorConstants::Svg::dimgray
@@ -337,9 +336,9 @@ void DecodeTrace::paint_mid(QPainter &p, ViewItemPaintParams &pp)
                owner_->row_item_appearance_changed(false, true);
        }
 
-       const QString err = decode_signal_->error_message();
+       const QString err = base_->get_error_message();
        if (!err.isEmpty())
-               draw_error(p, err, pp);
+               paint_error(p, pp);
 
 #if DECODETRACE_SHOW_RENDER_TIME
        qDebug() << "Rendering" << base_->name() << "took" << render_time_.elapsed() << "ms";
@@ -908,28 +907,6 @@ void DecodeTrace::draw_range(const Annotation* a, QPainter &p,
                best_annotation, Qt::ElideRight, rect.width()));
 }
 
-void DecodeTrace::draw_error(QPainter &p, const QString &message,
-       const ViewItemPaintParams &pp)
-{
-       const int y = get_visual_y();
-
-       double samples_per_pixel, pixels_offset;
-       tie(pixels_offset, samples_per_pixel) = get_pixels_offset_samples_per_pixel();
-
-       p.setPen(ErrorBgColor.darker());
-       p.setBrush(ErrorBgColor);
-
-       const QRectF bounding_rect = QRectF(pp.left(), INT_MIN / 2 + y, pp.right(), INT_MAX);
-
-       const QRectF text_rect = p.boundingRect(bounding_rect, Qt::AlignCenter, message);
-       const qreal r = text_rect.height() / 4;
-
-       p.drawRoundedRect(text_rect.adjusted(-r, -r, r, r), r, r, Qt::AbsoluteSize);
-
-       p.setPen(Qt::black);
-       p.drawText(text_rect, message);
-}
-
 void DecodeTrace::draw_unresolved_period(QPainter &p, int left, int right) const
 {
        double samples_per_pixel, pixels_offset;

diff --git a/pv/views/trace/decodetrace.hpp b/pv/views/trace/decodetrace.hpp
index 1b79161dbdb8143328651baf3a0440283fe3e830..80c9cf750b341f155fe1a6b077fe60dca6339b5a 100644 (file)
--- a/pv/views/trace/decodetrace.hpp
+++ b/pv/views/trace/decodetrace.hpp
@@ -120,7 +120,6 @@ class DecodeTrace : public Trace
        Q_OBJECT
 
 private:
-       static const QColor ErrorBgColor;
        static const QColor NoDecodeColor;
        static const QColor ExpandMarkerWarnColor;
        static const QColor ExpandMarkerHiddenColor;

diff --git a/pv/views/trace/trace.cpp b/pv/views/trace/trace.cpp
index ecc1aecc901a8adb9599c34e34f69ee896399727..1f77598d609c04235dfb3d0e3282b693edf6ee32 100644 (file)
--- a/pv/views/trace/trace.cpp
+++ b/pv/views/trace/trace.cpp
@@ -49,6 +49,7 @@ const int Trace::LabelHitPadding = 2;
 
 const QColor Trace::BrightGrayBGColor = QColor(0, 0, 0, 10 * 255 / 100);
 const QColor Trace::DarkGrayBGColor = QColor(0, 0, 0, 15 * 255 / 100);
+const QColor Trace::ErrorBgColor = QColor(0xEF, 0x29, 0x29);
 
 Trace::Trace(shared_ptr<data::SignalBase> signal) :
        base_(signal),
@@ -62,6 +63,8 @@ Trace::Trace(shared_ptr<data::SignalBase> signal) :
                this, SLOT(on_name_changed(const QString&)));
        connect(signal.get(), SIGNAL(color_changed(const QColor&)),
                this, SLOT(on_color_changed(const QColor&)));
+       connect(signal.get(), SIGNAL(error_message_changed(const QString&)),
+               this, SLOT(on_error_message_changed(const QString&)));
 
        GlobalSettings::add_change_handler(this);
 
@@ -175,6 +178,26 @@ void Trace::paint_label(QPainter &p, const QRect &rect, bool hover)
                Qt::AlignCenter | Qt::AlignVCenter, base_->name());
 }
 
+void Trace::paint_error(QPainter &p, const ViewItemPaintParams &pp)
+{
+       const QString message = base_->get_error_message();
+
+       const int y = get_visual_y();
+
+       p.setPen(ErrorBgColor.darker());
+       p.setBrush(ErrorBgColor);
+
+       const QRectF bounding_rect = QRectF(pp.left(), INT_MIN / 2 + y, pp.right(), INT_MAX);
+
+       const QRectF text_rect = p.boundingRect(bounding_rect, Qt::AlignCenter, message);
+       const qreal r = text_rect.height() / 4;
+
+       p.drawRoundedRect(text_rect.adjusted(-r, -r, r, r), r, r, Qt::AbsoluteSize);
+
+       p.setPen(Qt::black);
+       p.drawText(text_rect, message);
+}
+
 QMenu* Trace::create_header_context_menu(QWidget *parent)
 {
        QMenu *const menu = ViewItem::create_header_context_menu(parent);
@@ -402,6 +425,14 @@ void Trace::on_color_changed(const QColor &color)
                owner_->row_item_appearance_changed(true, true);
 }
 
+void Trace::on_error_message_changed(const QString &msg)
+{
+       (void)msg;
+
+       if (owner_)
+               owner_->row_item_appearance_changed(false, true);
+}
+
 void Trace::on_popup_closed()
 {
        popup_ = nullptr;

diff --git a/pv/views/trace/trace.hpp b/pv/views/trace/trace.hpp
index 05be7053dac7bff45888609f5e826a6744d08cf7..741f346957735c92aa1f62f1dc37e5cc3c85aad4 100644 (file)
--- a/pv/views/trace/trace.hpp
+++ b/pv/views/trace/trace.hpp
@@ -91,6 +91,7 @@ private:
 
        static const QColor BrightGrayBGColor;
        static const QColor DarkGrayBGColor;
+       static const QColor ErrorBgColor;
 
 protected:
        Trace(shared_ptr<data::SignalBase> signal);
@@ -127,6 +128,13 @@ public:
         */
        virtual void paint_label(QPainter &p, const QRect &rect, bool hover);
 
+       /**
+        * Paints the signal's current error message text.
+        * @param p the QPainter to paint into.
+        * @param pp The painting parameters object to paint with.
+        */
+       virtual void paint_error(QPainter &p, const ViewItemPaintParams &pp);
+
        virtual QMenu* create_header_context_menu(QWidget *parent);
 
        virtual QMenu* create_view_context_menu(QWidget *parent, QPoint &click_pos);
@@ -142,7 +150,7 @@ public:
 
        /**
         * Computes the outline rectangle of the viewport hit-box.
-        * @param rect the rectangle of the viewport area.
+        * @param pp The painting parameters object to paint with.
         * @return Returns the rectangle of the hit-box.
         * @remarks The default implementation returns an empty hit-box.
         */
@@ -184,8 +192,8 @@ protected:
 
 protected Q_SLOTS:
        virtual void on_name_changed(const QString &text);
-
        virtual void on_color_changed(const QColor &color);
+       virtual void on_error_message_changed(const QString &msg);
 
        void on_popup_closed();