-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add safety_limiter_msgs package #16
Conversation
[#39] PASSED on kineticAll tests passed
[#39] PASSED on melodicAll tests passed
|
@at-wat PTAL |
[#40] PASSED on kineticAll tests passed
[#40] PASSED on melodicAll tests passed
|
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.
How about adding some values which are already sent to diagnostics topic?
https://github.com/at-wat/neonavigation/blob/970c3fab11c07aec76714809a1b3953a1ea40d4c/safety_limiter/src/safety_limiter.cpp#L671-L695
[#41] PASSED on kineticAll tests passed
[#41] PASSED on melodicAll tests passed
|
@DaikiMaekawa basically looks good to me. Let's merge it after implementing a publisher in safety_limiter. |
[#43] PASSED on melodicAll tests passed
[#43] PASSED on kineticAll tests passed
|
float64 limit_ratio | ||
bool is_cloud_available | ||
bool has_watchdog_timed_out | ||
time stuck_started_since |
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.
meaning of zero value (not being stuck) should be described as a comment.
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.
added
[#44] PASSED on kineticAll tests passed
[#44] PASSED on melodicAll tests passed
|
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.
A dependency from the meta-package should be added.
neonavigation_msgs/neonavigation_msgs/package.xml
Lines 13 to 16 in 17cd056
<exec_depend>planner_cspace_msgs</exec_depend> | |
<exec_depend>costmap_cspace_msgs</exec_depend> | |
<exec_depend>map_organizer_msgs</exec_depend> | |
<exec_depend>trajectory_tracker_msgs</exec_depend> |
safety_limiter_msgs/package.xml
Outdated
<?xml version="1.0"?> | ||
<package format="2"> | ||
<name>safety_limiter_msgs</name> | ||
<version>0.0.0</version> |
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.
should be 0.3.1
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.
done
[#45] PASSED on kineticAll tests passed
[#45] PASSED on melodicAll tests passed
|
@DaikiMaekawa dependency from |
[#46] PASSED on kineticAll tests passed
[#46] PASSED on melodicAll tests passed
|
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
I'll update PR title and merge this soon.
merging |
No description provided.