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

Add the RunOnce decorator node #472

Closed
wants to merge 2 commits into from
Closed

Conversation

galou
Copy link
Contributor

@galou galou commented Nov 14, 2022

This adds a RunOnce decorator node that will call its child' executeTick() only once. After the first time, it'll return the return value of child's status().

Signed-off-by: Gaël Écorchard [email protected]

@facontidavide
Copy link
Collaborator

It is ok for me to merge this

Signed-off-by: Gaël Écorchard <[email protected]>
Co-authored-by: Davide Faconti <[email protected]>
@galou
Copy link
Contributor Author

galou commented Nov 25, 2022

I changed according to your wish but the functionality was not affected.

@facontidavide
Copy link
Collaborator

I am still thinking about what the desired behavior is for Asynchronous children.

May be this make more sense?

auto status = child_node_->executeTick();
if(status == NosteStatus::SUCCESS || status == NosteStatus::FAILURE) {
   already_ticked_ = true;
}
return status;

Otherwise we don't complete RUNNING children

@galou
Copy link
Contributor Author

galou commented Nov 25, 2022

I thought that the async child would change it's status without the need to be ticked. Am I right?

@facontidavide
Copy link
Collaborator

facontidavide commented Nov 25, 2022

no, not necessarily. This is much safer, in my opinion

facontidavide added a commit that referenced this pull request Mar 30, 2023
@facontidavide
Copy link
Collaborator

Merged with some important improvements

3efde1e

@galou
Copy link
Contributor Author

galou commented Mar 30, 2023

Nice, sorry for not having reacted on this.

@galou galou deleted the run_once_node branch March 30, 2023 11:53
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.

2 participants