Skip to content
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

Jump to bottom

Port hd_monitor to ROS2 #334

Merged
merged 10 commits into from
Jul 3, 2024
Merged

Port hd_monitor to ROS2 #334

merged 10 commits into from
Jul 3, 2024

Conversation

limaanto
Copy link
Contributor

As promised in #319 (though it took quite a while..), here is a ported version of the HD monitor script.

After the discussions of #319, I decided to :

  • drop the temperature check that was not deemed mandatory and would have required significant work
  • use a widely available and windows-compatible tool for the usage check, and shutil.disk_usage which comes with all python installs seemed like a good choice
  • Introduce a new parameter path to configure where the check is ran on. By default this is the home directory but can be changed with a ros param
  • Use only the local hostname as the device_id

I did not take the time to write the test yet, let me know if this is mandatory for merging (or if there is any other point for that matter)

@ct2034
Copy link
Collaborator

ct2034 commented Mar 28, 2024

Thanks for your contribution @limaanto :)
Please add also a test. But you should be able to adopt https://github.com/ros/diagnostics/blob/ros2/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor_launchtest.py directly.

@ct2034 ct2034 added ros2 PR tackling a ROS2 branch needs more work Someone has worked on this but more work is needed labels Mar 28, 2024
@limaanto
Copy link
Contributor Author

Hi, I rebased the branch on ros2 and implemented a successful launch test (based on the recommended ntp_monitor's)
Tell me if I'm missing something 😃

@ct2034 ct2034 self-requested a review May 13, 2024 13:09
@ct2034 ct2034 self-assigned this May 13, 2024
limaanto added a commit to limaanto/diagnostics that referenced this pull request May 28, 2024
@limaanto
Improve hd_monitor code quality
b9e0a43 
Remove useless print, parameter and format strings according to review of ros#334
limaanto added a commit to limaanto/diagnostics that referenced this pull request May 28, 2024
@limaanto
Improve hd_monitor code quality
a7e68ac 
Remove useless print, parameter and format strings according to review of ros#334
@limaanto
Copy link
Contributor Author

Hi, I included your review remarks, rebased on the latest ros2 and passed black.
Cheers :)

ct2034 added a commit that referenced this pull request Jun 27, 2024
@ct2034
Port hd_monitor to ROS2 #334
6195501 
Thanks to https://github.com/limaanto

Squashed commit of the following:

commit d21cd6d736a1050b185c070af01f10503a24f4df
Merge: 861f924 e67a69c
Author: Christian Henkel <christian.henkel2@de.bosch.com>
Date:   Thu Jun 27 10:06:10 2024 +0200

    Merge branch 'ros2' into port-hd-monitor

    Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

commit 861f924
Author: Antoine Lima <antoine@sagou.in>
Date:   Mon Jun 3 18:13:56 2024 +0200

    Fix hd_monitor test

commit b9ecd1e
Author: Antoine Lima <antoine@sagou.in>
Date:   Wed May 29 01:30:40 2024 +0200

    Pass black formatter on hd_monitor and associated test

commit a7e68ac
Author: Antoine Lima <antoine@sagou.in>
Date:   Wed May 29 01:28:40 2024 +0200

    Improve hd_monitor code quality

    Remove useless print, parameter and format strings according to review of #334

commit 8eba67b
Author: Antoine Lima <antoine@sagou.in>
Date:   Wed May 29 01:26:55 2024 +0200

    Implement low and crit parameters in hd_monitor

commit 6c60eda
Author: Antoine Lima <antoine@sagou.in>
Date:   Thu Mar 28 12:13:28 2024 +0100

    Add launch test for hd_monitor

commit b29cacf
Author: Antoine Lima <antoine@sagou.in>
Date:   Thu Mar 28 12:13:10 2024 +0100

    Fix execution flag of hd_monitor

commit 4cc315d
Author: Antoine Lima <antoine@sagou.in>
Date:   Fri Mar 15 17:46:08 2024 +0100

    Adapt documentation and CMakeList to new hd_monitor.py

commit 2a4eece
Author: Antoine Lima <antoine@sagou.in>
Date:   Fri Mar 15 17:34:13 2024 +0100

    Port hd_monitor.py

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
@ct2034 ct2034 mentioned this pull request Jun 27, 2024
Closed
@ct2034
Copy link
Collaborator

ct2034 commented Jun 27, 2024

@limaanto Please rebase branch and fix flake8 errors

@limaanto
Copy link
Contributor Author

That should be good 👍

@ct2034
Copy link
Collaborator

ct2034 commented Jul 2, 2024 •
edited
Loading

@limaanto friendly ping

