Stm outline #54
Stm outline #54
Conversation
lib/state_machine/state_machine.cpp
Outdated
| update data | ||
| should check if a transition is needed with current data | ||
| then run transition to move to next state | ||
| return (None || transition_message) |
There was a problem hiding this comment.
This should be an std::optional return type then
lib/state_machine/state_machine.cpp
Outdated
| } | ||
|
|
||
| // Transition to next state | ||
| void StateMachine::transition(Message message) |
lib/state_machine/state_machine.cpp
Outdated
|
|
||
| void StateMachine::reset() | ||
| { | ||
| current_state = states[0]; |
There was a problem hiding this comment.
Isn't the point of having an enum class so that you can just set current_state = State::kInitialState? Idk if I'm missing something here but why are we instantiating a local object that is kind of similar to the enum class we already defined?
lib/state_machine/state_machine.hpp
Outdated
| void reset(); | ||
|
|
||
| // TODOLater change states to actual states | ||
| State states[5] |
lib/state_machine/state_machine.hpp
Outdated
| = {State::kInitialState, State::state1, State::state2, State::state3, State::state4}; | ||
|
|
||
| // TODOLater change transitions to actual transitions | ||
| Transition transitions[5] = { |
There was a problem hiding this comment.
You've implicitly used the number of states a few times now without stating where the number 5 comes from. Better design would be to create a variable somewhere (e.g. kNumStates = 5) and refer to that?
lib/state_machine/state_machine.hpp
Outdated
| Transition(State::state3, State::state4, Message::message1), | ||
| }; | ||
|
|
||
| State current_state = states[0]; |
There was a problem hiding this comment.
Would this not be initialized in constructor?
lib/state_machine/transition.hpp
Outdated
| // transition. | ||
| class Transition { | ||
| public: | ||
| Transition(State from_state, State to_state, Message transition_message) |
There was a problem hiding this comment.
Should this not be a separate function within this class? Also why are we storing all of this as class members?
lib/state_machine/transition.hpp
Outdated
| @@ -0,0 +1,24 @@ | |||
| #pragma once | |||
|
|
|||
| #include "message.cpp" | |||
There was a problem hiding this comment.
Don't include a cpp file. Always include headers (all functions, classes, etc. should be defined there anyways).
lib/state_machine/state_machine.hpp
Outdated
|
|
||
| std::optional<Message> checkTransition(); | ||
|
|
||
| void reset(); |
There was a problem hiding this comment.
This is not defined in your set of transitions so how do you define this behavior?
Start of the outline of the state machine code, is very far from done. This will be the basic structure of the state machine instances with appropriate states and transitions.