Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 63 additions & 61 deletions src/gui/SystemInfoDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,65 @@ SystemInfoDialog::SystemInfoDialog(MemoryHistoryRing* history, CpuHistoryRing* c
m_tickLagMeter = tickLagMeter;
}
auto* layout = new QVBoxLayout(bodyWidget());

// One timeframe for every chart in the dialog, in the window header where
// the network dialog keeps its own (#5496). Two per-tab selectors with the
// same four choices could disagree: a range chosen on Overview was not
// the range Memory then showed. Hidden while a tab with no chart is
// current — Threads has a fixed 60 s window and Logs has no time axis —
// which is how the network dialog handles its Logs and TCI pages.
auto* header = new QHBoxLayout;
header->addStretch(1);
m_rangeLabel = new QLabel(QStringLiteral("Timeframe"), bodyWidget());
m_rangeLabel->setAccessibleName(QStringLiteral("Chart timeframe"));
m_range = new QComboBox(bodyWidget());
m_range->setObjectName(QStringLiteral("systemInfoTimeframe"));
m_range->setAccessibleName(QStringLiteral("Chart timeframe"));
m_range->setAccessibleDescription(
QStringLiteral("Choose how much recent history the charts display."));
m_range->setFixedWidth(132);
// The issue's four (#2554); the rings hold an hour raw, so nothing longer
// is offered.
m_range->addItem(QStringLiteral("1 minute"), 60);
m_range->addItem(QStringLiteral("5 minutes"), 5 * 60);
m_range->addItem(QStringLiteral("15 minutes"), 15 * 60);
m_range->addItem(QStringLiteral("1 hour"), 60 * 60);
m_range->setCurrentIndex(1); // 5 minutes: 200 points at 1.5 s
// Hidden, not removed: the row keeps its height while Threads or Logs is
// current, so the tab strip does not jump under the pointer.
for (QWidget* w : {static_cast<QWidget*>(m_rangeLabel), static_cast<QWidget*>(m_range)}) {
QSizePolicy policy = w->sizePolicy();
policy.setRetainSizeWhenHidden(true);
w->setSizePolicy(policy);
}
// Both refreshes, not only the current tab's: switching tabs must never
// show a chart still drawn to the previous range.
connect(m_range, &QComboBox::currentIndexChanged, this,
&SystemInfoDialog::refreshMemoryChart);
connect(m_range, &QComboBox::currentIndexChanged, this,
&SystemInfoDialog::refreshOverview);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked the ordering hazard this introduces and it holds: m_range is fully populated and setCurrentIndex(1) has already run by line 243, both connects come after it, and buildOverviewTab()/buildMemoryTab() only run at 264-267 — so neither slot can fire against null graph pointers during construction, and the default selection raises no spurious refresh. refreshOverview is null-guarded on all four m_overview*Graph members besides.

No change requested; recording it because "one combo now drives two refreshes" is the place this would have gone wrong.

header->addWidget(m_rangeLabel);
header->addWidget(m_range);
layout->addLayout(header);

auto* tabs = new QTabWidget(bodyWidget());
// The issue's order: Overview / Threads / Memory / (Painters) / Logs.
tabs->addTab(buildOverviewTab(), QStringLiteral("Overview"));
tabs->addTab(buildThreadsTab(), QStringLiteral("Threads"));
QWidget* threadsTab = buildThreadsTab();
tabs->addTab(threadsTab, QStringLiteral("Threads"));
tabs->addTab(buildMemoryTab(), QStringLiteral("Memory"));
tabs->addTab(buildLogsTab(), QStringLiteral("Logs"));
QWidget* logsTab = buildLogsTab();
tabs->addTab(logsTab, QStringLiteral("Logs"));
// By page, not by index: the order has changed once already (#5427 put
// Overview first) and the rule is about which pages draw a chart.
const auto showTimeframeForPage = [this, tabs, threadsTab, logsTab](int) {
QWidget* page = tabs->currentWidget();
const bool showTimeframe = page != threadsTab && page != logsTab;
m_rangeLabel->setVisible(showTimeframe);
m_range->setVisible(showTimeframe);
};
connect(tabs, &QTabWidget::currentChanged, this, showTimeframeForPage);
showTimeframeForPage(tabs->currentIndex());
layout->addWidget(tabs);

auto* buttonRow = new QHBoxLayout;
Expand Down Expand Up @@ -567,36 +620,14 @@ QWidget* SystemInfoDialog::buildMemoryTab()
auto* page = new QWidget;
auto* layout = new QVBoxLayout(page);

