Skip to content

Commit

Permalink
iio-widgets/RangeGuiStrategy: Add safety checks for wrong data inputs
Browse files Browse the repository at this point in the history
Signed-off-by: Andrei-Fabian-Pop <[email protected]>
  • Loading branch information
Andrei-Fabian-Pop committed Apr 19, 2024
1 parent e0a878f commit 6671e60
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 28 deletions.
36 changes: 33 additions & 3 deletions gui/include/gui/widgets/titlespinbox.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

#include <QLabel>
#include <QPushButton>
#include <QSpinBox>
#include <QLineEdit>
#include <QWidget>
#include "scopy-gui_export.h"

Expand All @@ -19,13 +19,43 @@ class SCOPY_GUI_EXPORT TitleSpinBox : public QWidget
void setTitle(QString title);
QPushButton *getSpinBoxUpButton();
QPushButton *getSpinBoxDownButton();
QSpinBox *getSpinBox();
QLineEdit *getLineEdit();

double step() const;
void setStep(double newStep);

double max() const;
void setMax(double newMax);

double min() const;
void setMin(double newMin);

void setValue(double newValue);
void setSpinButtonsDisabled(bool isDisabled);

private:
/**
* @brief truncValue This function is needed because the QString::number function that
* would be normally used to parse a QString to a number does not work well with large
* numbers that end with multiple zeroes. The QString returned will be in scientific
* notation. The alternative is that we can set a number of fixed decimal points, but
* it would look awkward so this function converts it to a QString and chops the
* trailing zeroes.
* @param value
* @return
*/
static QString truncValue(double value);

QPushButton *m_spinBoxUpButton;
QPushButton *m_spinBoxDownButton;
QLabel *m_titleLabel;
QSpinBox *m_spinBox;

// This lineedit will act like a spinbox because the QSpinBox
// is more restrictive than we would like :/
QLineEdit *m_lineedit;
double m_min;
double m_max;
double m_step;
};
} // namespace scopy
#endif // TITLESPINBOX_H
101 changes: 90 additions & 11 deletions gui/src/widgets/titlespinbox.cpp
Original file line number Diff line number Diff line change
@@ -1,34 +1,40 @@
#include "titlespinbox.h"
#include "stylehelper.h"

#include <string>

#include <QBoxLayout>
#include <QLoggingCategory>
#include <utils.h>

using namespace scopy;
Q_LOGGING_CATEGORY(CAT_TITLESPINBOX, "TitleSpinBox")

TitleSpinBox::TitleSpinBox(QString title, QWidget *parent)
: QWidget(parent)
, m_titleLabel(new QLabel(title, this))
, m_spinBox(new QSpinBox(this))
, m_lineedit(new QLineEdit(this))
, m_spinBoxUpButton(new QPushButton(this))
, m_spinBoxDownButton(new QPushButton(this))
, m_min(0)
, m_step(1)
, m_max(99)
{
QHBoxLayout *mainLayout = new QHBoxLayout(this);
mainLayout->setContentsMargins(0, 0, 0, 0);
setLayout(mainLayout);
m_spinBox->setStyleSheet("border: 0px solid gray; font-weight: normal;");
m_lineedit->setStyleSheet("border: 0px solid gray; font-weight: normal;");

StyleHelper::MenuSmallLabel(m_titleLabel);
m_spinBox->setButtonSymbols(QSpinBox::NoButtons);
m_spinBox->setMaximumHeight(25);
m_lineedit->setMaximumHeight(25);

QWidget *spinboxWidget = new QWidget(this);
QVBoxLayout *spinboxWidgetLayout = new QVBoxLayout(spinboxWidget);
spinboxWidgetLayout->setSpacing(0);
spinboxWidgetLayout->setMargin(0);

spinboxWidgetLayout->addWidget(m_titleLabel);
spinboxWidgetLayout->addWidget(m_spinBox);
spinboxWidgetLayout->addWidget(m_lineedit);

QWidget *buttonWidget = new QWidget(this);
buttonWidget->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Preferred);
Expand All @@ -50,13 +56,48 @@ TitleSpinBox::TitleSpinBox(QString title, QWidget *parent)
buttonWidgetLayout->addWidget(m_spinBoxDownButton);

