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 behaviour trees to gazoo example #213

Merged
merged 3 commits into from
Aug 12, 2019

Conversation

JShep1
Copy link

@JShep1 JShep1 commented Jul 31, 2019

No description provided.

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like CI failed because it couldn't find any delphyne.decorators module.

delphyne-demos/demos/gazoo.py Outdated Show resolved Hide resolved
delphyne-demos/demos/gazoo.py Show resolved Hide resolved
delphyne-demos/demos/gazoo.py Outdated Show resolved Hide resolved
@JShep1 JShep1 requested a review from hidmic August 1, 2019 17:23
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, delphyne-gazoo 's smoke test is timing out @JShep1

@JShep1
Copy link
Author

JShep1 commented Aug 7, 2019

Could it be the addition of behaviour trees slows down simulation? I did see a noticeable slowdown after py-trees implementation was added.

@hidmic
Copy link
Contributor

hidmic commented Aug 7, 2019

Can you reproduce locally?

@JShep1
Copy link
Author

JShep1 commented Aug 7, 2019

Yes, it seems to run indefinitely locally

@JShep1
Copy link
Author

JShep1 commented Aug 8, 2019

It seems like this is the test when a finite amount of time is passed in for the simulation to run for. The error comes when the simulation stops (py-trees stops ticking after the desired time has passed) but the program does not exit.

I'm trying to find out if this is a general issue or one specifically for the gazoo example so I tried running this PR that passed CI and it ended up having the same issue (perhaps CI doesn't check for this case on the other examples... or maybe it's me).

Can someone try running my gazoo example and Brian's previously mentioned PR scriptlets example for a finite time with command delphyne-[gazoo/scriptlets] -b -d 2 and seeing if the example exits successfully?

@JShep1
Copy link
Author

JShep1 commented Aug 8, 2019

If this issue is consistent across all examples, perhaps a solution could be a post tick handler which holds a global counter that checks if the desired number of iterations has passed and initiates a system exit if so. We could also tick_once() in a loop for the desired amount of times. There might be a better solution that I'm not currently seeing in the py-tree documentation. Thoughts? @hidmic @BMarchi

@hidmic
Copy link
Contributor

hidmic commented Aug 8, 2019

I wonder why it does not abide to number_of_iterations.

@JShep1
Copy link
Author

JShep1 commented Aug 8, 2019

I guess it technically does considering it doesn't tick past the input number of iterations - I guess it doesn't return/exit?

@JShep1
Copy link
Author

JShep1 commented Aug 8, 2019

After some investigation it appears that the run_async_for(...) function (that we took out with the introduction of behaviour trees) was doing the work for ending the simulation when the user desires to run for a specified amount of time.

The launch_interactive_simulation(...) function calls launch_manager.wait(float("Inf")) which then waits for either the user to exit out of the visualizer if open, a CTRL+C, or the time that the user may have provided to elapse. The latter of those options is no longer being provided given the removal of run_async_for(...). It was a member function of runner which is no longer used.

We could (as far as my knowledge goes) add the tick_tock() call to be a process managed by launcher, this would probably mean adding a function into utilities.py similar to launch_visualizer(...). In doing so, launcher should then track that process and upon it recognizing that this process it is tracking has exited, exits itself as well.
or if possible just clean up and exit after the tick_tock(...) function is called.

@hidmic
Copy link
Contributor

hidmic commented Aug 9, 2019

Alright, now CI is happy. LGTM @JShep1 !

@JShep1
Copy link
Author

JShep1 commented Aug 9, 2019

I think that the line I added should be put in every example... not sure if colcon test tests the other examples that have gotten green CI with that specific case

@hidmic
Copy link
Contributor

hidmic commented Aug 9, 2019

That's correct!

@hidmic
Copy link
Contributor

hidmic commented Aug 12, 2019

@JShep1 merge?

@JShep1
Copy link
Author

JShep1 commented Aug 12, 2019

Sorry, wasn't sure if we were still holding off on pushing these through

@JShep1 JShep1 merged commit f93dcb5 into hidmic/py_trees-based-api Aug 12, 2019
@JShep1 JShep1 deleted the jshep1/gazoo_tree_demo branch August 12, 2019 15:36
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