-
Notifications
You must be signed in to change notification settings - Fork 659
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(tier4_autoware_utils): add published time debug class into utils #6440
feat(tier4_autoware_utils): add published time debug class into utils #6440
Conversation
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
f9f6613
to
4f24256
Compare
c84d1b0
to
19ddcab
Compare
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
19ddcab
to
fde0fae
Compare
Could you move Reasons:
Example: private:
rclcpp::Node * node_;
std::string end_name_;
rclcpp::QoS qos_;
struct GidCompare
{
bool operator()(const rmw_gid_t & lhs, const rmw_gid_t & rhs) const
{
return std::memcmp(lhs.data, rhs.data, RMW_GID_STORAGE_SIZE) < 0;
}
};
std::map<rmw_gid_t, rclcpp::Publisher<PublishedTime>::SharedPtr, GidCompare> publishers_;
void ensure_publisher_exists(const rmw_gid_t & gid_key, const std::string & topic_name); |
Placing the Moving the private:
rclcpp::Node * node_;
std::string end_name_;
rclcpp::QoS qos_;
using PublishedTime = autoware_internal_msgs::msg::PublishedTime; |
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
common/tier4_autoware_utils/include/tier4_autoware_utils/ros/published_time_publisher.hpp
Outdated
Show resolved
Hide resolved
a5da243
to
550b9b0
Compare
@xmfcx Thank you for the detailed review. 🙏🏼 |
550b9b0
to
7626af7
Compare
Signed-off-by: Berkay Karaman <[email protected]>
Signed-off-by: Berkay Karaman <[email protected]>
Signed-off-by: Berkay Karaman <[email protected]>
7626af7
to
b42b772
Compare
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.
thanks!
Signed-off-by: M. Fatih Cırıt <[email protected]>
a1a595c should fix it. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6440 +/- ##
==========================================
- Coverage 14.79% 14.78% -0.02%
==========================================
Files 1915 1915
Lines 132352 132547 +195
Branches 39333 39496 +163
==========================================
+ Hits 19580 19593 +13
- Misses 90939 91086 +147
- Partials 21833 21868 +35
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@soblin @TakaHoribe @takayuki5168 Hi, we need a code owner approvement. Could you check the PR? |
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.
@brkay54
Thank you for the contribution. Can you create a unit test for the feature? Generally, we create a test for functions in the tier4_autoware_utils package.
@brkay54 could you open up a new PR for the tests of the code added here? I will merge it, it is blocking some other PRs. |
…autowarefoundation#6440) Signed-off-by: Berkay Karaman <[email protected]> Signed-off-by: kaigohirao <[email protected]>
…autowarefoundation#6440) Signed-off-by: Berkay Karaman <[email protected]>
Description
Part of:
Required for:
Depends on:
This interface would be enabled only if the use_published_time = true.
Tests performed
Tested on PSim and e2e_simulator, worked well.
Effects on system behavior
Not applicable.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.