Skip to content

ReactiveSequence _skipIf issue fix #526

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

Closed

Conversation

ephson24
Copy link

Hi Davide,

I fixed the _skipIf issue described in this issue and created two tests: SkippingLogic.RepeatedSkippingReactiveSequence and SkippingLogic.RepeatedNoSkippingReactiveSequence to verify.

PS: The issue is only observable when ticking the tree continuously. E.g Calling tickOnce() inside a while loop. I hope this helps.

…ed two tests: SkippingLogic.RepeatedSkippingReactiveSequence and SkippingLogic.RepeatedNoSkippingReactiveSequence
@facontidavide
Copy link
Collaborator

I will look into this, thanks

@facontidavide
Copy link
Collaborator

problem found and fixed. Thanks for taking the time to report this!

facontidavide added a commit that referenced this pull request Mar 14, 2023
@ephson24
Copy link
Author

ephson24 commented Mar 14, 2023

problem found and fixed. Thanks for taking the time to report this!

One more thing I just realized. The issue persists if you have for example an async node as the last child of the ReactiveSequence. I haven't been able to look into it yet but I think it happens when the child returns RUNNING as I am doing in my Continue node.

<BehaviorTree ID="PowerManagerT">
    <ReactiveSequence>
      <Script code=" LOW_BATT:=20.0; isLowBattery:=false "/>
      <CheckLevel deviceType="BATT"
                  percentage="{LOW_BATT}"
                  result="{isLowBattery}"/>
      <SendWarning deviceType="BATT" deviceStatus="{isLowBattery}" _skipIf="!isLowBattery"/>
      <Continue />
    </ReactiveSequence>
  </BehaviorTree>

PS: Also run within a while loop

@facontidavide
Copy link
Collaborator

facontidavide commented Mar 14, 2023

Trying to understand this...

  1. Does Continue finally return SUCCESS, after a while?
  2. What is exactly the behavior you get and what do you expect to happen instead?

In this logic, EVERY time that Continue returns RUNNING, you should execute Script, CheckLevel and SendWarning (if not skipped).

You can not count the number of tickOnce, because it is unclear how many times Continue will need to be ticked.

Also, keep in mind that ReactiveSequence should have only one async child

@ephson24
Copy link
Author

ephson24 commented Mar 14, 2023

Trying to understand this...

  1. Does Continue finally return SUCCESS, after a while?
  2. What is exactly the behavior you get and what do you expect to happen instead?

In this logic, EVERY time that Continue returns RUNNING, you should execute Script, CheckLevel and SendWarning (if not skipped).

You can not count the number of tickOnce, because it is unclear how many times Continue will need to be ticked.

Also, keep in mind that ReactiveSequence should have only one async child

  1. Continue never returns SUCCESS.
  2. The idea is to keep this branch/tree running forever in parallel with the main robot behavior

Yes, Continue is the only Async node in this tree.

In this logic, EVERY time that Continue returns RUNNING, you should execute Script, CheckLevel and SendWarning (if not skipped).

My point exactly: But what happens is, SendWarning (even though skipped because the boolean is false) still runs.

@facontidavide facontidavide reopened this Mar 14, 2023
@ephson24
Copy link
Author

problem found, I think, it is a bug in SimpleActionNode.

I will

Amazing 🥇 )!! Thanks for your quick response and reaction with regards to this issue.
Is the merge with master gonna happen anytime soon or its best I develop with the 4.1 branch for now?

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.

3 participants