-
Notifications
You must be signed in to change notification settings - Fork 31
feat(autoware_adapi_v1_msgs): update diag messages #92
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(autoware_adapi_v1_msgs): update diag messages #92
Conversation
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Signed-off-by: Takagi, Isamu <isamu.takagi@tier4.jp>
autoware_adapi_v1_msgs/DiagLeafStruct[] diags | ||
autoware_adapi_v1_msgs/DiagLinkStruct[] links |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better for backward compatibility?
autoware_adapi_v1_msgs/DiagLeafStruct[] diags | |
autoware_adapi_v1_msgs/DiagLinkStruct[] links | |
autoware_adapi_v1_msgs/DiagLinkStruct[] links | |
autoware_adapi_v1_msgs/DiagLeafStruct[] diags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the code perspective there is no difference. From the binary perspective there are no guarantees and there should be no guarantees. For example, if a DiagNodeStruct is changed, compatibility will be lost even if the order is kept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was it added second to maintain alignment with DiagGraphStatus
? If there's no guarantee, I'd prefer keeping it at the end to make its addition more obvious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To match the order of fields in DiagGraphStruct and DiagGraphStatus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
======================================
Coverage 0.00% 0
======================================
Files 501 0 -501
Lines 27337 0 -27337
======================================
+ Misses 27337 0 -27337
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Update diagnostics API messages to support latch feature.
Related to autowarefoundation/autoware-documentation#674
How was this PR tested?
Check build passes.
Notes for reviewers
None.
Effects on system behavior
None.