Skip to content
This repository has been archived by the owner on Jun 10, 2021. It is now read-only.

Added some doc to system_metric_collector #75

Merged

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jan 17, 2020

Added some doc to system_metric_collector

There are two parameters defined:

- **measurement_period**:
```sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using yaml for better readability

Copy link
Contributor

@piraka9011 piraka9011 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Have some comments for better docs.


## Usage:

You can run the following command to generate CPU and Memory statistics. for both, all the system and the node running:
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the second sentence, you only need the first

ros2 run system_metrics_collector main
```

Change `publish_period` or `measurement_period` using `--ros-args`:
Copy link
Contributor

Choose a reason for hiding this comment

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

publish_period or measurement_period can be modified using...

Copy link
Contributor

@dabonnie dabonnie left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I forsee this documentation drastically changing as we add the preferred mechanisms to run the collection processes. Specifically once we have the composable and lifecycle PRs merged, the current main will be used as an example or simply removed entirely.

You can run the following command to generate CPU and Memory statistics. for both, all the system and the node running:

```
ros2 run system_metrics_collector main
Copy link
Contributor

Choose a reason for hiding this comment

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

This usage isn't quite correct. At some point in the next 2 weeks, we will be moving this main to an example directory. The preferred way to launch the system CPU and system memory collector will be to use their entry points (standalone mains) or the launch file. Per process CPU and memory will use composition.

Copy link
Member

Choose a reason for hiding this comment

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

@dabonnie Is this the correct usage now? If yes, I think we can merge as it is and the refactoring PR should update the doc accordingly at this time?

Copy link
Contributor

@dabonnie dabonnie Jan 17, 2020

Choose a reason for hiding this comment

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

@thomas-moulard sounds good, why I didn't submit as a blocking review ;-)

@dabonnie dabonnie merged commit 17d2a36 into ros-tooling:master Jan 17, 2020
@dabonnie
Copy link
Contributor

Merging as build passes but codecov is broken.

@ahcorde ahcorde deleted the ahcorde/system_metric_collector_doc branch January 20, 2020 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants