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.

Sign up for GitHub

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

Jump to bottom

[decorators] finally-style decorators and idioms #427

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

[decorators] finally-style decorators and idioms #427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"bierner.markdown-preview-github-styles",
"bungcip.better-toml",
"eamodio.gitlens",
"joaompinto.vscode-graphviz",
"ms-python.python",
"omnilib.ufmt",
"redhat.vscode-yaml",
Expand Down
10 changes: 7 additions & 3 deletions .devcontainer/py310/.devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@
"vscode": {
"extensions": [
"bierner.github-markdown-preview",
"bierner.markdown-preview-github-styles",
"bungcip.better-toml",
"streetsidesoftware.code-spell-checker",
"lextudio.restructuredtext",
"eamodio.gitlens",
"joaompinto.vscode-graphviz",
"ms-python.python",
"omnilib.ufmt"
"omnilib.ufmt",
"redhat.vscode-yaml",
"streetsidesoftware.code-spell-checker",
"tht13.rst-vscode"
]
}
},
Expand Down
5 changes: 5 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,15 @@
"behaviours",
"bierner",
"bungcip",
"epilog",
"graphviz",
"literalinclude",
"noodly",
"omnilib",
"py_trees",
"pydot",
"pypi",
"seealso",
"ufmt",
"usort"
]
Expand Down
2 changes: 1 addition & 1 deletion CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Release Notes

Forthcoming
-----------
* ...
* [decorators] finally-style decorators and idioms, `#427 <https://github.com/splintered-reality/py_trees/pull/427>`_

2.2.3 (2023-02-08)
------------------
Expand Down
32 changes: 32 additions & 0 deletions docs/demos.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,38 @@ py-trees-demo-eternal-guard
:linenos:
:caption: py_trees/demos/eternal_guard.py

.. _py-trees-demo-eventually-program:

py-trees-demo-eventually
------------------------

.. automodule:: py_trees.demos.eventually
:members:
:special-members:
:show-inheritance:
:synopsis: demo the eventually idiom

.. literalinclude:: ../py_trees/demos/eventually.py
:language: python
:linenos:
:caption: py_trees/demos/eventually.py

.. _py-trees-demo-eventually-swiss-program:

py-trees-demo-eventually-swiss
------------------------------

.. automodule:: py_trees.demos.eventually_swiss
:members:
:special-members:
:show-inheritance:
:synopsis: demo the general purpose eventually idiom

.. literalinclude:: ../py_trees/demos/eventually_swiss.py
:language: python
:linenos:
:caption: py_trees/demos/eventually_swiss.py

.. _py-trees-demo-logging-program:

