feat: add Jetson Thor fan discovery for split PWM/RPM hwmon pairing#837
Merged
johnnynunez merged 3 commits intorbonghi:masterfrom Apr 2, 2026
Merged
feat: add Jetson Thor fan discovery for split PWM/RPM hwmon pairing#837johnnynunez merged 3 commits intorbonghi:masterfrom
johnnynunez merged 3 commits intorbonghi:masterfrom
Conversation
Enable jtop fan discovery for Jetson Thor, where fan control and tachometer data are exposed in separate hwmon nodes. This change merges split PWM/RPM hwmon entries into one logical fan, allowing jtop to show Thor fan speed control and RPM correctly.
Contributor
Reviewer's GuideRefactors fan discovery and nvfancontrol integration to support Jetson Thor’s split PWM/RPM hwmon layout, adds richer metadata (pwm_enable, kickstart, topology-based matching), and simplifies the Fan/FanService APIs and logging while improving robustness of parsing and matching. Sequence diagram for hwmon fan discovery and Thor split PWM/RPM mergesequenceDiagram
participant FanService
participant get_all_cooling_system
participant _collect_hwmon_fans
participant _merge_split_fans
participant is_thor
participant sys_class_hwmon as sys_class_hwmon
FanService->>get_all_cooling_system: root_dir
alt root_dir_not_directory
get_all_cooling_system-->>FanService: {}
else root_dir_ok
get_all_cooling_system->>sys_class_hwmon: listdir(root_dir)
get_all_cooling_system->>_collect_hwmon_fans: root_dir
_collect_hwmon_fans->>sys_class_hwmon: listdir(each_hwmon_dir)
_collect_hwmon_fans-->>get_all_cooling_system: pwm_files,rpm_only
get_all_cooling_system->>_merge_split_fans: pwm_files,rpm_only
alt no_pwm_or_no_rpm_only
_merge_split_fans-->>get_all_cooling_system: pwm_files
else have_pwm_and_rpm_only
loop match_by_topology
_merge_split_fans->>_merge_split_fans: compare of_node/device
alt match_found
_merge_split_fans->>_merge_split_fans: attach rpm_to_pwm_entry
end
end
_merge_split_fans->>is_thor: ()
alt is_thor_true_and_single_pwm_and_rpm
_merge_split_fans->>_merge_split_fans: attach_rpm_to_single_pwm
else generic_single_device_fallback
_merge_split_fans->>_merge_split_fans: attach_rpm_to_single_pwm
end
_merge_split_fans-->>get_all_cooling_system: merged_pwm_files
end
get_all_cooling_system-->>FanService: merged_pwm_files
end
FanService->>FanService: _fan_list = merged_pwm_files + get_all_legacy_fan()
Sequence diagram for nvfancontrol metadata merging into fan statussequenceDiagram
participant FanService
participant get_status
participant nvfancontrol_is_active
participant nvfancontrol_query
FanService->>get_status:()
get_status->>get_status: build fan_status from _fan_list
alt _nvfancontrol_false
get_status-->>FanService: fan_status_with_manual_or_temp_control
else _nvfancontrol_true
get_status->>nvfancontrol_is_active:()
alt service_active
get_status->>nvfancontrol_query:()
nvfancontrol_query-->>get_status: nvfan_query
else service_inactive
get_status-->>get_status: nvfan_query = {}
end
alt nvfan_query_not_empty
alt single_fan_single_nv_entry
get_status->>get_status: merge_single_nv_entry_into_single_fan
else multiple_fans_or_entries
loop for_each_fan_in_fan_status
alt fan_name_in_nvfan_query
get_status->>get_status: update_fan_status_with_nv_metadata
else no_matching_nv_entry
get_status->>get_status: set_profile_to_manual
end
end
end
else nvfan_query_empty
loop for_each_fan
get_status->>get_status: set_profile_to_manual
end
end
get_status-->>FanService: fan_status_with_nvfan_metadata
end
Updated class diagram for Fan and FanServiceclassDiagram
class GenericInterface {
}
class Fan {
+Fan()
+all_profiles(name)
+set_profile(name, profile)
+get_profile(name)
+get_profile_default(name)
+get_speed(name, idx)
+set_speed(name, speed, idx)
+get_rpm(name, idx)
+profile
+speed
+rpm
-_data
-_init
-_controller
}
class FanService {
+FanService(config)
+initialization()
+get_configs()
+get_profile(name)
+set_profile(name, profile)
+set_speed(name, speed, index)
+get_status()
-_config
-_fan_list
-_nvfancontrol
}
class Config {
+get(key, default)
+set(key, value)
}
Fan --|> GenericInterface
FanService --> Config
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider either using or removing the new
read_nvfancontrol_status()helper to avoid keeping unused parsing logic that can drift fromnvfancontrol_query()over time. - In
_merge_split_fans, any remainingunmatched_rpmentries are silently dropped; adding a warning log for those cases would make diagnosing mis-paired or unpaired hwmon RPM nodes much easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider either using or removing the new `read_nvfancontrol_status()` helper to avoid keeping unused parsing logic that can drift from `nvfancontrol_query()` over time.
- In `_merge_split_fans`, any remaining `unmatched_rpm` entries are silently dropped; adding a warning log for those cases would make diagnosing mis-paired or unpaired hwmon RPM nodes much easier.
## Individual Comments
### Comment 1
<location path="jtop/core/fan.py" line_range="76-85" />
<code_context>
+ for entry in sorted(os.listdir(root_dir)):
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against `OSError` when listing `root_dir` itself to make `_collect_hwmon_fans` more robust.
In `_collect_hwmon_fans`, `os.listdir(full_path)` is wrapped in `try/except OSError`, but the initial `sorted(os.listdir(root_dir))` is not. If `root_dir` is temporarily unreadable, this will raise and skip the error handling in `get_all_cooling_system`. Consider wrapping the outer `os.listdir(root_dir)` in the same `try/except OSError` and returning empty dicts on failure to keep fan discovery robust.
Suggested implementation:
```python
rpm_only = {}
try:
entries = sorted(os.listdir(root_dir))
except OSError:
return {}, {}
for entry in entries:
```
1. Ensure `_collect_hwmon_fans`'s return statement matches the `return {}, {}` shape used in the new `except OSError` block. If `_collect_hwmon_fans` currently returns a different number or type of values, adjust the `return` in the `except` to match (e.g., `return {}` or `return {}, {}, {}`).
2. Confirm all call sites of `_collect_hwmon_fans` correctly handle the empty return structure so that downstream code treats this as "no fans discovered" rather than an error.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable jtop fan discovery for Jetson Thor, where fan control and tachometer data are exposed in separate hwmon nodes. This change merges split PWM/RPM hwmon entries into one logical fan, allowing jtop to show Thor fan speed control and RPM correctly.
Summary by Sourcery
Improve fan discovery and control metadata handling, including support for split PWM/RPM hwmon layouts such as Jetson Thor.
New Features:
Enhancements: