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

Reactive async #93

Open
wants to merge 19 commits into
base: master
from
Open

Reactive async #93

wants to merge 19 commits into from

Conversation

@miccol
Copy link
Member

@miccol miccol commented Apr 30, 2019

Hello,

this is an attempt to solve Issue #83.

The main important change is the implementation of the function propagateHalt that lets a node to trigger the haltChildren routine of its parent.

@facontidavide The idea behind this is, before ticking an async child, the BT should halt the other running children (if any). Let me know what do you think about this.

There are also gtests to test that tests that no race condition occurs.

ASSERT_TRUE(action_1.startTimePoint().time_since_epoch().count() >
action_2.stopTimePoint().time_since_epoch().count() );

Thank you
MC

@facontidavide
Copy link
Collaborator

@facontidavide facontidavide commented May 1, 2019

i will take my time and look at this carefully. Thanks!!!

@hlzl
Copy link

@hlzl hlzl commented May 2, 2019

Nice to see someone working on this, thanks! I'm also going to try to have a look next week.

This caught my attention:

@facontidavide The idea behind this is, before ticking an async child, the BT should halt the other running children (if any). Let me know what do you think about this.

Have you thought about letting the first async node finish execution when ticking the second async node instead of halting the first async node right away? This would mean both would run at the same time for a certain period. That's how I would imagine a reactive sequence to behave and it would allow for some nice use cases.

@miccol
Copy link
Member Author

@miccol miccol commented May 3, 2019

Nice to see someone working on this, thanks! I'm also going to try to have a look next week.

This caught my attention:

@facontidavide The idea behind this is, before ticking an async child, the BT should halt the other running children (if any). Let me know what do you think about this.

Have you thought about letting the first async node finish execution when ticking the second async node instead of halting the first async node right away? This would mean both would run at the same time for a certain period. That's how I would imagine a reactive sequence to behave and it would allow for some nice use cases.

That is possible already (but be careful in doing so). When sending the halt, it calls the halt() function, however the execution of the tick() is not stopped if not handled explicitly. To handle the interruption of the tick() execution you can do something like this:

In the halt() you set a variable

void MoveBaseAction::halt()
{
_halt_requested.store(true);
}

and you check it in the tick()

while (!_halt_requested && count++ < 25)
{
SleepMS(10);
}

Anyway, If you want to discuss more about this, I think it is better if we move the conversation in another place (i.e. not in a PR).

Cheers
MC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.