Nav2_behavior_tree tests input_port doesn't get set

[redvinaa]

Bug report

Required Info:

• Operating System:
  • Ubuntu 24.04 (jazzy docker)
• Computer:
• ROS2 Version:
  • jazzy docker
• Version or commit hash:
  • a782e7a (current main HEAD)
• DDS implementation:

Steps to reproduce issue

1. Set the input port value here to something else than the default value (for example 2).
2. Add EXPECT_TRUE(false) to the end of any test, so prints are shown.
3. Print input port value in recovery_node tick function (to stderr so gtest doesn't hide it)

std::cerr << "RecoveryNode: number_of_retries = " << number_of_retries_ << std::endl;

4. Run tests.

Expected behavior

Prints should say number_of_recoveries = 2 (or whatever random value you set).

Actual behavior

Prints say number_of_recoveries = 1 (because the default value is 1).

Additional information

In the background the value does get set, but to string type. So the default value of type int really doesn't get overwritten.
This is because the input_ports member is actually of type

using PortsRemapping = std::unordered_map<std::string, std::string>;

[SteveMacenski]

@redvinaa what is your suggested update? It seems from this ticket that this only is an issue for the unit tests, not functional code correct? Are there any tests today that use the retries feature?

[redvinaa]

I think it's only used in tests, so it doesn't affect functional code. It was initially blocking #5247, but then I went with another solution, using blackboard variables. It's just luck that it runs without error in the existing tests (because it's set to the default value).

It would probably take more time for me to find the solution than what I currently have. Though I think someone who is more trained in BTCPP than I am can tell very quickly what the solution could be.

[SteveMacenski]

OK - in this case, I'm going to close as a wontfix since its not blocking anymore and its only used in test fixtures. Not saying its suboptimal, but we don't rely on that behavior and its not run-time code so I lean towards just bypassing it since we don't use it now nor plan to rely on it in the foreseeable future.