// Header row: what is being measured, and — top-right, where the network
// dialog keeps its own — how much history the chart shows. The selector
// lives on this tab rather than the window because Threads has a fixed
// 60 s window and Logs has none; the network dialog reaches the same
// outcome by hiding its combo on those pages.
// Header row: what is being measured. How much history the chart shows
// is the window's timeframe above the tabs, shared with Overview (#5496).
auto* header = new QHBoxLayout;
m_memorySummary = new QLabel(QStringLiteral("Sampling…"), page);
m_memorySummary->setObjectName(QStringLiteral("systemInfoMemorySummary"));
m_memorySummary->setAccessibleName(QStringLiteral("Process memory summary"));
header->addWidget(m_memorySummary);
header->addStretch(1);
auto* rangeLabel = new QLabel(QStringLiteral("Timeframe"), page);
rangeLabel->setAccessibleName(QStringLiteral("Chart timeframe"));
m_memoryRange = new QComboBox(page);
m_memoryRange->setObjectName(QStringLiteral("systemInfoTimeframe"));
m_memoryRange->setAccessibleName(QStringLiteral("Chart timeframe"));
m_memoryRange->setAccessibleDescription(
QStringLiteral("Choose how much recent memory history the chart displays."));
m_memoryRange->setFixedWidth(132);
// The issue's four; the ring holds an hour raw, so nothing longer is offered
// until the compacting history arrives with the Overview tab.
m_memoryRange->addItem(QStringLiteral("1 minute"), 60);
m_memoryRange->addItem(QStringLiteral("5 minutes"), 5 * 60);
m_memoryRange->addItem(QStringLiteral("15 minutes"), 15 * 60);
m_memoryRange->addItem(QStringLiteral("1 hour"), 60 * 60);
m_memoryRange->setCurrentIndex(1); // 5 minutes: 200 points at 1.5 s
connect(m_memoryRange, &QComboBox::currentIndexChanged, this,
&SystemInfoDialog::refreshMemoryChart);
header->addWidget(rangeLabel);
header->addWidget(m_memoryRange);
layout->addLayout(header);

// Readouts: the numbers, not only the line. Virtual is a readout only —
Expand Down Expand Up @@ -665,12 +696,12 @@ QWidget* SystemInfoDialog::buildMemoryTab()
return page;
}

int SystemInfoDialog::selectedMemoryRangeSeconds() const
int SystemInfoDialog::selectedRangeSeconds() const
{
if (m_memoryRange == nullptr) {
if (m_range == nullptr) {
return 5 * 60;
}
return m_memoryRange->currentData().toInt();
return m_range->currentData().toInt();
}

void SystemInfoDialog::applyMemorySample(const MemorySample& sample)
Expand Down Expand Up @@ -732,7 +763,7 @@ void SystemInfoDialog::refreshMemoryChart()
if (m_memoryGraph == nullptr) {
return;
}
const int rangeSeconds = selectedMemoryRangeSeconds();
const int rangeSeconds = selectedRangeSeconds();
// The window ends at the newest sample, not at the wall clock: a dialog
// whose sampling is paused shows the history it has, in place, instead of
// sliding it off the left edge while nothing new arrives.
Expand Down Expand Up @@ -763,28 +794,7 @@ QWidget* SystemInfoDialog::buildOverviewTab()
auto* page = new QWidget;
auto* layout = new QVBoxLayout(page);

// Timeframe top-right, as on the Memory tab and in the network dialog;
// per tab rather than per window for the reason the Memory tab gives.
auto* header = new QHBoxLayout;
header->addStretch(1);
auto* rangeLabel = new QLabel(QStringLiteral("Timeframe"), page);
rangeLabel->setAccessibleName(QStringLiteral("Chart timeframe"));
m_overviewRange = new QComboBox(page);
m_overviewRange->setObjectName(QStringLiteral("systemInfoOverviewTimeframe"));
m_overviewRange->setAccessibleName(QStringLiteral("Chart timeframe"));
m_overviewRange->setAccessibleDescription(
QStringLiteral("Choose how much recent history the Overview charts display."));
m_overviewRange->setFixedWidth(132);
m_overviewRange->addItem(QStringLiteral("1 minute"), 60);
m_overviewRange->addItem(QStringLiteral("5 minutes"), 5 * 60);
m_overviewRange->addItem(QStringLiteral("15 minutes"), 15 * 60);
m_overviewRange->addItem(QStringLiteral("1 hour"), 60 * 60);
m_overviewRange->setCurrentIndex(1);
connect(m_overviewRange, &QComboBox::currentIndexChanged, this,
&SystemInfoDialog::refreshOverview);
header->addWidget(rangeLabel);
header->addWidget(m_overviewRange);
layout->addLayout(header);
// The charts' timeframe is the window's, above the tabs (#5496).

// Four cards across, the network dialog's row.
auto* cards = new QHBoxLayout;
Expand Down Expand Up @@ -893,14 +903,6 @@ QWidget* SystemInfoDialog::buildOverviewTab()
return scroll;
}

int SystemInfoDialog::selectedOverviewRangeSeconds() const
{
if (m_overviewRange == nullptr) {
return 5 * 60;
}
return m_overviewRange->currentData().toInt();
}

