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.

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

feat: system diagnostic monitor message #96

Merged
merged 16 commits into from
Sep 15, 2023

Conversation

isamu-takagi
Copy link
Contributor

@isamu-takagi isamu-takagi commented Aug 16, 2023

Related Links

autowarefoundation/autoware.universe#3829 (comment)

Description

Add diagnostic graph message.

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Code is properly formatted
  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • Code is properly formatted
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets
  • Write release notes

CI Checks

  • Build and test for PR: Required to pass before the merge.
  • Check spelling: NOT required to pass before the merge. It is up to the reviewer(s). See here if you want to add some words to the spell check dictionary.

@isamu-takagi isamu-takagi self-assigned this Aug 16, 2023
@isamu-takagi isamu-takagi changed the title Feat/system diagnostic graph feat: system diagnostic monitor message Aug 16, 2023
Copy link

@asana17 asana17 left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment on lines 3 to 9
byte stop
byte autonomous
byte local
byte remote
byte emergency_stop
byte comfortable_stop
byte pull_over
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't these be boolean values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed 62c3ddd

Comment on lines 1 to 2
uint32 index
bool used
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might want to have a README for this message to explain what these fields mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add comment ddcc615

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is specifically used to express each operation mode, then it might be better to change the name to something like "OperationModeDiagnositics", "OperationModeAvailability", etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed 62c3ddd

Signed-off-by: Takagi, Isamu <[email protected]>
Signed-off-by: Takagi, Isamu <[email protected]>
@isamu-takagi isamu-takagi enabled auto-merge (squash) September 12, 2023 07:00
Copy link
Collaborator

@yukkysaito yukkysaito left a comment

Choose a reason for hiding this comment

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

LGTM
@mitsudome-r Can you review this for double-check?

@isamu-takagi isamu-takagi merged commit 71cc77d into tier4/universe Sep 15, 2023
10 checks passed
@isamu-takagi isamu-takagi deleted the feat/system-diagnostic-graph branch September 15, 2023 10:33
@isamu-takagi isamu-takagi restored the feat/system-diagnostic-graph branch September 15, 2023 12:23
@isamu-takagi isamu-takagi deleted the feat/system-diagnostic-graph branch September 15, 2023 12:24
@mitsudome-r
Copy link
Collaborator

mitsudome-r commented Sep 18, 2023

Just as a record, LGTM

isamu-takagi added a commit that referenced this pull request Nov 30, 2023
* feat: add diagnostic graph

Signed-off-by: Takagi, Isamu <[email protected]>

* feat: update diagnostic graph

Signed-off-by: Takagi, Isamu <[email protected]>

* feat: add links

Signed-off-by: Takagi, Isamu <[email protected]>

* feat: split status and struct

Signed-off-by: Takagi, Isamu <[email protected]>

* add name

Signed-off-by: Takagi, Isamu <[email protected]>

* add summary

Signed-off-by: Takagi, Isamu <[email protected]>

* add link

Signed-off-by: Takagi, Isamu <[email protected]>

* add stamp

Signed-off-by: Takagi, Isamu <[email protected]>

* add stamp

Signed-off-by: Takagi, Isamu <[email protected]>

* rename message

Signed-off-by: Takagi, Isamu <[email protected]>

* add comment

Signed-off-by: Takagi, Isamu <[email protected]>

---------

Signed-off-by: Takagi, Isamu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants