Added transition_callback #77
Conversation
|
I don't think it matters too much since the user will have both the input and output state directly in the method call as opposed to via the context. |
macros/src/codegen.rs
Outdated
| @@ -500,8 +502,10 @@ pub fn generate_code(sm: &ParsedStateMachine) -> proc_macro2::TokenStream { | |||
| quote!{ | |||
| #action_code | |||
| let out_state = #states_type_name::#out_state; | |||
| let in_state = #states_type_name::#in_state; | |||
There was a problem hiding this comment.
Does this actually work for in_state that has data associated with it? I'm not sure how we're moving the internal state machine data around.
There was a problem hiding this comment.
Nope, the sample does not compile. Nor do the tests. I'll rework this so it will all work and compile as intended. I think we do not want the data to be passed to the transition callback, since it will make things way more complex (if not impossible for a generic callback).
macros/src/parser/state_machine.rs
Outdated
| \"custom_guard_error\", \ | ||
| \"derive_states\", \ | ||
| \"derive_events\" | ||
| "Unknown keyword {}. Support keywords: [\"name\", |
There was a problem hiding this comment.
Out of curiosity, how does this shown up in the console? The \ ending were supposed to prevent newlines from being inserted into the formatted string.
There was a problem hiding this comment.
I think it was some autoformat, but in my console it looks good though.
error: Unknown keyword unknown. Support keywords: ["name",
"transitions",
"temporary_context",
"custom_guard_error",
"derive_states",
"derive_events",
]
but since it has nothing to do with this MR, I'll revert it.
MartinBroers
force-pushed
the
transition-callback
branch
from
2f97f41 to
43694af
Compare
This patch adds a transition_callback function. The function, if not used, will be optimized away by the compiler. Signed-off-by: Martin Broers <[email protected]>
MartinBroers
force-pushed
the
transition-callback
branch
from
43694af to
0b23445
Compare
examples/transition_callback.rs
Outdated
| #![deny(missing_docs)] | ||
|
|
||
| use std::rc::Rc; | ||
| use std::sync::Mutex; |
There was a problem hiding this comment.
I don't see why we need all these sync primitives, and I think they may make the example more complex than it should be.
Also, could we insert this into an existing example instead?
There was a problem hiding this comment.
Yes, moved to dominos example. I also removed the sync primitives. We needed them to have a non-mut reference to self in the callback. Don't see the need to make it mutable at this point in time. If we need it mutable, we can do so in another feature request?
There was a problem hiding this comment.
I think its probably correct that we don't get mut access in the transition callback, since we shouldn't be mutating the state machine
Signed-off-by: Martin Broers <[email protected]>
@ryan-summers Thanks for merging !66. I also like the function of a transition callback, so I went straight on it.
In terms of refinement and/or discussion, I am doubting mostly about this right now:
Or should it be like this:
So, call the transition callback after the internal state has been updated?