Support on_entry and on_exit for states

[MartinBroers]

Issue #60 describes what I needed. I may have sprinkled a few Debug statements here and there which may not have been required, but I am open to suggestions to remove them.

I am opening this MR since @pleepleus asked to have a go at it. I don't want him to start from scratch.

[MartinBroers]

Some general feedback is below. I don't know the state of this or if the intent is to merge it as-is or if it's just inteded to be a starting point.

If this is just a reference implementation for someone else to pick up, can we mark it as a draft? Otherwise, I'm not sure if I should be reviewing or not.

Yes, wasn't totally clear on that. Will work on this to make it final and ping you when I'm happy. Also, open for edits by others if they have time before I do.

[MartinBroers]

I have added two new features, generate_entry_exit_states and generate_transition_callback. Code will not be generated when these flags are not present. Please let me know what you think of these additions. I am a bit unsure on the naming of the whole thing, but naming is important.

[dgehriger]

@ryan-summers : do you know what the state of this PR is? Has it gone stale?

[ryan-summers]

I haven't seen any response from the author to my review, so I would assume it's been abandoned, but maybe these comments may grab their attention as well :)

[MartinBroers]

@dgehriger I had it on my list to fix, but got other priorities to work on first. I spent some time to rework the comments, but I failed in properly understanding the macros. Then I got other things on my plate and this got pushed down on my todo list.

So, to re-open the issue for myself and start thinking about it again, I have the token parser to parse < and >, but I am failing to get those functions into the generated result. Its been a while since I've looked at it though.

So, in transition.rs I now have

if input.parse::<Token![<]>().is_ok() 

which generates an entry for StateTransitions. Now, I need to get those fields back in the result, if they are not None, then generate a function for them. My code is very much in progress, but I'll push it anyway, and I am very open to remarks about how to get this issue done with.

[andygikling]

Bump. This appears to be one of the more feature-rich state machines in rust that anyone uses. on_enter and on_exit are table stakes for a state machine library. Can we get this feature in here please?

[ryan-summers]

I have nothing against the implementation @andygikling, but it is not yet done to my knowledge.

This, like many open-sourced projects, is maintained primarily through volunteer time. I'm happy to review any proposed changes you may have for the repo, but I don't have significant amounts of time to implement new features unless I need them for specific projects that I'm working on.

[andygikling]

Ok thanks I'll take a closer look and see what's missing!

[MartinBroers]

So, I've finally gotten around to update this PR. I implemented the notation with > and < to indicate entry- and exit functions.

@ryan-summers pinging for review.

[ryan-summers]

I see that I initially proposed it, but the > and < notation is extremely confusing to me.

Would it make more sense to provide generic on_enter_<snake-case statename> and on_exit_<snake-case statename> for all states and provide them in the trait definition? Then, if the user wants to implement the entry/exit functions, they just have to provide a custom implementation.

This keeps the notation entirely out of the DSL and the compiler can optimize out the empty enter/exit calls if the user doesn't actually need them.

It also should make the necessary code changes here pretty simple.

[MartinBroers]

@ryan-summers Thanks for the feedback. That was my original thought as well. When I read your comment I tried the flag and in rebasing the flag didn't work. So I updated it now and now both ways should work. I kinda like your suggestion with the '<' and '>' as well, so I left them in for now. Updated README and added an example.

[ryan-summers]

@ryan-summers Thanks for the feedback. That was my original thought as well. When I read your comment I tried the flag and in rebasing the flag didn't work. So I updated it now and now both ways should work.

I was actually advocating for removing the flag entirely and having the on_entry and on_exit functions defined by-default for all users. This means the DSL doesn't actually change at all and this change isn't breaking. The trait would provide the default implementation, so for example:

enum States {
    Start,
}

trait StateMachineContext {
    fn on_entry_start() {
        // Default, empty implementation that the user can override in their context definition.
    }
    fn on_exit_start() {
    }

}

[MartinBroers]

I have removed the flag and let all on_entry and on_exit functions be autogenerated from now. I think it is pretty neat now.

Thanks for your time. I will have a look at the rebase error and the pipeline failures.

Should I also add my example to the tests suite?

[ryan-summers]

@MartinBroers I removed the transition_callback function for now because I wanted to separate it from the on_entry and on_exit change. I'm not opposed to the idea, but I think it needs more refinement/discussion first, and the other callbacks are ready for now. Feel free to open up a secondary PR for the transition callback changes if you'd still like them!

I think I'd go for the route where it's defined as empty-by-default and the user doesn't need to specify any flags etc.