Trigger events by name - Githubissues

jp-embedded / scxmlcc

The SCXML state machine to C++ compiler

GNU General Public License v3.0

140 stars 34 forks source link

Trigger events by name #71

Closed sstiller closed 5 years ago

sstiller commented 6 years ago

Currently only existing events can be triggered in the SM. Unknown events lead to compile errors (if the SM is changed, events can be removed from the implementation).

In my real world example I have a machine which does not know anything about the integrated state machine. It even does not know, which signals are possible, if event sources are added during runtime by a plugin.

Another problem: I want to be able to exchange the state machine implementation depending on the use case (the SM can also be loaded as a plugin). That means, I don't know, which events are implemented in the current SM.

There are more cases where I need much more flexibility.

For this reason I added a function to trigger an event with its name by overloading dispatch(). The most interesting commit is this one: https://github.com/sstiller/scxmlcc/commit/5c8a108bb3604cfa57ac64afbf56ba89154f0ff6

I know, the performance is worse than the current solution, but it provides many possibilities like nice timer interfaces.

Before merging:

This code should be tested a little bit more

Maybe later:

Event name caching should be implemented (thread safe?) to improve performance for some events. Example: SM only knows event ev1, but dispatch() is called with "ev1.foo.bar.something". Other example: dispatch() is called with "ev2.foo.bar.something", but ev2* is not implemented

What do you think about adding this code?

jp-embedded commented 6 years ago

That is a great extension and gives more flexibility, where performance is not critical. I have thought about this also. Thanks :-)

Could you add a command line option which enables this? I like the generated state machines to be as simple as possible at not bloated with unused (depending on the use case) functionality as this and other future ideas are added.

jp-embedded commented 6 years ago

Also, a few unit tests would be very nice to ensure the functionality won't be broken as the code changes.

jp-embedded commented 6 years ago

is it ready to be merged in?

sstiller commented 6 years ago

There are three things to do before merging:

[ ] Tests are still missing.
[ ] The microsoft compiler version used in appveyor seems to have a problem with the initialisation of the event map: https://ci.appveyor.com/project/jp-embedded/scxmlcc/build/1.0.235/job/5136c6rdr091yobb I don't have a MS compiler to test it, but I think, it's easy to fix.
[ ] I have an issue in the master branch with datamodel update and eventless transitions, which should be fixed before, because this PR makes it a little bit worse.

jp-embedded commented 6 years ago

Regarding eventless transitions. You mean this issue? https://github.com/jp-embedded/scxmlcc/issues/40#issuecomment-427275167

I will try to prioritize this as soon as I have time for it.

sstiller commented 6 years ago

Regarding eventless transitions. You mean this issue? #40 (comment)

No, it's an other problem. I will create a new issue and / or PR for it as soon as I have time for it.

jp-embedded commented 6 years ago

okay :-)

sstiller commented 5 years ago

I did some performance tests for 8edcbed, the caching really helps a lot.

sstiller commented 5 years ago

The tests work now, if the makefiles are used. msvs and cmake need fixing by someone who knows how to do it.