Add configurable hardware sensor status bar - #2076
Conversation
📝 WalkthroughWalkthroughAdds Linux hardware sensor discovery with cached readings and statistics. Extends Poem
Merge Risk: 🔵 Low · up to Mouse clicks on the hardware sensor status-bar row may trigger an adjacent UI action instead of being handled by the status bar, depending on function-bar visibility. The issue is localized and non-blocking but should be addressed or explicitly accepted by the owner. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50a90f06-947f-446e-805d-86a54664a570
📒 Files selected for processing (20)
CategoriesPanel.cDisplayOptionsPanel.cHardwareSensor.hMakefile.amScreenManager.cSettings.cSettings.hStatusBar.cStatusBar.hStatusBarConfig.hStatusBarSensorOptionsPanel.cStatusBarSensorOptionsPanel.hStatusBarSensorsPanel.cStatusBarSensorsPanel.hlinux/LibSensors.clinux/LibSensors.hlinux/LinuxMachine.clinux/LinuxMachine.hlinux/Platform.clinux/Platform.h
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| StatusBarSensorOptionsPanel* StatusBarSensorOptionsPanel_new(Machine* host, StatusBarSensorsPanel* sensorsPanel) { | ||
| StatusBarSensorOptionsPanel* this = AllocThis(StatusBarSensorOptionsPanel); | ||
| Panel* super = &this->super; | ||
|
|
||
| FunctionBar* fuBar = FunctionBar_new(StatusBarSensorOptionsFunctions, NULL, NULL); | ||
| Panel_init(super, 1, 1, 1, 1, Class(OptionItem), true, fuBar); | ||
|
|
||
| this->host = host; | ||
| this->sensorsPanel = sensorsPanel; | ||
| this->sensor = NULL; | ||
|
|
||
| Panel_setHeader(super, "Sensor options"); | ||
|
|
||
| return this; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Remove the unused host member or document its purpose.
this->host is assigned here and never read in this translation unit. Panel_setHeader also runs twice for the same panel: once here and once in StatusBarSensorOptionsPanel_fill. Drop the field from StatusBarSensorOptionsPanel and the parameter from the constructor, or keep them and remove the duplicate header assignment.
🧰 Tools
🪛 Cppcheck (2.21.0)
[style] 127-127: The function 'ScreenSettings_getActiveSortKey' is never used.
(unusedFunction)
[style] 115-115: The function 'StatusBarSensorOptionsPanel_new' is never used.
(unusedFunction)
| for (size_t selection = 0; selection < this->settings->statusBarSensorCount; selection++) { | ||
| const StatusBarSensorConfig* config = &this->settings->statusBarSensors[selection]; | ||
| bool found = false; | ||
|
|
||
| #if defined(HTOP_LINUX) && defined(HAVE_SENSORS_SENSORS_H) | ||
| for (size_t i = 0; i < count; i++) { | ||
| const char* id = NULL; | ||
| const char* chip = NULL; | ||
| const char* label = NULL; | ||
| const char* feature = NULL; | ||
| HardwareSensorType type; | ||
|
|
||
| if (!Platform_getHardwareSensor(this->host, i, &id, &chip, &label, &feature, &type, NULL) || !id) | ||
| continue; | ||
|
|
||
| if (String_eq(id, config->id)) { | ||
| char name[64]; | ||
| StatusBar_formatSensorName(name, sizeof(name), chip, label, feature, type); | ||
| StatusBarSensorsPanel_addConfigured(this, name, id, config->enabled, config->showMin, config->showAverage, config->showMax); | ||
| found = true; | ||
| break; | ||
| } | ||
| } | ||
| #endif | ||
|
|
||
| if (!found) { | ||
| char name[256]; | ||
| xSnprintf(name, sizeof(name), "%s (unavailable)", config->id); | ||
| StatusBarSensorsPanel_addConfigured(this, name, config->id, config->enabled, config->showMin, config->showAverage, config->showMax); | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
Sensor lookup is O(configured × discovered).
For every configured sensor the code rescans all discovered sensors. The counts are small in practice, so this is acceptable, but a single pass that marks matched sensors would be simpler and avoids the nested scan. Treat this as optional.
🧰 Tools
🪛 Cppcheck (2.21.0)
[style] 165-165: The function 'Row_idEqualCompare' is never used.
(unusedFunction)
[style] 176-176: The function 'Row_isChildOf' is never used.
(unusedFunction)
[style] 186-186: The function 'skipEndOfLine' is never used.
(unusedFunction)
| extern ObjectClass StatusBarSensorListItem_class; | ||
| extern const PanelClass StatusBarSensorsPanel_class; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the declaration convention for ObjectClass instances in the repository.
rg -nP '^\s*(extern\s+)?(const\s+)?ObjectClass\s+\w+_class' --type=c --type=cpp -g '*.h' -g '*.c'Repository: htop-dev/htop
Length of output: 151
🏁 Script executed:
#!/bin/bash
set -eu
file="$(fd -t f 'StatusBarSensorsPanel\.(h|c)$' . | head -n 1)"
printf '%s\n' "== target: $file =="
cat -n "$file"
printf '%s\n' '== ObjectClass declarations and definitions =='
rg -n --glob '*.{h,c}' '(^|[[:space:]])(extern[[:space:]]+)?(const[[:space:]]+)?ObjectClass[[:space:]]+[A-Za-z_][A-Za-z0-9_]*_class' .
printf '%s\n' '== target symbol uses =='
rg -n 'StatusBarSensorListItem_class' .Repository: htop-dev/htop
Length of output: 15603
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '== ObjectClass definition and class access macros =='
cat -n Object.h | sed -n '1,180p'
printf '%s\n' '== representative headers and definitions =='
cat -n ListItem.h | sed -n '1,90p'
cat -n ListItem.c | sed -n '50,85p'
cat -n ScreensPanel.h | sed -n '35,65p'
cat -n ScreenTabsPanel.c | sed -n '20,45p'
printf '%s\n' '== all assignments and writes through ObjectClass references =='
rg -n --glob '*.{c,h}' 'ObjectClass|->extends|Class\(' | head -n 300Repository: htop-dev/htop
Length of output: 6102
Declare and define StatusBarSensorListItem_class as const ObjectClass.
The class metadata is read through const ObjectClass* and is not modified. This matches ListItem_class and StatusBarSensorsPanel_class.
Add an optional Linux status bar backed by libsensors, with configurable temperature and fan sensors, per-sensor Min/Avg/Max statistics, ordering, persistence, and terminal-width-aware rendering. Sensor polling is limited to when the status bar is enabled, while startup discovery keeps sensor configuration available in Setup. Signed-off-by: Massimo Mazzariol <mazzariol.massimo@gmail.com>
e71d638 to
ffe0445
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 13acab5e-5530-42c6-b812-e3784e4992d4
📒 Files selected for processing (5)
CategoriesPanel.cStatusBarSensorOptionsPanel.cStatusBarSensorOptionsPanel.hStatusBarSensorsPanel.cStatusBarSensorsPanel.h
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
|
||
|
|
||
| struct StatusBarSensorOptionsPanel_; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Add a second blank line after each include block.
The project convention requires exactly two blank lines between the last contiguous #include directive and the first declaration.
StatusBarSensorsPanel.h#L15-L17: Insert one blank line beforestruct StatusBarSensorOptionsPanel_;.StatusBarSensorOptionsPanel.h#L12-L14: Insert one blank line beforetypedef struct StatusBarSensorOptionsPanel_.
Based on learnings: after the last contiguous #include directive and before the first declaration or typedef, there must be exactly two blank lines.
📍 Affects 2 files
StatusBarSensorsPanel.h#L15-L17(this comment)StatusBarSensorOptionsPanel.h#L12-L14
Source: Learnings
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6b9bd413-88b3-432a-bad6-4b65b9ea80d6
📒 Files selected for processing (4)
ScreenManager.cStatusBar.cStatusBarSensorOptionsPanel.cStatusBarSensorsPanel.c
💤 Files with no reviewable changes (1)
- StatusBarSensorOptionsPanel.c
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| if (showStatusBar) | ||
| StatusBar_draw(this->host, State_hideFunctionBar(this->state) ? LINES - 1 : LINES - 2); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Consume mouse clicks on the status-bar row.
When the function bar is visible, a click on LINES - 2 reaches the panel hit test. When the function bar is hidden, a click on LINES - 1 invokes FunctionBar_synthesizeEvent. Handle the status-bar row before function-bar and panel dispatch.
|
I think this should be a meter like other similar info is being displayed as a meter in |
Thanks @fasterit for the feedback! Well, that makes sense I had a look at the existing Meter/Header infrastructure, and I think I can rework this PR to use the normal htop meter design instead of introducing a separate status-bar row. The libsensors discovery and sampling code can stay in the platform/backend layer while the custom status-bar UI, ScreenManager integration and Status Bar setup page can be removed and replaced by a Meter implementation using the existing header configuration. But before I start reworking it, would you prefer a single Hardware Sensors meter displaying the selected sensors, or individual per-sensor meters exposed through Available Meters? I'm happy to realign the implementation with whichever approach fits htop's design at its best. Please let me know and thanks again for the consideration of this feature |
|
I don't know, @massimomazzariol. I have no need for this type of a meter. |
2 participants
Summary
Add an optional horizontal hardware sensor status bar above the function bar on Linux systems with libsensors support.
Therefore, the status bar can display temperature and fan sensors while keeping the main process view compact.
Screenshots
Hardware sensor status bar
The status bar displays the selected hardware sensors directly above the function bar
Display option
The hardware sensor status bar can be enabled or disabled from the standard Display options
Sensor configuration
Sensors can be individually enabled, reordered, and configured to show runtime minimum, average, and maximum values
Features
htoprcSensor configuration uses libsensors chip and feature identifiers rather than display labels, so configuration does not depend on sensor labels
Setup
The feature adds:
Show hardware sensor status barunder the general Display optionsStatus BarSetup page containing the available sensorsNewly discovered sensors are appended disabled once an explicit configuration exists, while configured sensors that are temporarily unavailable remain in the configuration
Runtime behavior
Hardware sensors are discovered during startup so they are available in Setup even when the status bar is disabled.
Statistics are collected only while the status bar is active and start from the first runtime sample.
The initial implementation does not periodically perform a full libsensors rediscovery for newly hot-plugged hardware.
Testing
Tested on Raspberry Pi with libsensors using:
--enable-werror --enable-unicode --enable-sensors--disable-sensors --disable-unicodeminimal buildmake distcheckFurther testing would be very welcome, especially on x86_64 Intel/AMD systems, different distributions and libsensors configurations, and systems exposing a larger variety of temperature and fan sensors (especially servers).
I would particularly appreciate feedback from maintainers and users with different hardware sensor layouts.