void SystemInfoDialog::setCardLevel(QLabel* value, SystemInfo::CardLevel level)
{
if (value == nullptr) {
Expand Down Expand Up @@ -960,7 +962,7 @@ void SystemInfoDialog::refreshOverview()
{
const CpuHistoryRing::Record* cpu = m_cpuRing->latest();
const MemoryHistoryRing::Record* mem = m_memoryRing->latest();
const int rangeSeconds = selectedOverviewRangeSeconds();
const int rangeSeconds = selectedRangeSeconds();
const double gapSeconds = CpuHistoryRing::connectGapSecondsFor(rangeSeconds);
ThemeManager& theme = ThemeManager::instance();

Expand Down
8 changes: 4 additions & 4 deletions src/gui/SystemInfoDialog.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ private slots:

void applyAlertStyle();
void refreshMemoryChart();
int selectedMemoryRangeSeconds() const;
void refreshOverview();
int selectedOverviewRangeSeconds() const;
// The window-level timeframe both refreshes draw to (#5496).
int selectedRangeSeconds() const;
// Colour a card's value for its band and expose the band as the label's
// "level" property ("normal" / "warning" / "danger") for tests and the
// automation bridge, which read properties and not stylesheets.
Expand Down Expand Up @@ -160,7 +160,8 @@ private slots:
CpuHistoryRing* m_cpuRing{&m_ownCpuRing};
UiTickLagMeter m_ownTickLagMeter;
UiTickLagMeter* m_tickLagMeter{&m_ownTickLagMeter};
QComboBox* m_overviewRange{nullptr};
QLabel* m_rangeLabel{nullptr};
QComboBox* m_range{nullptr};
QLabel* m_cardCpuValue{nullptr};
QLabel* m_cardMaxThreadValue{nullptr};
QLabel* m_cardMaxThreadCaption{nullptr};
Expand All @@ -171,7 +172,6 @@ private slots:
TimeSeriesGraphWidget* m_overviewThreadsGraph{nullptr};
TimeSeriesGraphWidget* m_overviewTickGraph{nullptr};
TimeSeriesGraphWidget* m_memoryGraph{nullptr};
QComboBox* m_memoryRange{nullptr};
QLabel* m_memorySummary{nullptr};
QLabel* m_memoryResident{nullptr};
QLabel* m_memoryPeak{nullptr};
Expand Down
8 changes: 5 additions & 3 deletions tests/system_info_dialog_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,10 @@ int main(int argc, char** argv)
qRegisterMetaType<AetherSDR::MemorySample>("AetherSDR::MemorySample");
SystemInfoDialog memoryDialog;

// The selector is the dialog's, in the window header (#5496); the
// lookup on the dialog finds it there as it did on the Memory tab.
auto* range = memoryDialog.findChild<QComboBox*>(QStringLiteral("systemInfoTimeframe"));
report("the Memory tab has a timeframe selector", range != nullptr);
report("the dialog has a timeframe selector", range != nullptr);
if (range != nullptr) {
report("it offers the issue's four timeframes", range->count() == 4);
report("it defaults to 5 minutes", range->currentData().toInt() == 5 * 60);
Expand Down Expand Up @@ -659,8 +661,8 @@ int main(int argc, char** argv)
{
qRegisterMetaType<AetherSDR::CpuSample>("AetherSDR::CpuSample");
SystemInfoDialog ov;
auto* range = ov.findChild<QComboBox*>(QStringLiteral("systemInfoOverviewTimeframe"));
report("the Overview tab has its own timeframe selector", range != nullptr && range->count() == 4);
report("the Overview tab has no timeframe selector of its own (#5496)",
ov.findChild<QComboBox*>(QStringLiteral("systemInfoOverviewTimeframe")) == nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is now the only Overview-side assertion about the timeframe, and it is purely negative — it passes as long as the old object name is absent. Combined with the Memory case (which asserts only that the readouts are undisturbed by setCurrentIndex(3)), deleting both connect lines in SystemInfoDialog.cpp:254-258 would leave the whole file green.

The redraw itself is genuinely hard to assert — TimeSeriesGraphWidget keeps m_rangeSeconds private with no getter — so I'm not asking for that. But the issue's third acceptance criterion is cheap in this already-registered socket-free target:

        report("the Overview tab has no timeframe selector of its own (#5496)",
               ov.findChild<QComboBox*>(QStringLiteral("systemInfoOverviewTimeframe")) == nullptr);
        // ...and the shared one hides on the tabs with no chart (#5496).
        auto* ovTabs  = ov.findChild<QTabWidget*>();
        auto* ovRange = ov.findChild<QComboBox*>(QStringLiteral("systemInfoTimeframe"));
        if (ovTabs != nullptr && ovRange != nullptr) {
            ov.show();
            QCoreApplication::processEvents();
            report("the timeframe shows on Overview", ovRange->isVisible());
            ovTabs->setCurrentIndex(1);   // Threads: fixed 60 s window
            report("the timeframe hides on Threads", !ovRange->isVisible());
            ovTabs->setCurrentIndex(2);   // Memory: charted again
            report("the timeframe returns on Memory", ovRange->isVisible());
            ov.hide();
        }

Non-blocking.

auto* cpuCard = ov.findChild<QLabel*>(QStringLiteral("systemInfoCardCpu"));
auto* maxCard = ov.findChild<QLabel*>(QStringLiteral("systemInfoCardMaxThread"));
auto* memCard = ov.findChild<QLabel*>(QStringLiteral("systemInfoCardMemory"));
Expand Down
Loading