From 17454ebba266d45f466f571067728e013dd0a880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20K=C3=A4berich?= Date: Mon, 11 Jul 2022 23:53:24 +0200 Subject: [PATCH] Math traces bugfixes + comments --- Software/PC_Application/Traces/trace.cpp | 72 ++++++++++++++----- Software/PC_Application/Traces/trace.h | 46 ++++++++++-- Software/PC_Application/Traces/tracemodel.cpp | 4 +- .../PC_Application/Traces/tracewaterfall.cpp | 3 + .../PC_Application/Traces/tracexyplot.cpp | 5 ++ 5 files changed, 104 insertions(+), 26 deletions(-) diff --git a/Software/PC_Application/Traces/trace.cpp b/Software/PC_Application/Traces/trace.cpp index 3515876..9e3c7f7 100644 --- a/Software/PC_Application/Traces/trace.cpp +++ b/Software/PC_Application/Traces/trace.cpp @@ -297,6 +297,8 @@ void Trace::fromMath() { source = Source::Math; clear(); + mathUpdateEnd = 0; + mathUpdateBegin = numeric_limits::max(); updateMathTracePoints(); scheduleMathCalculation(0, data.size()); emit typeChanged(this); @@ -386,10 +388,10 @@ bool Trace::resolveMathSourceHashes() return success; } -bool Trace::updateMathTracePoints() +void Trace::updateMathTracePoints() { if(!mathSourceTraces.size()) { - return false; + return; } double startX = std::numeric_limits::lowest(); double stopX = std::numeric_limits::max(); @@ -410,25 +412,20 @@ bool Trace::updateMathTracePoints() } } unsigned int samples = round((stopX - startX) / stepSize + 1); - bool fullUpdate = false; +// qDebug() << "Updated trace points, now"<>::quiet_NaN(); + } + } } if(samples > 0 && (startX != data.front().x || stopX != data.back().x)) { - fullUpdate = true; - } - if(fullUpdate) { - // steps changed, complete update required mathUpdateBegin = 0; mathUpdateEnd = samples; - for(unsigned int i=0;i>::quiet_NaN(); - } - return true; - } else { - return false; } } @@ -441,6 +438,7 @@ void Trace::mathSourceTraceDeleted(Trace *t) void Trace::scheduleMathCalculation(unsigned int begin, unsigned int end) { +// qDebug() << "Scheduling calculation from"<= data.size() || mathUpdateEnd >= data.size() + 1) { + qWarning() << "Not calculating math trace, out of limits. Requested from" << mathUpdateBegin << "to" << mathUpdateEnd <<" but data is of size" << data.size(); return; } if(mathFormula.isEmpty()) { @@ -573,8 +572,25 @@ bool Trace::addMathSource(Trace *t, QString variableName) connect(t, &Trace::dataChanged, this, [=](unsigned int begin, unsigned int end){ updateMathTracePoints(); auto startX = t->sample(begin).x; - auto stopX = t->sample(end).x; - scheduleMathCalculation(index(startX), index(stopX)); + auto stopX = t->sample(end-1).x; + +// qDebug() << "Source update from"< int { + if(data.size() == 0) { + return 0; + } + auto lower = lower_bound(data.begin(), data.end(), x, [](const Data &lhs, const double x) -> bool { + return lhs.x < x; + }); + if(lower == data.end()) { + // actually beyond the last sample, return the index of the last anyway to avoid access past data + return data.size() - 1; + } + return lower - data.begin(); + }; + + scheduleMathCalculation(calcIndex(startX), calcIndex(stopX)+1); }); return true; } @@ -1050,6 +1066,28 @@ bool Trace::isVisible() return visible; } +bool Trace::canBePaused() +{ + switch(source) { + case Source::Live: + return true; + case Source::File: + return false; + case Source::Calibration: + return false; + case Source::Math: + // depends on the math, if it depends on any live data, it may be paused + for(auto ms : mathSourceTraces) { + if(ms.first->canBePaused()) { + return true; + } + } + return false; + default: + return false; + } +} + void Trace::pause() { if(!paused) { diff --git a/Software/PC_Application/Traces/trace.h b/Software/PC_Application/Traces/trace.h index d4cb415..059987c 100644 --- a/Software/PC_Application/Traces/trace.h +++ b/Software/PC_Application/Traces/trace.h @@ -66,6 +66,7 @@ public: QString name() { return _name; } QColor color() { return _color; } bool isVisible(); + bool canBePaused(); void pause(); void resume(); bool isPaused(); @@ -163,6 +164,9 @@ public: void setMathFormula(const QString &newMathFormula); bool mathFormularValid() const; + // When loading setups, some traces may be used as a math source before they are loaded. + // If that happens, their hashes are added to a list. Call this function for every new trace + // after all traces from the setup file have been created. It will look for the missing traces bool resolveMathSourceHashes(); public slots: @@ -171,11 +175,23 @@ public slots: void addMarker(Marker *m); void removeMarker(Marker *m); - // functions for handling source == Source::Math + // Functions for handling source == Source::Math + + // Checks whether the trace data depends on trace t. + // If onlyDirectDependency is true, only the direct math sources are checked (i.e. the math sources of this trace). + // If onlyDirectDependency is false, math sources and all their sources are checked recursively bool mathDependsOn(Trace *t, bool onlyDirectDependency = false); + // Checks whether a trace can potentially be used as a math source. It can not be used if: + // - it is the trace itself + // - it is in a different domain than an already used math source + // - it depends on this trace bool canAddAsMathSource(Trace *t); + // Adds another trace as a math source, with a custom variable name. The same trace may be added multiple times with + // different names, older variable names will be replaced with the new one bool addMathSource(Trace *t, QString variableName); + // Removes a trace as a math source (if its variable name is still used in the mathFormula, this will break the calculation) void removeMathSource(Trace *t); + // Retrieves the variable name used for the specified trace. If the trace is not used as a math source, an emptry string is returned QString getSourceVariableName(Trace *t); signals: @@ -194,13 +210,25 @@ signals: private slots: void markerVisibilityChanged(Marker *m); - // functions for handling source == Source::Math - bool updateMathTracePoints(); - void mathSourceTraceDeleted(Trace *t); - void scheduleMathCalculation(unsigned int begin, unsigned int end); - void calculateMath(); - void clearMathSources(); + // Functions for handling source == Source::Math + // Updates the datapoints, based on the available span of all math sources. + // The least common span is used for this trace. This function only creates the datapoints + // and sets the X coordinate. The Y coordinates are set in calculateMath() + void updateMathTracePoints(); + // Keeps track of deleted traces and removes them from the math sources + void mathSourceTraceDeleted(Trace *t); + // Schedules an update of this trace, should be called whenever any source data changes. + // As it is likely that multiple source traces will change directly after each other, no + // calculation is performed directly. Instead the calculation is only scheduled and executed + // shortly after. This prevents calculating data that will be overwritten immediately afterwards + void scheduleMathCalculation(unsigned int begin, unsigned int end); + // Actually calculates the Y coordinate values of this trace. It expects that the data vector is + // already set up with the required amount of points and X coordinates + void calculateMath(); + // Removes all math sources, use this when switching to a different source mode + void clearMathSources(); + // Attempts to add a math source using the trace hash instead of a pointer (when loading setups) bool addMathSource(unsigned int hash, QString variableName); private: @@ -209,6 +237,10 @@ private: QColor _color; Source source; + // Hash for identifying the trace when loading/saving setup files. + // When writing setup files, the hash is created from the JSON representation of the trace. + // When loading setup files, the hash specified in the setup file is used (prevents different + // hashes when the JSON format changes in newer versions) unsigned int hash; bool hashSet; bool JSONskipHash; diff --git a/Software/PC_Application/Traces/tracemodel.cpp b/Software/PC_Application/Traces/tracemodel.cpp index e28a243..e1a31cb 100644 --- a/Software/PC_Application/Traces/tracemodel.cpp +++ b/Software/PC_Application/Traces/tracemodel.cpp @@ -63,7 +63,7 @@ void TraceModel::toggleVisibility(unsigned int index) void TraceModel::togglePause(unsigned int index) { - if (index < traces.size()) { + if (index < traces.size() && traces[index]->canBePaused()) { if(traces[index]->isPaused()) { traces[index]->resume(); } else { @@ -118,7 +118,7 @@ QVariant TraceModel::data(const QModelIndex &index, int role) const } break; case ColIndexPlayPause: - if (role == Qt::DecorationRole && trace->getSource() == Trace::Source::Live) { // TODO trace needs function to check if it may change due to live data + if (role == Qt::DecorationRole && trace->canBePaused()) { if (trace->isPaused()) { return QIcon(":/icons/pause.svg"); } else { diff --git a/Software/PC_Application/Traces/tracewaterfall.cpp b/Software/PC_Application/Traces/tracewaterfall.cpp index 64b59e1..7e9e12d 100644 --- a/Software/PC_Application/Traces/tracewaterfall.cpp +++ b/Software/PC_Application/Traces/tracewaterfall.cpp @@ -530,6 +530,9 @@ void TraceWaterfall::traceDataChanged(unsigned int begin, unsigned int end) data.back()[i] = trace->sample(i); if(yAxis.getAutorange() && !YAxisUpdateRequired) { double val = yAxis.sampleToCoordinate(trace->sample(i)); + if(isnan(val) || isinf(val)) { + continue; + } if(val < min) { min = val; } diff --git a/Software/PC_Application/Traces/tracexyplot.cpp b/Software/PC_Application/Traces/tracexyplot.cpp index 74433e3..b73882c 100644 --- a/Software/PC_Application/Traces/tracexyplot.cpp +++ b/Software/PC_Application/Traces/tracexyplot.cpp @@ -781,6 +781,11 @@ void TraceXYPlot::updateAxisTicks() continue; } + if(isnan(point.y()) || isinf(point.y())) { + // skip NaN and Infty values + continue; + } + if(point.y() > max) { max = point.y(); }