py-trees-demo-logging
Expand Down
19 changes: 19 additions & 0 deletions docs/dot/demo-eventually-swiss.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
digraph pastafarianism {
ordering=out;
graph [fontname="times-roman"];
node [fontname="times-roman"];
edge [fontname="times-roman"];
"Count with Result" [fillcolor=cyan, fontcolor=black, fontsize=9, label="Count with Result", shape=octagon, style=filled];
"Work to Success" [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ Work to Success", shape=box, style=filled];
"Count with Result" -> "Work to Success";
Counter [fillcolor=gray, fontcolor=black, fontsize=9, label=Counter, shape=ellipse, style=filled];
"Work to Success" -> Counter;
SetResultTrue [fillcolor=gray, fontcolor=black, fontsize=9, label=SetResultTrue, shape=ellipse, style=filled];
"Work to Success" -> SetResultTrue;
"On Failure" [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ On Failure", shape=box, style=filled];
"Count with Result" -> "On Failure";
SetResultFalse [fillcolor=gray, fontcolor=black, fontsize=9, label=SetResultFalse, shape=ellipse, style=filled];
"On Failure" -> SetResultFalse;
Failure [fillcolor=gray, fontcolor=black, fontsize=9, label=Failure, shape=ellipse, style=filled];
"On Failure" -> Failure;
}
17 changes: 17 additions & 0 deletions docs/dot/demo-eventually.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
digraph pastafarianism {
ordering=out;
graph [fontname="times-roman"];
node [fontname="times-roman"];
edge [fontname="times-roman"];
"Count and Record" [fillcolor=gold, fontcolor=black, fontsize=9, label="Count and Record\nSuccessOnOne", shape=parallelogram, style=filled];
Counting [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ Counting", shape=box, style=filled];
"Count and Record" -> Counting;
SetCountingFlagTrue [fillcolor=gray, fontcolor=black, fontsize=9, label=SetCountingFlagTrue, shape=ellipse, style=filled];
Counting -> SetCountingFlagTrue;
Counter [fillcolor=gray, fontcolor=black, fontsize=9, label=Counter, shape=ellipse, style=filled];
Counting -> Counter;
Eventually [fillcolor=ghostwhite, fontcolor=black, fontsize=9, label=Eventually, shape=ellipse, style=filled];
"Count and Record" -> Eventually;
SetCountingFlagFalse [fillcolor=gray, fontcolor=black, fontsize=9, label=SetCountingFlagFalse, shape=ellipse, style=filled];
Eventually -> SetCountingFlagFalse;
}
17 changes: 17 additions & 0 deletions docs/dot/demo-finally-single-tick.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
digraph pastafarianism {
ordering=out;
graph [fontname="times-roman"];
node [fontname="times-roman"];
edge [fontname="times-roman"];
root [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ root", shape=box, style=filled];
SetFlagFalse [fillcolor=gray, fontcolor=black, fontsize=9, label=SetFlagFalse, shape=ellipse, style=filled];
root -> SetFlagFalse;
Parallel [fillcolor=gold, fontcolor=black, fontsize=9, label="Parallel\nSuccessOnOne", shape=parallelogram, style=filled];
root -> Parallel;
Counter [fillcolor=gray, fontcolor=black, fontsize=9, label=Counter, shape=ellipse, style=filled];
Parallel -> Counter;
Finally [fillcolor=ghostwhite, fontcolor=black, fontsize=9, label=Finally, shape=ellipse, style=filled];
Parallel -> Finally;
SetFlagTrue [fillcolor=gray, fontcolor=black, fontsize=9, label=SetFlagTrue, shape=ellipse, style=filled];
Finally -> SetFlagTrue;
}
19 changes: 19 additions & 0 deletions docs/dot/eventually-swiss.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
digraph pastafarianism {
ordering=out;
graph [fontname="times-roman"];
node [fontname="times-roman"];
edge [fontname="times-roman"];
"Eventually (Swiss)" [fillcolor=cyan, fontcolor=black, fontsize=9, label="Eventually (Swiss)", shape=octagon, style=filled];
"Work to Success" [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ Work to Success", shape=box, style=filled];
"Eventually (Swiss)" -> "Work to Success";
Worker [fillcolor=gray, fontcolor=black, fontsize=9, label=Worker, shape=ellipse, style=filled];
"Work to Success" -> Worker;
"On Success" [fillcolor=gray, fontcolor=black, fontsize=9, label="On Success", shape=ellipse, style=filled];
"Work to Success" -> "On Success";
"On Failure" [fillcolor=orange, fontcolor=black, fontsize=9, label="Ⓜ On Failure", shape=box, style=filled];
"Eventually (Swiss)" -> "On Failure";
"On Failure*" [fillcolor=gray, fontcolor=black, fontsize=9, label="On Failure*", shape=ellipse, style=filled];
"On Failure" -> "On Failure*";
Failure [fillcolor=gray, fontcolor=black, fontsize=9, label=Failure, shape=ellipse, style=filled];
"On Failure" -> Failure;
}
13 changes: 13 additions & 0 deletions docs/dot/eventually.dot
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
digraph pastafarianism {
ordering=out;
graph [fontname="times-roman"];
node [fontname="times-roman"];
edge [fontname="times-roman"];
Eventually [fillcolor=gold, fontcolor=black, fontsize=9, label="Eventually\nSuccessOnOne", shape=parallelogram, style=filled];
Worker [fillcolor=gray, fontcolor=black, fontsize=9, label=Worker, shape=ellipse, style=filled];
Eventually -> Worker;
"Eventually*" [fillcolor=ghostwhite, fontcolor=black, fontsize=9, label="Eventually*", shape=ellipse, style=filled];
Eventually -> "Eventually*";
"On Completion" [fillcolor=gray, fontcolor=black, fontsize=9, label="On Completion", shape=ellipse, style=filled];
"Eventually*" -> "On Completion";
}
16 changes: 16 additions & 0 deletions docs/examples/eventually.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import py_trees

if __name__ == "__main__":
worker = py_trees.behaviours.Success(name="Worker")
on_completion = py_trees.behaviours.Success(name="On Completion")
root = py_trees.idioms.eventually(
name="Eventually",
worker=worker,
on_completion=on_completion,
)
py_trees.display.render_dot_tree(
root, py_trees.common.string_to_visibility_level("all")
)
18 changes: 18 additions & 0 deletions docs/examples/eventually_swiss.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-

import py_trees

if __name__ == "__main__":
worker = py_trees.behaviours.Success(name="Worker")
on_failure = py_trees.behaviours.Success(name="On Failure")
on_success = py_trees.behaviours.Success(name="On Success")
root = py_trees.idioms.eventually_swiss(
name="Eventually (Swiss)",
workers=[worker],
on_failure=on_failure,
on_success=on_success,
)
py_trees.display.render_dot_tree(
root, py_trees.common.string_to_visibility_level("all")
)
12 changes: 12 additions & 0 deletions docs/idioms.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ Either Or

.. _oneshot-section:

Eventually
----------

.. automethod:: py_trees.idioms.eventually
:noindex:

Eventually - Swiss Variant
--------------------------

.. automethod:: py_trees.idioms.eventually_swiss
:noindex:

Oneshot
-------

Expand Down
Binary file added docs/images/demo-eventually-swiss.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/images/demo-eventually.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion py_trees/behaviour.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def iterate(self, direct_descendants: bool = False) -> typing.Iterator[Behaviour
yield child
yield self

# TODO: better type refinement of 'viso=itor'
# TODO: better type refinement of 'visitor'
def visit(self, visitor: typing.Any) -> None:
"""
Introspect on this behaviour with a visitor.
Expand Down
1 change: 1 addition & 0 deletions py_trees/behaviours.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ def update(self) -> common.Status:
:data:`~py_trees.common.Status.RUNNING` while not expired, the given completion status otherwise
"""
self.counter += 1
self.feedback_message = f"count: {self.counter}"
if self.counter <= self.duration:
return common.Status.RUNNING
else:
Expand Down
62 changes: 62 additions & 0 deletions py_trees/decorators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
* :class:`py_trees.decorators.EternalGuard`
* :class:`py_trees.decorators.Inverter`
* :class:`py_trees.decorators.OneShot`
* :class:`py_trees.decorators.OnTerminate`
* :class:`py_trees.decorators.Repeat`
* :class:`py_trees.decorators.Retry`
* :class:`py_trees.decorators.StatusToBlackboard`
Expand Down Expand Up @@ -920,3 +921,64 @@ def update(self) -> common.Status:
the behaviour's new status :class:`~py_trees.common.Status`
"""
return self.decorated.status


class OnTerminate(Decorator):
"""
Trigger the child for a single tick on :meth:`terminate`.

Always return :data:`~py_trees.common.Status.RUNNING` and on
on :meth:`terminate`, call the child's
:meth:`~py_trees.behaviour.Behaviour.update` method, once.

This is useful to cleanup, restore a context switch or to
implement a finally-like behaviour.

.. seealso:: :meth:`py_trees.idioms.eventually`
"""

def __init__(self, name: str, child: behaviour.Behaviour):
"""
Initialise with the standard decorator arguments.

Args:
name: the decorator name
child: the child to be decorated
"""
super(OnTerminate, self).__init__(name=name, child=child)

def tick(self) -> typing.Iterator[behaviour.Behaviour]:
"""
Bypass the child when ticking.

Yields:
a reference to itself
"""
self.logger.debug(f"{self.__class__.__name__}.tick()")
self.status = self.update()
yield self

def update(self) -> common.Status:
"""
Return with :data:`~py_trees.common.Status.RUNNING`.

Returns:
the behaviour's new status :class:`~py_trees.common.Status`
"""
return common.Status.RUNNING

def terminate(self, new_status: common.Status) -> None:
"""Tick the child behaviour once."""
self.logger.debug(
"{}.terminate({})".format(
self.__class__.__name__,
"{}->{}".format(self.status, new_status)
if self.status != new_status
else f"{new_status}",
)
)
if new_status == common.Status.INVALID:
self.decorated.tick_once()
Copy link

Choose a reason for hiding this comment

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

Is a tick_once necessarily enough to process the child behavior's sequence? The child behavior might be making an asynchronous call (e.g., to a ROS service, as I contributed in this py_trees_ros PR). I think this should instead tick the child behavior to completion.

(Which brings up a question of how often to tick the child behavior -- perhaps that should be a parameter passed to __init__?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not at all, you can see in my comments to the class a single tick won't suffice for all cases (i.e. here.

I think what we need then is a renamed decorator and two idioms. One for a single-tick finally with the decorator and one for a more complex multi-tick finally as highlighted in that comment.

Copy link

@amalnanavati amalnanavati Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I missed that part of the comment.

Adding onto what you wrote, and in light of our discussion on cloning, the multi-tick idiom would need three children passed in: the main behavior and two copies of the finally behavior, one to run on success and one to run on failure.

Copy link

Choose a reason for hiding this comment

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

I'm still trying to think of how we can eliminate the need to clone behaviors. What if our idiom is as follows:

Sequence
  |  FailureIsSuccess
  |    |  StatusToBlackboard
  |    |    |  Work
  |  Finally
  |  BlackboardToStatus

This way, if the work fails, the failure is passed up, and if finally fails, the failure is passed up. The only way this tree succeeds if both the Work and Finally succeeds.

Copy link
Member Author

@stonier stonier Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(Which brings up a question of how often to tick the child behavior -- perhaps that should be a parameter passed to init?)

With respect to this, I think the most intuitive default should be to tick as often as it needs to go to completion, i.e. to SUCCESS || FAILURE. If for some reason you explicitly need to control the # of ticks, you can use a Counter and a Parallel in the subtree to control that.

Copy link

Choose a reason for hiding this comment

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

Ah, I wasn't thinking about # of ticks, I was thinking about ticking rate. I agree with you that we should tick as necessary to complete the tree.

Copy link
Member Author

@stonier stonier Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'm still trying to think of how we can eliminate the need to clone behaviors. What if our idiom is as follows:

Sequence
  |  FailureIsSuccess
  |    |  StatusToBlackboard
  |    |    |  Work
  |  Finally
  |  BlackboardToStatus

After using them quite heavily, I found I am not a fan of the FailureIsSuccess style decorators and almost never use them now. They make it awfully hard to inspect a tree - human brains don't tend to fare so well with flip-flops, negations and even worse, double negations. A larger-tree is far more acceptable than a hard-to-read tree, especially if you make use of viz tooling that collapses parts of the tree.

Also, I think we should indeed pass in two handler behaviours (or subtrees). That makes it wonderfully general - you can pass in different handlers for failure/success or a clone of them.

# Do not need to stop the child here - this method
# is only called by Decorator.stop() which will handle
# that responsibility immediately after this method returns.
2 changes: 2 additions & 0 deletions py_trees/demos/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
from . import display_modes # usort:skip # noqa: F401
from . import dot_graphs # usort:skip # noqa: F401
from . import either_or # usort:skip # noqa: F401
from . import eventually # usort:skip # noqa: F401
from . import eventually_swiss # usort:skip # noqa: F401
from . import lifecycle # usort:skip # noqa: F401
from . import selector # usort:skip # noqa: F401
from . import sequence # usort:skip # noqa: F401
Expand Down
Loading