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

Concurrent states #14

Open
chrisbitter opened this issue Mar 30, 2024 · 9 comments
Open

Concurrent states #14

chrisbitter opened this issue Mar 30, 2024 · 9 comments

Comments

@chrisbitter
Copy link

I am currently exploring YASMIN as alternative to Smach for ROS2.
Is it possible to model concurrent states (analogous to https://wiki.ros.org/smach/Tutorials/Concurrent%20States)?

If not, any idea of how this can be integrated? Happy to contribute.

@mgonzs13
Copy link
Collaborator

mgonzs13 commented Mar 30, 2024

Hey @chrisbitter, this will be a great feature to add to YASMIN. A primitive version could be this, but the outcomes of the states and the cancelations should be added. In this case, a set of states are executed concurrently and the same outcome is returned in any case. Do you have any suggestions?

from threading import Thread
from typing import List
from yasmin import State
from yasmin.blackboard import Blackboard


class Concurrence(State):

    def __init__(self, outcome: str, states: List[State]) -> None:

        super().__init__([outcome])

        self.states = states

    def execute(self, blackboard: Blackboard) -> str:

        state_threads = []

        for s in self.states:
            state_threads.append(Thread(target=s, args=(blackboard,)))
            state_threads[-1].run()

        for t in state_threads:
            t.join()

        return self._outcomes[0]

@mgonzs13
Copy link
Collaborator

mgonzs13 commented Jun 3, 2024

Hey @chrisbitter, how is this going?

@teapfw
Copy link

teapfw commented Jan 13, 2025

Thanks @mgonzs13 for the boilerplate. I required this feature for a project and added it to my fork, modelling it off of Smach's interface for concurrency. I've only written the Python support so far and tested minimally, but may open a PR if I get to the C++ side as well.

@mgonzs13
Copy link
Collaborator

Hi @teapfw, thanks for the work. A PR with the changes would be great. There are some minor errors and fixes that you can treat: comments on init params are wrong, you should avoid using str directly and the string composed in the str starts with State Machine. Besides, using the state class name is not a good idea to create the outcome map.

@mgonzs13 mgonzs13 reopened this Jan 14, 2025
@teapfw
Copy link

teapfw commented Jan 14, 2025

Thanks for taking a look and for your suggestions. Why do you say it is not a good idea to use the state class name for building the outcome map? Is it because of the possibility for different states to have the same string name? If so, perhaps generating UUIDs for each state would be a better option.

@mgonzs13
Copy link
Collaborator

mgonzs13 commented Jan 14, 2025

I'm thinking of the case of adding the same state more than once. Besides, the C++ version does not work equal to the Python version when generating the names. The UUIDs will not help since the complete name will be unknown before execution.

@teapfw
Copy link

teapfw commented Jan 14, 2025

I have not considered the C++ side extensively, but I've modified the way the concurrent states are mapped to be more robust and added a test for verification. The class string is not used anymore, and instead the index associated with each state in the list argument is used as an identifier. The only caveat I observe with the new approach is the same instance of a state cannot be run concurrently with itself; instead, a new instance must be created.

@mgonzs13
Copy link
Collaborator

Another option could be implementing an add_state method similar to the one used in the state machine. Btw, have you tried this with the new updates?

@teapfw
Copy link

teapfw commented Jan 15, 2025

Yes, I've run the pytest I wrote for it if that's what you mean, and confirmed the states run in parallel.

Using a separate method is another option indeed, but would be somewhat more verbose. Currently, the states can be used directly in the Concurrence constructor's outcome map. If moved into a separate method, it would be possible to assign a string identifier to each state and to use that identifier in the outcome map to correlate them. This is more cumbersome, as the user now needs to create an additional string for each state. However, it would also make it possible to run the exact same instance of a state concurrently with itself. This is prevented currently due to how the correlations are built in the constructor. That said, I'm not sure running a single instance in parallel with itself is a sensible thing to do to begin with. I'm undecided on which approach is better. Both should be implementable in C++ with shared pointers.

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

No branches or pull requests

3 participants