// here we preffer the pressed signal rather than the clicked one to speed up the change of values
connect(m_spinBoxUpButton, &QPushButton::pressed, m_spinBox,
[this] { m_spinBox->setValue(m_spinBox->value() + m_spinBox->singleStep()); });
connect(m_spinBoxDownButton, &QPushButton::pressed, m_spinBox,
[this] { m_spinBox->setValue(m_spinBox->value() - m_spinBox->singleStep()); });
connect(m_spinBoxUpButton, &QPushButton::pressed, m_lineedit, [this] {
bool ok = true;
QString text = m_lineedit->text();
double value = text.toDouble(&ok);
if(!ok) {
// If the cast fails that means that there is an issue with the text and the
// min/max/step values are useless here. The signal will just be skipped and
// a debug message will de displayed.
qDebug(CAT_TITLESPINBOX) << "Cannot increase the value:" << text;
return;
}

double newValue = value + m_step;
if(newValue > m_max) {
newValue = value;
}

m_lineedit->setText(truncValue(newValue));
});

connect(m_spinBoxDownButton, &QPushButton::pressed, m_lineedit, [this] {
bool ok = true;
QString text = m_lineedit->text();
double value = text.toDouble(&ok);
if(!ok) {
// If the cast fails that means that there is an issue with the text and the
// min/max/step values are useless here. The signal will just be skipped and
// a debug message will de displayed.
qDebug(CAT_TITLESPINBOX) << "Cannot decrease the value:" << text;
return;
}

double newValue = value - m_step;
if(newValue < m_min) {
newValue = value;
}

m_lineedit->setText(truncValue(newValue));
});

spinboxWidgetLayout->addWidget(m_titleLabel);
spinboxWidgetLayout->addWidget(m_spinBox);
spinboxWidgetLayout->addWidget(m_lineedit);

mainLayout->addWidget(spinboxWidget);
mainLayout->addItem(new QSpacerItem(0, 0, QSizePolicy::Expanding, QSizePolicy::Preferred));
Expand All @@ -71,6 +112,44 @@ QPushButton *TitleSpinBox::getSpinBoxUpButton() { return m_spinBoxUpButton; }

QPushButton *TitleSpinBox::getSpinBoxDownButton() { return m_spinBoxDownButton; }

QSpinBox *TitleSpinBox::getSpinBox() { return m_spinBox; }
QLineEdit *TitleSpinBox::getLineEdit() { return m_lineedit; }

double TitleSpinBox::step() const { return m_step; }

void TitleSpinBox::setStep(double newStep) { m_step = newStep; }

double TitleSpinBox::max() const { return m_max; }

void TitleSpinBox::setMax(double newMax) { m_max = newMax; }

double TitleSpinBox::min() const { return m_min; }

void TitleSpinBox::setMin(double newMin) { m_min = newMin; }

void TitleSpinBox::setValue(double newValue) { m_lineedit->setText(truncValue(newValue)); }

void TitleSpinBox::setSpinButtonsDisabled(bool isDisabled)
{
m_spinBoxUpButton->setVisible(!isDisabled);
m_spinBoxDownButton->setVisible(!isDisabled);
}

QString TitleSpinBox::truncValue(double value)
{
QString sReturn = QString::number(value, 'f', 7); // magic number
int i = sReturn.size() - 1;
int toChop = 0;
while(sReturn[i] == '0') {
++toChop;
--i;
}

if(sReturn[i] == '.') {
++toChop;
}

sReturn.chop(toChop);
return sReturn;
}

#include "moc_titlespinbox.cpp"
10 changes: 10 additions & 0 deletions iio-widgets/include/iio-widgets/guistrategy/rangeguistrategy.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ public Q_SLOTS:
void requestData() override;

private:
/**
* @brief tryParse will try an parse a QString to a double and if it fails it will try
* and parse it io an int and the cast it back to a double.
* @param number A QString that represents a double or an int.
* @param success This will be set to false if the QString parse fails and true if the
* number is parsed successfully.
* @return The double that was extracted from the QString.
*/
double tryParse(QString number, bool *success);

QWidget *m_ui;
TitleSpinBox *m_spinBox;
};
Expand Down
82 changes: 70 additions & 12 deletions iio-widgets/src/guistrategy/rangeguistrategy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ RangeAttrUi::RangeAttrUi(IIOWidgetFactoryRecipe recipe, QWidget *parent)
m_ui->layout()->addWidget(m_spinBox);
Q_EMIT requestData();

