Add scripted return status support for TestNode mocks#1125
Add scripted return status support for TestNode mocks#1125pepeRossRobotics wants to merge 5 commits into
Conversation
…t-scripts Introduces `return_status_script` to allow dynamic computation of the TestNode's completion status based on blackboard variables at runtime. This provides greater flexibility for mocking different scenarios in tests. Refines post-execution logic by replacing a single `post_script` with distinct `success_script` and `failure_script` that execute based on the TestNode's final status. The `return_status_script` takes precedence over a statically defined `return_status`. Adds comprehensive validation to prevent the TestNode from returning an `IDLE` status and to ensure a valid completion status mechanism is always configured.
Adjusts line wrapping and whitespace across examples, headers, source files, and tests to enhance readability and maintain a unified code style.
|
Thanks for this contribution — the feature itself is welcome and addresses #995 well, the test coverage is good, and I verified locally that everything builds and passes. However, there are a few things that need to be addressed before this can be merged: 1. We can not accept API changes: Changing The optional isn't actually needed for the feature: since NodeStatus return_status = NodeStatus::SUCCESS;unchanged. Concretely:
That makes the whole PR purely additive. 2. Misleading error message for out-of-range script results.
3. Unrelated change in The 4. Please leave the existing tutorial Tutorial 15 corresponds to a published page on behaviortree.dev; restructuring its tree (the added |
…an up unrelated changes - Revert TestNodeConfig::return_status from std::optional<NodeStatus> back to plain NodeStatus to preserve source compatibility with downstream code - Use a local has_return_status flag in loadSubstitutionRuleFromJSON instead of .reset() to validate JSON configs without breaking the field type - Fix misleading error message in validateTestNodeStatus: now reports the actual integer value instead of hardcoding "IDLE" - Restore t15_nodes_mocking.cpp to its original form (no Fallback, no mock_should_fail flag); return_status_script feature is demonstrated only in t15_nodes_mocking_strict_failure.cpp - Revert unrelated gtest_wakeup.cpp changes (steady_clock → system_clock, ASSERT_LT bound back to 25 ms) The return_status_script feature itself is unchanged.
|
Comments addressed, also opened #1164 |
This adds support for computing a TestNode return status from a script, so mock behavior can depend on blackboard state without adding extra nodes to the tree.
It also updates the substitution tests and tutorial examples to cover both the observable and strict-failure mock flows.
Addresses #995