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(external_cmd_converter): replace polling takeData function with the callback function #7263

Conversation

N-Eiki
Copy link
Contributor

@N-Eiki N-Eiki commented Jun 4, 2024

Description

In this PR, instead of getting msg in callback, it is changed to get msg using takeData function of polling_subscriber in utils.

Related links

Tests performed

the module works as before

before

before.mp4

after

after.mp4

Notes for reviewers

Interface changes

none

ROS Topic Changes

ROS Parameter Changes

Effects on system behavior

none

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.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

N-Eiki added 3 commits June 4, 2024 15:34
@N-Eiki N-Eiki requested a review from TakaHoribe as a code owner June 4, 2024 06:59
@github-actions github-actions bot added the component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) label Jun 4, 2024
@N-Eiki N-Eiki marked this pull request as draft June 4, 2024 07:01
@N-Eiki N-Eiki marked this pull request as ready for review June 4, 2024 07:23
@N-Eiki N-Eiki marked this pull request as draft June 4, 2024 10:22
@N-Eiki N-Eiki marked this pull request as ready for review June 5, 2024 07:46
@shmpwk shmpwk requested review from zusizusi and shtokuda June 5, 2024 09:14
@shmpwk
Copy link
Contributor

shmpwk commented Jun 5, 2024

@N-Eiki
Thank you! Could you use the new msg?
https://github.com/orgs/autowarefoundation/discussions/3862
There is a error.
https://github.com/autowarefoundation/autoware.universe/actions/runs/9379885736/job/25830740791?pr=7263#step:8:8862

[ 75%] Built target external_cmd_converter_node
In file included from /__w/autoware.universe/autoware.universe/vehicle/external_cmd_converter/src/node.cpp:15:
/__w/autoware.universe/autoware.universe/vehicle/external_cmd_converter/include/external_cmd_converter/node.hpp:43:31: error: ‘autoware_auto_control_msgs’ does not name a type
   43 | using ControlCommandStamped = autoware_auto_control_msgs::msg::AckermannControlCommand;
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~
gmake[2]: *** [CMakeFiles/external_cmd_converter.dir/build.make:76: CMakeFiles/external_cmd_converter.dir/src/node.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:139: CMakeFiles/external_cmd_converter.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
---

Summary: 17 packages finished [6min 45s]
  1 package failed: external_cmd_converter
  3 packages had stderr output: autoware_cmake autoware_lint_common external_cmd_converter
Error: Process completed with exit code 2.

@N-Eiki N-Eiki marked this pull request as draft June 5, 2024 10:53
N-Eiki and others added 7 commits June 5, 2024 20:07
… to snake_case, delete redundant if sentences

Signed-off-by: N-Eiki <[email protected]>
Signed-off-by: N-Eiki <[email protected]>
Signed-off-by: N-Eiki <[email protected]>
…com:N-Eiki/autoware.universe into feat/use_takeData_in_external_cmd_converter
@N-Eiki
Copy link
Contributor Author

N-Eiki commented Jun 6, 2024

cppcheck-all / cppcheck-all (pull_request) is just for monitoring, so this failure is no problem.
build-and-test-differential / clang-tidy-differential (pull_request) This failure is not caused by this package, so this failure is no problem.

@N-Eiki N-Eiki marked this pull request as ready for review June 6, 2024 08:29
@N-Eiki N-Eiki requested a review from shmpwk June 6, 2024 08:29
@N-Eiki N-Eiki requested a review from shmpwk June 6, 2024 10:50
@shmpwk shmpwk added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jun 7, 2024
Signed-off-by: N-Eiki <[email protected]>
@N-Eiki N-Eiki requested a review from shmpwk June 10, 2024 02:23
Copy link
Contributor

@shmpwk shmpwk left a comment

Choose a reason for hiding this comment

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

LGTM

@shmpwk shmpwk merged commit e4bda73 into autowarefoundation:main Jun 10, 2024
23 of 24 checks passed
KhalilSelyan pushed a commit that referenced this pull request Jul 22, 2024
…the callback function (#7263)

* delete duplicated sentence. \n replace callback data get function with takeData function.

Signed-off-by: N-Eiki <[email protected]>

* add maintainer

Signed-off-by: N-Eiki <[email protected]>

* delete  name

Signed-off-by: N-Eiki <[email protected]>

* style(pre-commit): autofix

* style(pre-commit): autofix

* handle to comment: delete waste line

Signed-off-by: N-Eiki <[email protected]>

* fix: treat clang-tidy-differential's warnings. for examples CamelCase to snake_case, delete redundant if sentences

Signed-off-by: N-Eiki <[email protected]>

* fix mailadress miss

Signed-off-by: N-Eiki <[email protected]>

* style(pre-commit): autofix

* format by clang-format

Signed-off-by: N-Eiki <[email protected]>

* delete static

Signed-off-by: N-Eiki <[email protected]>

* unify to *_sub_

Signed-off-by: N-Eiki <[email protected]>

---------

Signed-off-by: N-Eiki <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:vehicle Vehicle-specific implementations, drivers, packages. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants