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

Backport/wait for server timeout #4261

Merged

Conversation

bektaskemal
Copy link
Contributor

Backports #3960 and #4178 to iron.

We would like to have this on iron if possible.

Copy link
Contributor

mergify bot commented Apr 12, 2024

@bektaskemal, all pull requests must be targeted towards the main development branch.
Once merged into main, it is possible to backport to @iron, but it must be in main
to have these changes reflected into new distributions.

embeddedadam and others added 2 commits April 12, 2024 18:22
* Make BT nodes have configurable wait times.

Previous solution provided hardcoded 1s value.
Right now the value can be configured for BT
Action, Cancel and Service nodes.

[ros-navigation#3920]

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>

* Make BT nodes have configurable wait times.

Previous solution provided hardcoded 1s value.
Right now the value can be configured for BT
Action, Cancel and Service nodes.

[ros-navigation#3920]

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>

* Fix typos, linting errors and value type from float to int

* Fix extra underscores

* Fix extra underscore

* Update unit tests with blackboard parameter

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>

* Fix formatting errors

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>

* Update system tests to match new parameter

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>

---------

Signed-off-by: Adam Galecki <embeddedadam@gmail.com>
Signed-off-by: Kemal Bektas <kemal.bektas@node-robotics.com>
…os-navigation#4178)

Signed-off-by: Felix <felix@fzeltner.de>
Co-authored-by: Felix <felix@fzeltner.de>
Signed-off-by: Kemal Bektas <kemal.bektas@node-robotics.com>
@bektaskemal bektaskemal force-pushed the backport/wait_for_server_timeout branch from 335534e to c0d2787 Compare April 12, 2024 16:23
@SteveMacenski
Copy link
Member

This seems fine to me. Please run the full colcon test on Nav2 (including system tests) and make sure everything comes out passing. Iron's CI isn't setup correctly for this. If so, then I can merge this.

@bektaskemal
Copy link
Contributor Author

I tested locally and two test failed for me but they seems irrelevant:

navigation2/nav2_behavior_tree/test/plugins/decorator/test_single_trigger_node.cpp:75: Failure
Expected equality of these values:
  bt_node_->executeTick()
    Which is: FAILURE
  BT::NodeStatus::RUNNING
    Which is: RUNNING
/home/kemalbektas/navigation2/nav2_behavior_tree/test/plugins/decorator/test_single_trigger_node.cpp:77: Failure
Expected equality of these values:
  bt_node_->executeTick()
    Which is: FAILURE
  BT::NodeStatus::SUCCESS
    Which is: SUCCESS

This one might be related to behaviortree_cpp_v3 version but I'm not sure.

[tester_node-17] [INFO] [1713261931.235650522] [nav2_gps_waypoint_tester]: Goal failed to process all waypoints, missed 1 wps.
23: [tester_node-17] Traceback (most recent call last):
23: [tester_node-17]   File "/home/kemalbektas/navigation2/nav2_system_tests/src/gps_navigation/tester.py", line 222, in <module>
23: [tester_node-17]     main()
23: [tester_node-17]   File "/home/kemalbektas/navigation2/nav2_system_tests/src/gps_navigation/tester.py", line 187, in main
23: [tester_node-17]     ComputePathToPose.Result().GOAL_OUTSIDE_MAP
23: [tester_node-17] AttributeError: 'ComputePathToPose_Result' object has no attribute 'GOAL_OUTSIDE_MAP'

Here is the issue that error codes are in Goal and not Result, but this is wrong in iron branch: https://github.com/ros-planning/navigation2/blob/iron/nav2_system_tests/src/gps_navigation/tester.py#L187

@SteveMacenski SteveMacenski merged commit 33d605c into ros-navigation:iron Apr 21, 2024
2 of 4 checks passed
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