Skip to content

Replace int with std::chrono in for the timer->setInterval() argument #524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hebasto merged 1 commit into from
Feb 4, 2022
Merged

Replace int with std::chrono in for the timer->setInterval() argument #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/qt/rpcconsole.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
#include <QTimer>
#include <QVariant>

#include <chrono>

const int CONSOLE_HISTORY = 50;
const int INITIAL_TRAFFIC_GRAPH_MINS = 30;
const QSize FONT_RANGE(4, 40);
Expand Down Expand Up @@ -1140,7 +1142,7 @@ void RPCConsole::on_sldGraphRange_valueChanged(int value)

void RPCConsole::setTrafficGraphRange(int mins)
{
ui->trafficGraph->setGraphRangeMins(mins);
ui->trafficGraph->setGraphRange(std::chrono::minutes{mins});
ui->lblGraphRange->setText(GUIUtil::formatDurationStr(std::chrono::minutes{mins}));
}

Expand Down
15 changes: 6 additions & 9 deletions src/qt/trafficgraphwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <QColor>
#include <QTimer>

#include <chrono>
#include <cmath>

#define DESIRED_SAMPLES 800
Expand All @@ -22,7 +23,6 @@ TrafficGraphWidget::TrafficGraphWidget(QWidget *parent) :
QWidget(parent),
timer(nullptr),
fMax(0.0f),
nMins(0),
vSamplesIn(),
vSamplesOut(),
nLastBytesIn(0),
Expand All @@ -42,10 +42,7 @@ void TrafficGraphWidget::setClientModel(ClientModel *model)
}
}

int TrafficGraphWidget::getGraphRangeMins() const
{
return nMins;
}
std::chrono::minutes TrafficGraphWidget::getGraphRange() const { return m_range; }

void TrafficGraphWidget::paintPath(QPainterPath &path, QQueue<float> &samples)
{
Expand Down Expand Up @@ -153,12 +150,12 @@ void TrafficGraphWidget::updateRates()
update();
}

void TrafficGraphWidget::setGraphRangeMins(int mins)
void TrafficGraphWidget::setGraphRange(std::chrono::minutes new_range)
{
nMins = mins;
int msecsPerSample = nMins * 60 * 1000 / DESIRED_SAMPLES;
m_range = new_range;
const auto msecs_per_sample{std::chrono::duration_cast<std::chrono::milliseconds>(m_range) / DESIRED_SAMPLES};
timer->stop();
timer->setInterval(msecsPerSample);
timer->setInterval(msecs_per_sample);

clear();
}
Expand Down
8 changes: 5 additions & 3 deletions src/qt/trafficgraphwidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include <QWidget>
#include <QQueue>

#include <chrono>

class ClientModel;

QT_BEGIN_NAMESPACE
Expand All @@ -22,22 +24,22 @@ class TrafficGraphWidget : public QWidget
public:
explicit TrafficGraphWidget(QWidget *parent = nullptr);
void setClientModel(ClientModel *model);
int getGraphRangeMins() const;
std::chrono::minutes getGraphRange() const;
Copy link
Contributor

@promag promag Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remove? It's not used.
Copy link
Contributor

@RandyMcMillan RandyMcMillan Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@promag @shaavan

I concur - It appears to be part of the original implementation but never used for anything...

it can and should be removed...

https://github.com/bitcoin-core/gui/blame/16781e1bc9f8ffc721ebea73434e0066957bc959/src/qt/trafficgraphwidget.cpp#L45

Screen Shot 2022-01-20 at 1 16 34 PM
Copy link
Contributor

@RandyMcMillan RandyMcMillan Jan 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@promag - great catch!
Copy link
Contributor Author

@shaavan shaavan Jan 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @promag for catching this. Addressed in 5efd391
Copy link
Contributor

@RandyMcMillan RandyMcMillan Jan 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK Removing the TrafficGraphWidget::getGraphRangeMins() function nACK

I use int TrafficGraphWidget::getGraphRangeMins() const in PR #539 (which fixes issue #532)

I have added a variation to of this PR to #539 c25d3f4 to ensure compatibility with the chrono lib.

gui/src/qt/trafficgraphwidget.cpp

Line 46 in c25d3f4

Note

https://github.com/bitcoin-core/gui/pull/524/files#diff-da38549cd85e0c422891b8ef1eb3cc33e76f0b80d15ca63aa7ad6f065c8ba694L44

protected:
void paintEvent(QPaintEvent *) override;

public Q_SLOTS:
void updateRates();
void setGraphRangeMins(int mins);
void setGraphRange(std::chrono::minutes new_range);
void clear();

private:
void paintPath(QPainterPath &path, QQueue<float> &samples);

QTimer *timer;
float fMax;
int nMins;
std::chrono::minutes m_range{0};
QQueue<float> vSamplesIn;
QQueue<float> vSamplesOut;
quint64 nLastBytesIn;
Expand Down