Skip to content

Add the possibility to monitor multiple hard drives#514

Open
Omega3395 wants to merge 2 commits into
ros:ros2from
Omega3395:ros2
Open

Add the possibility to monitor multiple hard drives#514
Omega3395 wants to merge 2 commits into
ros:ros2from
Omega3395:ros2

Conversation

@Omega3395
Copy link
Copy Markdown

Add the possibility to monitor multiple hard drives within a single instance of the node (it was previously possible to do it with multiple hd_monitor instances).

@mergify mergify Bot added the ros2 PR tackling a ROS2 branch label Jun 23, 2025
@ct2034 ct2034 added the enhancement This tackles a new feature of the code (and not a bug) label Jul 7, 2025
self._updater = Updater(self)
self._updater.setHardwareID(hostname)
self._updater.add(f'{hostname} HD Usage', self.check_disk_usage)
self._updater.setHardwareID("main_pc")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why not hostname?


# Add diagnostic check for each path
for i, path in enumerate(self._paths):
self._updater.add(f'HD Usage {i}', lambda diag, idx=i: self.check_disk_usage(diag, idx))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we have the path in here

self.add_on_set_parameters_callback(self.callback_config)
self.declare_parameter('path', self._path, ParameterDescriptor(
description='Path in which to check remaining space.'))
self.declare_parameter('path', self._paths, ParameterDescriptor(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self.declare_parameter('path', self._paths, ParameterDescriptor(
self.declare_parameter('paths', self._paths, ParameterDescriptor(

Please change this everywhere accordingly

Comment on lines 108 to +110
if param.name == 'path':
self._path = str(
Path(param.value).expanduser().resolve(strict=True)
)
# Handle both single path (string) and multiple paths (array)
if isinstance(param.value, str):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we could also additionally keep the path param for backwards compatibility. what do you think?

return SetParametersResult(successful=True)

def check_disk_usage(self, diag: DiagnosticStatus) -> DiagnosticStatus:
def check_disk_usage(self, diag: DiagnosticStatus, path_index: int = 0) -> DiagnosticStatus:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why introduce an index here? why no by path directly?

diag.level = DiagnosticStatus.ERROR

total_go = total // (1024 * 1024)
total_gb = total // (1024 ** 3)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

was this wrong? Why is it ^3 now?

@ct2034 ct2034 added the needs more work Someone has worked on this but more work is needed label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement This tackles a new feature of the code (and not a bug) needs more work Someone has worked on this but more work is needed ros2 PR tackling a ROS2 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants