fix(system_monitor): change communication to unix domain socket

[ito-san]

Signed-off-by: ito-san [email protected]

Description

There is a bug that the ROS node hdd_monitor communicates with the daemon process hdd_reader running on another ECU instead of that running on local computer when system_monitor runs on multiple ECUs at the same time.
The hdd_monitor and hdd_reader uses INADDR_ANY for socket programming, and the socket was bound to all network interfaces instead of localhost only.

The ROS node and daemon process is designed to communicate within localhost only, so I made a change to use an UNIX domain socket for local socket communication.

Changes

1. Use an UNIX domain socket(/tmp/hdd_reader) instead of INADDR_ANY.
2. Code refactoring to clean up complex codes.

Related links

TIER IV INTERNAL LINK TO SLACK

Tests performed

Verify all diagnostic topics regarding hdd_monitor are reported correctly.

1. Run Autoware.

2. Run rqt_runtime_monitor.

   ros2 run rqt_runtime_monitor rqt_runtime_monitor

   1. HDD Connection
      

   2. HDD Usage
      

   3. HDD Temperature
      

   4. HDD PowerOnHours
      

   5. HDD TotalDataWritten
      

   6. HDD Recovered Error
      

   7. HDD ReadDataRate
      

   8. HDD ReadIOPS
      

   9. HDD WriteDataRate
      

   10. HDD WriteIOPS
       

Verify no warning and error messages in a normal run.

Notes for reviewers

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

• I've confirmed the contribution guidelines.
• The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• The PR follows the pull request guidelines.
• The PR has been properly tested.
• The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

• There are no open discussions or they are tracked via tickets.
• The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 780 lines in your changes missing coverage. Please review.

Project coverage is 11.38%. Comparing base (071668b) to head (d547315).
Report is 5538 commits behind head on main.

Files with missing lines	Patch %	Lines
...tem/system_monitor/src/hdd_monitor/hdd_monitor.cpp	0.00%	526 Missing ⚠️
...m_monitor/reader/hdd_reader/hdd_reader_service.cpp	0.00%	85 Missing ⚠️
system/system_monitor/include/hdd_reader/ata.hpp	0.00%	75 Missing ⚠️
system/system_monitor/include/hdd_reader/nvme.hpp	0.00%	44 Missing ⚠️
...stem_monitor/reader/hdd_reader/hdd_reader_main.cpp	0.00%	22 Missing ⚠️
...m_monitor/include/hdd_reader/hdd_reader_common.hpp	0.00%	19 Missing ⚠️
...include/system_monitor/hdd_monitor/hdd_monitor.hpp	0.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2608      +/-   ##
==========================================
- Coverage   11.94%   11.38%   -0.57%     
==========================================
  Files        1321     1276      -45     
  Lines       91859    88286    -3573     
  Branches    24467    23077    -1390     
==========================================
- Hits        10975    10053     -922     
+ Misses      69528    67570    -1958     
+ Partials    11356    10663     -693     

Flag	Coverage Δ		*Carryforward flag
differential	0.00% <0.00%> (?)
total	11.44% <0.00%> (-0.50%)	⬇️	Carriedforward from 45d203e

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ito-san]

@asana17 Sorry but many changes were made. Verified hdd_monitor works correctly.

[asana17]

The hdd_monitor functions are LGTM.
From the parameter file hdd_monitor.param.yaml, can't multiple local HDDs or disks other than "disk0" currently be monitored?

[asana17]

The hdd_monitor functions are LGTM. From the parameter file hdd_monitor.param.yaml, can't multiple local HDDs or disks other than "disk0" currently be monitored?

This can be solved in future PR. so I remain it.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity.

[xmfcx]

@ito-san is this PR still relevant? Could you rebase this if it's necessary?
If not could you close it?

[xmfcx]

@ito-san I will close this issue since there was no response. Feel free to reopen if necessary.