connect(m_spinBox->getSpinBox(), QOverload<int>::of(&QSpinBox::valueChanged), this,
[this](int value) { Q_EMIT emitData(QString::number(value)); });
connect(m_spinBox->getLineEdit(), &QLineEdit::textChanged, this,
[this](QString text) { Q_EMIT emitData(text); });
}

RangeAttrUi::~RangeAttrUi() { m_ui->deleteLater(); }
Expand All @@ -61,28 +61,86 @@ bool RangeAttrUi::isValid()

void RangeAttrUi::receiveData(QString currentData, QString optionalData)
{
QSignalBlocker blocker(m_spinBox->getSpinBox());
QSignalBlocker blocker(m_spinBox->getLineEdit());
QString availableAttributeValue = QString(optionalData).mid(1, QString(optionalData).size() - 2);
QStringList optionsList = availableAttributeValue.split(" ", Qt::SkipEmptyParts);
int optionListSize = optionsList.size();

if(optionListSize != 3) {
// Ideally the build factory system will catch this and init this as a lineedit, but we will
// treatthis just in case. The following rules will apply:
// - The optionalData will be ignored
// - The step will be set to 1
// - The min and the max will not be set
qCritical(CAT_ATTR_GUI_STRATEGY)
<< "The Range ui strategy cannot be initialized with the optional data ("
<< availableAttributeValue << ") and will try to partially initialize.";

bool ok;
double value = currentData.toDouble(&ok);
if(!ok) {
qCritical(CAT_ATTR_GUI_STRATEGY)
<< "Cannot partially initialize, something is very wrong here. " << currentData
<< " is not a double.";
m_spinBox->getLineEdit()->setText(currentData);
return;
}

m_spinBox->setStep(1);
m_spinBox->setValue(value);
return;
}

bool ok = true, finalOk = true;
double min = optionsList[0].toDouble(&ok);

double min = tryParse(optionsList[0], &ok);
finalOk &= ok;
double step = optionsList[1].toDouble(&ok);

double step = tryParse(optionsList[1], &ok);
finalOk &= ok;
double max = optionsList[2].toDouble(&ok);

double max = tryParse(optionsList[2], &ok);
finalOk &= ok;

double currentNum = QString(currentData).toDouble(&ok);
finalOk &= ok;

if(!finalOk) {
qWarning(CAT_ATTR_GUI_STRATEGY)
<< "Could not parse the values from" << availableAttributeValue << "as double ";
qCritical(CAT_ATTR_GUI_STRATEGY)
<< "Could not parse the values from" << availableAttributeValue << "as double or int.";
m_spinBox->setSpinButtonsDisabled(true);
m_spinBox->getLineEdit()->setText(currentData);
} else {
m_spinBox->setSpinButtonsDisabled(false);
m_spinBox->setMin(min);
m_spinBox->setMax(max);
m_spinBox->setStep(step);
m_spinBox->setValue(currentNum);
}
m_spinBox->getSpinBox()->setMinimum(min);
m_spinBox->getSpinBox()->setMaximum(max);
m_spinBox->getSpinBox()->setSingleStep(step);
m_spinBox->getSpinBox()->setValue(currentNum);

Q_EMIT displayedNewData(currentData, optionalData);
}

double RangeAttrUi::tryParse(QString number, bool *success)
{
// Try to parse as double first
bool ok = true;
double result = number.toDouble(&ok);
if(ok) {
*success = true;
return result;
}

// Try to parse as int and cast to double
int result_int = number.toInt(&ok);
if(ok) {
*success = true;
result = static_cast<double>(result_int);
return result;
}

*success = false;
return -1;
}

#include "moc_rangeguistrategy.cpp"
4 changes: 2 additions & 2 deletions iio-widgets/src/iiowidgetfactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
#include <iioutil/connectionprovider.h>
#include <QLoggingCategory>

#define ATTR_BUFFER_SIZE 256
#define ATTR_BUFFER_SIZE 16384
using namespace scopy;
Q_LOGGING_CATEGORY(CAT_ATTRFACTORY, "AttrFactory")

Expand All @@ -55,7 +55,7 @@ QList<IIOWidget *> IIOWidgetFactory::buildAllAttrsForChannel(struct iio_channel
}
}

for(const auto &attributeName : channelAttributes) {
for(const QString &attributeName : channelAttributes) {
if(attributeName.endsWith("_available")) {
continue;
}
Expand Down

0 comments on commit 6671e60

Please sign in to comment.