From: Soeren Apel Date: Sat, 22 Aug 2020 20:49:16 +0000 (+0200) Subject: Make error handling generic improve math error detail X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=f6a93932056dab5e2f75207b65197b436d4141a5;p=pulseview.git Make error handling generic improve math error detail --- diff --git a/pv/data/decodesignal.cpp b/pv/data/decodesignal.cpp index 256c29b8..b665ba89 100644 --- 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 lock(output_mutex_); - return error_message_; -} - const vector 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 bfbf5ee0..c9ebb2a6 100644 --- a/pv/data/decodesignal.hpp +++ b/pv/data/decodesignal.hpp @@ -27,7 +27,6 @@ #include #include -#include #include @@ -115,7 +114,6 @@ public: void pause_decode(); void resume_decode(); bool is_paused() const; - QString error_message() const; const vector get_channels() const; void auto_assign_signals(const shared_ptr 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 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 2ad90212..e560a2e7 100644 --- 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 unknowns; diff --git a/pv/data/mathsignal.hpp b/pv/data/mathsignal.hpp index 6127b177..67cc5068 100644 --- 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 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 104243b1..94e1f6a6 100644 --- a/pv/data/signalbase.cpp +++ b/pv/data/signalbase.cpp @@ -123,7 +123,8 @@ SignalBase::SignalBase(shared_ptr 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 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 89bfc252..a55b3fc2 100644 --- a/pv/data/signalbase.hpp +++ b/pv/data/signalbase.hpp @@ -87,6 +87,7 @@ private: class SignalBase : public QObject, public enable_shared_from_this { 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 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 ea33823d..e6b6d496 100644 --- 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 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)0), last_sample); - const int64_t end_sample = min(max((ceil(end) + 1).convert_to(), - (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)0), last_sample); + const int64_t end_sample = min(max((ceil(end) + 1).convert_to(), + (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 93c7c5a9..2902c936 100644 --- 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 1b79161d..80c9cf75 100644 --- 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 ecc1aecc..1f77598d 100644 --- 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 signal) : base_(signal), @@ -62,6 +63,8 @@ Trace::Trace(shared_ptr 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 05be7053..741f3469 100644 --- 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 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();