@ct2034 ct2034 merged commit 05a9645 into ros:ros2 Jul 3, 2024
11 of 12 checks passed
ct2034 pushed a commit to ct2034/diagnostics that referenced this pull request Jul 3, 2024
@limaanto
Port hd_monitor to ROS2 (ros#334)
b835834 
* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)
@ct2034 ct2034 mentioned this pull request Jul 3, 2024
Merged
ct2034 pushed a commit to ct2034/diagnostics that referenced this pull request Jul 3, 2024
@limaanto
Port hd_monitor to ROS2 (ros#334)
57044db 
* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)
@ct2034 ct2034 mentioned this pull request Jul 3, 2024
Merged
@limaanto limaanto deleted the port-hd-monitor branch July 3, 2024 12:36
ct2034 pushed a commit to ct2034/diagnostics that referenced this pull request Jul 3, 2024
@limaanto
Port hd_monitor to ROS2 (ros#334)
af5657c 
* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)

# Conflicts:
#	diagnostic_common_diagnostics/CMakeLists.txt
#	diagnostic_common_diagnostics/mainpage.dox
@ct2034 ct2034 mentioned this pull request Jul 3, 2024
Merged
@ct2034
Copy link
Collaborator

ct2034 commented Jul 3, 2024

💚 All backports created successfully

Status Branch Result
✅ ros2-humble
✅ ros2-iron
✅ ros2-jazzy

Questions ?

Please refer to the Backport tool documentation

ct2034 added a commit that referenced this pull request Jul 17, 2024
@ct2034
fixing pep257 problems introduced by #334
1405877 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit that referenced this pull request Jul 17, 2024
@ct2034
fixing pep257 problems introduced by #334 (#384)
b504059 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit to ct2034/diagnostics that referenced this pull request Jul 17, 2024
@ct2034
fixing pep257 problems introduced by ros#334 (ros#384)
2b25e4b 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit to ct2034/diagnostics that referenced this pull request Jul 17, 2024
@ct2034
fixing pep257 problems introduced by ros#334 (ros#384)
e496d1a 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit to ct2034/diagnostics that referenced this pull request Jul 17, 2024
@ct2034
fixing pep257 problems introduced by ros#334 (ros#384)
3974f2a 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit that referenced this pull request Jul 17, 2024
@ct2034 @limaanto
[ros2-iron] Port hd_monitor to ROS2 (#334) (#382)
605b72b 
* Port hd_monitor to ROS2 (#334)

* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)

* fixing pep257 problems introduced by #334 (#384)

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Antoine Lima <7421319+limaanto@users.noreply.github.com>
ct2034 added a commit that referenced this pull request Jul 17, 2024
@ct2034 @limaanto
[ros2-humble] Port hd_monitor to ROS2 (#334) (#381)
fe98a48 
* Port hd_monitor to ROS2 (#334)

* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)

* fixing pep257 problems introduced by #334 (#384)

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Antoine Lima <7421319+limaanto@users.noreply.github.com>
ct2034 pushed a commit to ct2034/diagnostics that referenced this pull request Jul 17, 2024
@limaanto
Port hd_monitor to ROS2 (ros#334)
fca9fc5 
* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)

# Conflicts:
#	diagnostic_common_diagnostics/CMakeLists.txt
#	diagnostic_common_diagnostics/mainpage.dox
@ct2034
Copy link
Collaborator

ct2034 commented Jul 17, 2024

💚 All backports created successfully

Status Branch Result
✅ ros2-jazzy

Questions ?

Please refer to the Backport tool documentation

ct2034 added a commit that referenced this pull request Jul 17, 2024
@ct2034 @limaanto
[ros2-jazzy] Port hd_monitor to ROS2 (#334) (#383)
18c4290 
* Port hd_monitor to ROS2 (#334)

* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality

(cherry picked from commit 05a9645)

# Conflicts:
#	diagnostic_common_diagnostics/CMakeLists.txt
#	diagnostic_common_diagnostics/mainpage.dox

* adding missing launchtest depdendency

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

* no changes to ros2 branch

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Antoine Lima <7421319+limaanto@users.noreply.github.com>
@ct2034 ct2034 removed the needs more work Someone has worked on this but more work is needed label Jul 22, 2024
ct2034 added a commit to ct2034/diagnostics that referenced this pull request Jul 22, 2024
@ct2034
fixing pep257 problems introduced by ros#334 (ros#384)
6b8f6a7 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
(cherry picked from commit b504059)
ct2034 added a commit that referenced this pull request Jul 22, 2024
@ct2034
fixing pep257 problems introduced by #334 (#384) (#392)
f92c157 
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
(cherry picked from commit b504059)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ct2034 ct2034 ct2034 approved these changes

Assignees

@ct2034 ct2034

Labels
ros2 PR tackling a ROS2 branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@limaanto @ct2034