FGFS-4.1/simgear
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve parsing of state machines

Browse Source 
State machines will now throw exceptions if errors are encountered
when loading the XML file.

This includes:
- Unknown <source> or <target> states referenced by transitions
- States and transitions without <name> elements
- Transitions without <target> or <condition> elements
- State machines with fewer than two states

Previously the state machine would load and then crash when hitting
such malformed states or transitions.  Now it will refuse to load,
while providing useful error messages as to the problem.
This commit is contained in:
Stuart Buchanan
2019-02-14 22:15:38 +00:00
parent 98215e3dbd
commit f076c43b90
1 changed files with 111 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

174
simgear/structure/StateMachine.cxx
View File

@@ -20,7 +20,7 @@
*/
#include <simgear_config.h>
#include "StateMachine.hxx"
#include <algorithm>
@@ -32,13 +32,13 @@
#include <simgear/props/condition.hxx>
#include <simgear/timing/timestamp.hxx>
#include <simgear/structure/exception.hxx>
namespace simgear
{
typedef std::vector<StateMachine::State_ptr> StatePtrVec;
static void readBindingList(SGPropertyNode* desc, const std::string& name,
static void readBindingList(SGPropertyNode* desc, const std::string& name,
SGPropertyNode* root, SGBindingList& result)
{
for (auto b : desc->getChildren(name)) {
@@ -54,8 +54,8 @@ class StateMachine::State::StatePrivate
{
public:
std::string _name;
SGBindingList _updateBindings,
_entryBindings,
SGBindingList _updateBindings,
_entryBindings,
_exitBindings;
};
@@ -71,14 +71,14 @@ public:
bool _excludeTarget;
SGSharedPtr<SGCondition> _condition;
};
///////////////////////////////////////////////////////////////////////////
class StateMachine::StateMachinePrivate : public SGPropertyChangeListener
{
public:
StateMachinePrivate(StateMachine* p) : _p(p) { }
void computeEligibleTransitions()
{
_eligible.clear();
@@ -88,7 +88,7 @@ public:
}
}
}
StateMachine* _p;
bool _initialised;
State_ptr _currentState;
@@ -96,14 +96,14 @@ public:
std::vector<Transition_ptr> _transitions;
std::vector<Transition*> _eligible;
SGTimeStamp _timeInState;
bool _listenerLockout; ///< block our listener when self-updating props
virtual void valueChanged(SGPropertyNode* changed)
{
if (_listenerLockout) {
return;
}
if (changed == _currentStateIndex) {
State_ptr s = _p->stateByIndex(changed->getIntValue());
_p->changeToState(s);
@@ -111,7 +111,7 @@ public:
_p->changeToStateName(changed->getStringValue());
}
}
// exposed properties
SGPropertyNode_ptr _root;
SGPropertyNode_ptr _currentStateIndex;
@@ -144,12 +144,12 @@ void StateMachine::State::update()
void StateMachine::State::fireEntryBindings()
{
fireBindingList(d->_entryBindings);
}
}
void StateMachine::State::fireExitBindings()
{
fireBindingList(d->_exitBindings);
}
}
void StateMachine::State::addUpdateBinding(SGBinding* aBinding)
{
@@ -184,38 +184,38 @@ StateMachine::Transition::~Transition()
StateMachine::State* StateMachine::Transition::target() const
{
return d->_target;
}
}
void StateMachine::Transition::addSourceState(State* aSource)
{
if (aSource == d->_target) { // should this be disallowed outright?
SG_LOG(SG_GENERAL, SG_WARN, d->_name << ": adding target state as source");
}
d->_sourceStates.insert(aSource);
}
}
bool StateMachine::Transition::applicableForState(State* aCurrent) const
{
if (d->_excludeTarget && (aCurrent == d->_target)) {
return false;
}
if (d->_sourceStates.empty()) {
return true;
}
return d->_sourceStates.count(aCurrent);
}
}
bool StateMachine::Transition::evaluate() const
{
return d->_condition->test();
}
}
void StateMachine::Transition::fireBindings()
{
fireBindingList(d->_bindings);
}
}
std::string StateMachine::Transition::name() const
{
@@ -231,12 +231,12 @@ void StateMachine::Transition::addBinding(SGBinding* aBinding)
{
d->_bindings.push_back(aBinding);
}
void StateMachine::Transition::setExcludeTarget(bool aExclude)
{
d->_excludeTarget = aExclude;
}
///////////////////////////////////////////////////////////////////////////
StateMachine::StateMachine() :
@@ -249,7 +249,7 @@ StateMachine::StateMachine() :
StateMachine::~StateMachine()
{
}
void StateMachine::init()
@@ -257,23 +257,23 @@ void StateMachine::init()
if (d->_initialised) {
return;
}
if (d->_states.empty()) {
throw sg_range_exception("StateMachine::init: no states defined");
}
d->_currentStateIndex = d->_root->getChild("current-index", 0, true);
d->_currentStateIndex->setIntValue(0);
d->_currentStateName = d->_root->getChild("current-name", 0, true);
d->_currentStateName->setStringValue("");
d->_currentStateIndex->addChangeListener(d.get());
d->_currentStateName->addChangeListener(d.get());
d->_timeInStateProp = d->_root->getChild("elapsed-time-msec", 0, true);
d->_timeInStateProp->setIntValue(0);
// TODO go to default state if found
innerChangeState(d->_states[0], NULL);
d->_initialised = true;
@@ -283,20 +283,23 @@ void StateMachine::shutdown()
{
d->_currentStateIndex->removeChangeListener(d.get());
d->_currentStateName->removeChangeListener(d.get());
}
void StateMachine::innerChangeState(State_ptr aState, Transition_ptr aTrans)
{
if (d->_currentState) {
d->_currentState->fireExitBindings();
SG_LOG(SG_GENERAL, SG_INFO, "Changing from state " << d->_currentState->name() << " to state:" << aState->name());
} else {
SG_LOG(SG_GENERAL, SG_INFO, "Initializing to state:" << aState->name());
}
// fire bindings before we change the state, hmmmm
// fire bindings before we change the state, hmmmm
if (aTrans) {
aTrans->fireBindings();
}
// update our private state and properties
d->_listenerLockout = true;
d->_currentState = aState;
@@ -305,11 +308,11 @@ void StateMachine::innerChangeState(State_ptr aState, Transition_ptr aTrans)
d->_currentStateIndex->setIntValue(indexOfState(aState));
d->_timeInStateProp->setIntValue(0);
d->_listenerLockout = false;
// fire bindings
d->_currentState->fireEntryBindings();
d->_currentState->update();
d->computeEligibleTransitions();
}
@@ -319,11 +322,11 @@ void StateMachine::changeToState(State_ptr aState, bool aOnlyIfDifferent)
if (std::find(d->_states.begin(), d->_states.end(), aState) == d->_states.end()) {
throw sg_exception("Requested change to state not in machine");
}
if (aOnlyIfDifferent && (aState == d->_currentState)) {
return;
}
innerChangeState(aState, NULL);
}
@@ -333,7 +336,7 @@ void StateMachine::changeToStateName(const std::string& aName, bool aOnlyIfDiffe
if (!st) {
throw sg_range_exception("unknown state:" + aName);
}
changeToState(st, aOnlyIfDifferent);
}
@@ -341,7 +344,7 @@ StateMachine::State_ptr StateMachine::state() const
{
return d->_currentState;
}
SGPropertyNode* StateMachine::root()
{
return d->_root;
@@ -352,27 +355,27 @@ void StateMachine::update(double aDt)
// do this first, for triggers which depend on time in current state
// (spring-loaded transitions)
d->_timeInStateProp->setIntValue(d->_timeInState.elapsedMSec());
Transition_ptr trigger;
for (auto trans : d->_eligible) {
if (trans->evaluate()) {
if (trigger != Transition_ptr()) {
SG_LOG(SG_GENERAL, SG_WARN, "ambiguous transitions! "
SG_LOG(SG_GENERAL, SG_WARN, "ambiguous transitions! "
<< trans->name() << " or " << trigger->name());
}
trigger = trans;
}
}
if (trigger != Transition_ptr()) {
SG_LOG(SG_GENERAL, SG_DEBUG, "firing transition:" << trigger->name());
innerChangeState(trigger->target(), trigger);
}
d->_currentState->update();
}
}
StateMachine::State_ptr StateMachine::findStateByName(const std::string& aName) const
{
@@ -381,7 +384,7 @@ StateMachine::State_ptr StateMachine::findStateByName(const std::string& aName)
return sp;
}
}
SG_LOG(SG_GENERAL, SG_WARN, "unknown state:" << aName);
return State_ptr();
}
@@ -391,17 +394,17 @@ StateMachine::State_ptr StateMachine::stateByIndex(unsigned int aIndex) const
if (aIndex >= d->_states.size()) {
throw sg_range_exception("invalid state index, out of bounds");
}
return d->_states[aIndex];
}
int StateMachine::indexOfState(State_ptr aState) const
{
StatePtrVec::const_iterator it = std::find(d->_states.begin(), d->_states.end(), aState);
if (it == d->_states.end()) {
return -1;
}
return it - d->_states.begin();
}
@@ -410,7 +413,7 @@ StateMachine::State_ptr StateMachine::createState(const std::string& aName)
if (findStateByName(aName) != NULL) {
throw sg_range_exception("duplicate state name");
}
State_ptr st = new State(aName);
addState(st);
return st;
@@ -431,38 +434,83 @@ void StateMachine::initFromPlist(SGPropertyNode* desc, SGPropertyNode* root)
d->_root = root->getNode(path, 0, true);
assert(d->_root);
}
int stateCount = 0;
for (auto stateDesc : desc->getChildren("state")) {
stateCount++;
std::string nm = stateDesc->getStringValue("name");
if (nm.empty()) {
SG_LOG(SG_GENERAL, SG_ALERT, "No name found for state in branch " << path);
throw sg_exception("No name element in state");
}
State_ptr st(new State(nm));
readBindingList(stateDesc, "enter", root, st->d->_entryBindings);
readBindingList(stateDesc, "update", root, st->d->_updateBindings);
readBindingList(stateDesc, "exit", root, st->d->_exitBindings);
addState(st);
} // of states iteration
if (stateCount < 2) {
SG_LOG(SG_GENERAL, SG_ALERT, "Fewer than two state elements found in branch " << path);
throw sg_exception("Fewer than two state elements found.");
}
for (auto tDesc : desc->getChildren("transition")) {
std::string nm = tDesc->getStringValue("name");
State_ptr target = findStateByName(tDesc->getStringValue("target"));
std::string target_id = tDesc->getStringValue("target");
if (nm.empty()) {
SG_LOG(SG_GENERAL, SG_ALERT, "No name found for transition in branch " << path);
throw sg_exception("No name element in transition");
}
if (target_id.empty()) {
SG_LOG(SG_GENERAL, SG_ALERT, "No target element in transition "
<< nm << " in state branch " << path);
throw sg_exception("No target element in transition");
}
State_ptr target = findStateByName(target_id);
if (target == NULL) {
SG_LOG(SG_GENERAL, SG_ALERT, "Unknown target state " << target_id << " in transition "
<< nm << " in state branch " << path);
throw sg_exception("No condition element in transition");
}
if (tDesc->getChild("condition") == NULL) {
SG_LOG(SG_GENERAL, SG_ALERT, "No condition element in transition "
<< nm << " in state branch " << path);
throw sg_exception("No condition element in transition");
}
SGCondition* cond = sgReadCondition(root, tDesc->getChild("condition"));
Transition_ptr t(new Transition(nm, target));
t->setTriggerCondition(cond);
t->setExcludeTarget(tDesc->getBoolValue("exclude-target", true));
for (auto src : tDesc->getChildren("source")) {
State_ptr srcState = findStateByName(src->getStringValue());
if (srcState == NULL) {
SG_LOG(SG_GENERAL, SG_ALERT, "Unknown source state " << src->getStringValue() << " in transition "
<< nm << " in state branch " << path);
throw sg_exception("No condition element in transition");
}
t->addSourceState(srcState);
}
readBindingList(tDesc, "binding", root, t->d->_bindings);
addTransition(t);
} // of states iteration
init();
}