From 99c1dd812437f18a1f95f5b4bb180f76a130c8b0 Mon Sep 17 00:00:00 2001 From: Florent Rougon Date: Wed, 18 Apr 2018 08:12:34 +0200 Subject: [PATCH 1/4] Add compiler options for GCC and Clang when CMAKE_BUILD_TYPE is Debug When CMAKE_BUILD_TYPE is Debug and we are compiling with GCC, add the following options to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS: -O0 -fno-omit-frame-pointer -fno-inline Ditto for Clang, except that -fno-inline-functions is used instead of -fno-inline. cf. thread starting at https://sourceforge.net/p/flightgear/mailman/message/36295412/ --- CMakeLists.txt | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d9160c0f..6b21730c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -397,7 +397,12 @@ if(CMAKE_COMPILER_IS_GNUCXX) message(WARNING "GCC 4.4 will be required soon, please upgrade") endif() - if(ENABLE_SIMD) + if (CMAKE_BUILD_TYPE STREQUAL "Debug") + set(CMAKE_C_FLAGS + "${CMAKE_C_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline") + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline") + elseif (ENABLE_SIMD) if (X86 OR X86_64) set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse") set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse") @@ -420,7 +425,12 @@ if (CLANG) # fix Boost compilation :( set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++") - if(ENABLE_SIMD) + if (CMAKE_BUILD_TYPE STREQUAL "Debug") + set(CMAKE_C_FLAGS + "${CMAKE_C_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline-functions") + set(CMAKE_CXX_FLAGS + "${CMAKE_CXX_FLAGS} -O0 -fno-omit-frame-pointer -fno-inline-functions") + elseif (ENABLE_SIMD) if (X86 OR X86_64) set(CMAKE_C_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse") set(CMAKE_CXX_FLAGS_RELEASE "-O3 -msse2 -mfpmath=sse") From d85d85e7dc10e5f3e3470627b3937c747ddcc9f9 Mon Sep 17 00:00:00 2001 From: James Turner Date: Sun, 22 Apr 2018 19:28:00 +0200 Subject: [PATCH 2/4] Subsystem improvements for testing - track more meta-data and a factory function for each subsystem, registered either explicitly or via a helper static class. - add a delegate to receive notifications of subsystem changes - make sub-grouped subsystems work more naturally, especially for child lookups - add some test coverage for all of this --- simgear/structure/CMakeLists.txt | 4 + simgear/structure/subsystem_mgr.cxx | 617 +++++++++++++++++++++------ simgear/structure/subsystem_mgr.hxx | 288 +++++++++++-- simgear/structure/subsystem_test.cxx | 407 ++++++++++++++++++ 4 files changed, 1152 insertions(+), 164 deletions(-) create mode 100644 simgear/structure/subsystem_test.cxx diff --git a/simgear/structure/CMakeLists.txt b/simgear/structure/CMakeLists.txt index 7c7df4f3..5d81ad67 100644 --- a/simgear/structure/CMakeLists.txt +++ b/simgear/structure/CMakeLists.txt @@ -47,6 +47,10 @@ simgear_component(structure structure "${SOURCES}" "${HEADERS}") if(ENABLE_TESTS) +add_executable(test_subsystems subsystem_test.cxx) +target_link_libraries(test_subsystems ${TEST_LIBS}) +add_test(subsystems ${EXECUTABLE_OUTPUT_PATH}/test_subsystems) + add_executable(test_state_machine state_machine_test.cxx) target_link_libraries(test_state_machine ${TEST_LIBS}) add_test(state_machine ${EXECUTABLE_OUTPUT_PATH}/test_state_machine) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index eeda8663..da180d5b 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -18,9 +18,7 @@ // // $Id$ -#ifdef HAVE_CONFIG_H -# include -#endif +#include #include @@ -36,6 +34,7 @@ const int SG_MAX_SUBSYSTEM_EXCEPTIONS = 4; using std::string; +using State = SGSubsystem::State; //////////////////////////////////////////////////////////////////////// // Implementation of SGSubsystem @@ -45,7 +44,6 @@ SGSubsystemTimingCb SGSubsystem::reportTimingCb = NULL; void* SGSubsystem::reportTimingUserData = NULL; SGSubsystem::SGSubsystem () - : _suspended(false) { } @@ -93,19 +91,32 @@ SGSubsystem::unbind () void SGSubsystem::suspend () { - _suspended = true; + suspend(true); } void SGSubsystem::suspend (bool suspended) { - _suspended = suspended; + // important we don't use is_suspended() here since SGSubsystemGroup + // overries. We record the actual state correctly here, even for groups, + // so this works out, whereas is_suspended() is always false for groups + if (_suspended == suspended) + return; + + const auto newState = suspended ? State::SUSPEND : State::RESUME; + const auto manager = get_manager(); + + if (manager) + manager->notifyDelegatesWillChange(this, newState); + _suspended = suspended; + if (manager) + manager->notifyDelegatesDidChange(this, newState); } void SGSubsystem::resume () { - _suspended = false; + suspend(false); } bool @@ -119,6 +130,45 @@ void SGSubsystem::stamp(const string& name) timingInfo.push_back(TimingInfo(name, SGTimeStamp::now())); } +void SGSubsystem::set_name(const std::string &n) +{ + assert(_name.empty()); + _name = n; +} + +void SGSubsystem::set_group(SGSubsystemGroup* group) +{ + _group = group; +} + +SGSubsystemGroup* SGSubsystem::get_group() const +{ + return _group; +} + +SGSubsystemMgr* SGSubsystem::get_manager() const +{ + if (!get_group()) + throw sg_exception("SGSubsystem::get_manager: subsystem " + name() + " has no group"); + return get_group()->get_manager(); +} + +std::string SGSubsystem::nameForState(State s) +{ + switch (s) { + case State::INIT: return "init"; + case State::REINIT: return "reinit"; + case State::POSTINIT: return "post-init"; + case State::SHUTDOWN: return "shutdown"; + case State::BIND: return "bind"; + case State::UNBIND: return "unbind"; + case State::ADD: return "add"; + case State::REMOVE: return "remove"; + case State::SUSPEND: return "suspend"; + case State::RESUME: return "resume"; + } +} + //////////////////////////////////////////////////////////////////////// // Implementation of SGSubsystemGroup. //////////////////////////////////////////////////////////////////////// @@ -129,16 +179,16 @@ private: Member (const Member &member); public: Member (); - virtual ~Member (); + ~Member (); - virtual void update (double delta_time_sec); + void update (double delta_time_sec); void reportTiming(void) { if (reportTimingCb) reportTimingCb(reportTimingUserData, name, &timeStat); } void updateExecutionTime(double time) { timeStat += time;} SampleStatistic timeStat; std::string name; - SGSharedPtr subsystem; + SGSubsystemRef subsystem; double min_step_sec; double elapsed_sec; bool collectTimeStats; @@ -151,79 +201,127 @@ public: SGSubsystemGroup::SGSubsystemGroup () : _fixedUpdateTime(-1.0), _updateTimeRemainder(0.0), - _initPosition(0) + _initPosition(-1) { } SGSubsystemGroup::~SGSubsystemGroup () { - // reverse order to prevent order dependency problems - for( size_t i = _members.size(); i > 0; i-- ) - { - delete _members[i-1]; - } + clearSubsystems(); } void SGSubsystemGroup::init () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->init(); + forEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::INIT); + s->init(); + this->notifyDidChange(s, State::INIT); + }); } SGSubsystem::InitStatus SGSubsystemGroup::incrementalInit() { - if (_initPosition >= _members.size()) - return INIT_DONE; + // special case this, simplifies the logic below + if (_members.empty()) { + return INIT_DONE; + } + + // termination test + if (_initPosition >= static_cast(_members.size())) { + return INIT_DONE; + } - SGTimeStamp st; - st.stamp(); - InitStatus memberStatus = _members[_initPosition]->subsystem->incrementalInit(); - _members[_initPosition]->initTime += st.elapsedMSec(); + if (_initPosition < 0) { + // first call + _initPosition = 0; + notifyWillChange(_members.front()->subsystem, State::INIT); + } + + const auto m = _members[_initPosition]; + SGTimeStamp st; + st.stamp(); + const InitStatus memberStatus = m->subsystem->incrementalInit(); + m->initTime += st.elapsedMSec(); - if (memberStatus == INIT_DONE) - ++_initPosition; + if (memberStatus == INIT_DONE) { + // complete init of this one + notifyDidChange(m->subsystem, State::INIT); + ++_initPosition; + + if (_initPosition < _members.size()) { + // start init of the next one + notifyWillChange( _members[_initPosition]->subsystem, State::INIT); + } + } return INIT_CONTINUE; } +void SGSubsystemGroup::forEach(std::function f) +{ + for (auto m : _members) { + f(m->subsystem); + } +} + +void SGSubsystemGroup::reverseForEach(std::function f) +{ + for (auto it = _members.rbegin(); it != _members.rend(); ++it) { + f((*it)->subsystem); + } +} + void SGSubsystemGroup::postinit () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->postinit(); + forEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::POSTINIT); + s->postinit(); + this->notifyDidChange(s, State::POSTINIT); + }); } void SGSubsystemGroup::reinit () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->reinit(); + forEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::REINIT); + s->reinit(); + this->notifyDidChange(s, State::REINIT); + }); } void SGSubsystemGroup::shutdown () { - // reverse order to prevent order dependency problems - for( size_t i = _members.size(); i > 0; i-- ) - _members[i-1]->subsystem->shutdown(); - _initPosition = 0; + reverseForEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::SHUTDOWN); + s->shutdown(); + this->notifyDidChange(s, State::SHUTDOWN); + }); + _initPosition = -1; } void SGSubsystemGroup::bind () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->bind(); + forEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::BIND); + s->bind(); + this->notifyDidChange(s, State::BIND); + }); } void SGSubsystemGroup::unbind () { - // reverse order to prevent order dependency problems - for( size_t i = _members.size(); i > 0; i-- ) - _members[i-1]->subsystem->unbind(); + reverseForEach([this](SGSubsystem* s){ + this->notifyWillChange(s, State::UNBIND); + s->unbind(); + this->notifyDidChange(s, State::UNBIND); + }); } void @@ -241,20 +339,18 @@ SGSubsystemGroup::update (double delta_time_sec) delta_time_sec = _fixedUpdateTime; } - bool recordTime = (reportTimingCb != NULL); + const bool recordTime = (reportTimingCb != nullptr); SGTimeStamp timeStamp; while (loopCount-- > 0) { - for( size_t i = 0; i < _members.size(); i++ ) - { + for (auto member : _members) { if (recordTime) timeStamp = SGTimeStamp::now(); - _members[i]->update(delta_time_sec); // indirect call + member->update(delta_time_sec); // indirect call - if ((recordTime)&&(reportTimingCb)) - { + if (recordTime && reportTimingCb) { timeStamp = SGTimeStamp::now() - timeStamp; - _members[i]->updateExecutionTime(timeStamp.toUSecs()); + member->updateExecutionTime(timeStamp.toUSecs()); } } } // of multiple update loop @@ -263,39 +359,42 @@ SGSubsystemGroup::update (double delta_time_sec) void SGSubsystemGroup::reportTiming(void) { - for( size_t i = _members.size(); i > 0; i-- ) - { - _members[i-1]->reportTiming(); + for (auto member : _members) { + member->reportTiming(); } } void SGSubsystemGroup::suspend () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->suspend(); + SGSubsystem::suspend(); + forEach([](SGSubsystem* s) { s->suspend(); }); } void SGSubsystemGroup::resume () { - for( size_t i = 0; i < _members.size(); i++ ) - _members[i]->subsystem->resume(); + forEach([](SGSubsystem* s) { s->resume(); }); + SGSubsystem::resume(); } string_list SGSubsystemGroup::member_names() const { string_list result; - for( size_t i = 0; i < _members.size(); i++ ) - result.push_back( _members[i]->name ); - + for (auto member : _members) { + result.push_back(member->name); + } + return result; } bool SGSubsystemGroup::is_suspended () const { + // important so suspended groups still count dt for their members + // but this does mean we need to be careful when notifying suspend + // and resume on groups return false; } @@ -303,45 +402,112 @@ void SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, double min_step_sec) { - Member * member = get_member(name, true); + if (name.empty()) { + SG_LOG(SG_GENERAL, SG_DEV_WARN, "adding subsystem to group without a name"); + // TODO, make this an exception in the future + } else if (name != subsystem->name()) { + SG_LOG(SG_GENERAL, SG_DEV_WARN, "adding subsystem to group with name '" << name + << "', but name() returns '" << subsystem->name() << "'"); + } + + notifyWillChange(subsystem, State::ADD); + Member* member = get_member(name, true); member->name = name; member->subsystem = subsystem; member->min_step_sec = min_step_sec; + subsystem->set_group(this); + notifyDidChange(subsystem, State::ADD); +} + +void +SGSubsystemGroup::set_subsystem (SGSubsystem * subsystem, double min_step_sec) +{ + set_subsystem(subsystem->name(), subsystem, min_step_sec); } SGSubsystem * SGSubsystemGroup::get_subsystem (const string &name) { - Member * member = get_member(name); - if (member != 0) + if (has_subsystem(name)) { + const Member* member = get_member(name, false); return member->subsystem; - else - return 0; -} - -void -SGSubsystemGroup::remove_subsystem (const string &name) -{ - MemberVec::iterator it = _members.begin(); - for (; it != _members.end(); ++it) { - if (name == (*it)->name) { - delete *it; - _members.erase(it); - return; + } + + // recursive search + for (const auto& m : _members) { + if (m->subsystem->is_group()) { + auto s = static_cast(m->subsystem.ptr())->get_subsystem(name); + if (s) + return s; } } - SG_LOG(SG_GENERAL, SG_WARN, "remove_subsystem: missing:" << name); + return {}; +} + +bool +SGSubsystemGroup::remove_subsystem(const string &name) +{ + // direct membership + auto it = std::find_if(_members.begin(), _members.end(), [name](const Member* m) + { return m->name == name; }); + if (it != _members.end()) { + // found it, great + const auto sub = (*it)->subsystem; + notifyWillChange(sub, State::REMOVE); + delete *it; + _members.erase(it); + notifyDidChange(sub, State::REMOVE); + return true; + } + + // recursive removal + for (const auto& m : _members) { + if (m->subsystem->is_group()) { + auto group = static_cast(m->subsystem.ptr()); + bool ok = group->remove_subsystem(name); + if (ok) { + return true; + } + } + } + + return false; +} + +void +SGSubsystemGroup::notifyWillChange(SGSubsystem* sub, SGSubsystemMgr::State s) +{ + auto manager = get_manager(); + if (manager) { + manager->notifyDelegatesWillChange(sub, s); + } +} + +void +SGSubsystemGroup::notifyDidChange(SGSubsystem* sub, SGSubsystemMgr::State s) +{ + auto manager = get_manager(); + if (manager) { + manager->notifyDelegatesDidChange(sub, s); + } } //------------------------------------------------------------------------------ void SGSubsystemGroup::clearSubsystems() { - for( MemberVec::iterator it = _members.begin(); - it != _members.end(); - ++it ) - delete *it; - _members.clear(); + // reverse order to prevent order dependency problems + for (auto it = _members.rbegin(); it != _members.rend(); ++it) + { + // hold a ref here in case (as is very likely) this is the + // final ref to the subsystem, so that we can notify-did-change safely + SGSubsystemRef sub = (*it)->subsystem; + notifyWillChange(sub, State::REMOVE); + delete *it; + notifyDidChange(sub, State::REMOVE); + } + + _members.clear(); } void @@ -353,25 +519,42 @@ SGSubsystemGroup::set_fixed_update_time(double dt) bool SGSubsystemGroup::has_subsystem (const string &name) const { - return (((SGSubsystemGroup *)this)->get_member(name) != 0); + auto it = std::find_if(_members.begin(), _members.end(), [name](const Member* m) + { return m->name == name; }); + return it != _members.end(); } -SGSubsystemGroup::Member * -SGSubsystemGroup::get_member (const string &name, bool create) +auto SGSubsystemGroup::get_member(const string &name, bool create) -> Member* { - for( size_t i = 0; i < _members.size(); i++ ) { - if (_members[i]->name == name) - return _members[i]; - } - if (create) { - Member * member = new Member; - _members.push_back(member); - return member; - } else { - return 0; + auto it = std::find_if(_members.begin(), _members.end(), [name](const Member* m) + { return m->name == name; }); + if (it == _members.end()) { + if (!create) + return nullptr; + + Member* m = new Member; + m->name = name; + _members.push_back(m); + return _members.back(); } + + return *it; } +SGSubsystemMgr* SGSubsystemGroup::get_manager() const +{ + auto parentGroup = get_group(); + if (parentGroup) { + return parentGroup->get_manager(); + } + + return _manager; +} + +void SGSubsystemGroup::set_manager(SGSubsystemMgr *manager) +{ + _manager = manager; +} //////////////////////////////////////////////////////////////////////// // Implementation of SGSubsystemGroup::Member @@ -431,18 +614,18 @@ SGSubsystemGroup::Member::update (double delta_time_sec) SGSubsystemMgr::SGSubsystemMgr () : - _groups(MAX_GROUPS), - _initPosition(0) + _groups(MAX_GROUPS) { - for (int i = 0; i < MAX_GROUPS; i++) - _groups[i].reset(new SGSubsystemGroup); + for (int i = 0; i < MAX_GROUPS; i++) { + auto g = new SGSubsystemGroup; + g->set_manager(this); + _groups[i].reset(g); + } } SGSubsystemMgr::~SGSubsystemMgr () { - // ensure get_subsystem returns NULL from now onwards, - // before the SGSubsystemGroup destructors are run - _subsystem_map.clear(); + _destructorActive = true; _groups.clear(); } @@ -538,33 +721,31 @@ void SGSubsystemMgr::add (const char * name, SGSubsystem * subsystem, GroupType group, double min_time_sec) { + if (get_subsystem(name) != nullptr) + throw sg_exception("Duplicate add of subsystem: " + std::string(name)); + SG_LOG(SG_GENERAL, SG_DEBUG, "Adding subsystem " << name); get_group(group)->set_subsystem(name, subsystem, min_time_sec); - - if (_subsystem_map.find(name) != _subsystem_map.end()) { - SG_LOG(SG_GENERAL, SG_ALERT, "Adding duplicate subsystem " << name); - throw sg_exception("duplicate subsystem:" + std::string(name)); - } - _subsystem_map[name] = subsystem; } -void +bool SGSubsystemMgr::remove(const char* name) { - SubsystemDict::iterator s =_subsystem_map.find(name); - if (s == _subsystem_map.end()) { - return; - } + // drop the cache + _subsystemNameCache.clear(); - _subsystem_map.erase(s); - -// tedious part - we don't know which group the subsystem belongs too - for (int i = 0; i < MAX_GROUPS; i++) { - if (_groups[i]->get_subsystem(name) != NULL) { - _groups[i]->remove_subsystem(name); - break; - } - } // of groups iteration +// we don't know which group the subsystem belongs too +// fortunately this is a very infrequently used code path, so the slow +// search is not a problem + for (auto group : _groups) { + bool didRemove = group->remove_subsystem(name); + if (didRemove) { + return true; + } + } // of groups iteration + + SG_LOG(SG_GENERAL, SG_WARN, "SGSubsystemMgr::remove: not found: " << name); + return false; } @@ -577,14 +758,34 @@ SGSubsystemMgr::get_group (GroupType group) SGSubsystem * SGSubsystemMgr::get_subsystem (const string &name) const { - SubsystemDict::const_iterator s =_subsystem_map.find(name); - - if (s == _subsystem_map.end()) - return 0; - else + if (_destructorActive) + return nullptr; + + auto s =_subsystemNameCache.find(name); + if (s != _subsystemNameCache.end()) { + // in the cache, excellent return s->second; + } + + for (auto g : _groups) { + auto sub = g->get_subsystem(name); + if (sub) { + // insert into the cache + _subsystemNameCache[name] = sub; + return sub; + } + } + + return nullptr; } +SGSubsystem* +SGSubsystemMgr::get_subsystem(const std::string &name, const std::string& instanceName) const +{ + return get_subsystem(name + "-" + instanceName); +} + + /** Trigger the timing callback to report data for all subsystems. */ void SGSubsystemMgr::reportTiming() @@ -594,4 +795,174 @@ SGSubsystemMgr::reportTiming() } // of groups iteration } +// anonymous namespace to hold registration informatipno + +namespace { + struct RegisteredSubsystemData + { + RegisteredSubsystemData(const std::string& aName, bool aInstanced, + SGSubsystemMgr::SubsystemFactoryFunctor aFunctor, + SGSubsystemMgr::GroupType aGroup, + double aInterval) : + name(aName), + instanced(aInstanced), + functor(aFunctor), + defaultGroup(aGroup), + defaultUpdateInterval(aInterval) + { + } + + std::string name; + bool instanced = false; + SGSubsystemMgr::SubsystemFactoryFunctor functor; + SGSubsystemMgr::GroupType defaultGroup; + double defaultUpdateInterval = 0.0; + + SGSubsystemMgr::DependencyVec depends; + }; + + using SybsystemRegistrationVec = std::vector; + + SybsystemRegistrationVec global_registrations; + + SybsystemRegistrationVec::const_iterator findRegistration(const std::string& name) + { + auto it = std::find_if(global_registrations.begin(), + global_registrations.end(), + [name](const RegisteredSubsystemData& d) + { return name == d.name; }); + return it; + } +} // of anonymous namespace + +void SGSubsystemMgr::registerSubsystem(const std::string& name, + SubsystemFactoryFunctor f, + GroupType group, + bool instanced, + double updateInterval, + std::initializer_list deps) +{ + if (findRegistration(name) != global_registrations.end()) { + throw sg_exception("duplicate subsystem registration for: " + name); + } + + global_registrations.push_back({name, instanced, f, group, updateInterval}); + if (deps.size() > 0) { + global_registrations.back().depends = deps; + } +} + +auto SGSubsystemMgr::defaultGroupFor(const char* name) -> GroupType +{ + auto it = findRegistration(name); + if (it == global_registrations.end()) { + throw sg_exception("unknown subsystem registration for: " + std::string(name)); + } + + return it->defaultGroup; +} + +double SGSubsystemMgr::defaultUpdateIntervalFor(const char* name) +{ + auto it = findRegistration(name); + if (it == global_registrations.end()) { + throw sg_exception("unknown subsystem registration for: " + std::string(name)); + } + + return it->defaultUpdateInterval; +} + +const SGSubsystemMgr::DependencyVec& +SGSubsystemMgr::dependsFor(const char* name) +{ + auto it = findRegistration(name); + if (it == global_registrations.end()) { + throw sg_exception("unknown subsystem registration for: " + std::string(name)); + } + + return it->depends; +} + +SGSubsystemRef +SGSubsystemMgr::create(const std::string& name) +{ + auto it = findRegistration(name); + if (it == global_registrations.end()) { + return {}; // or should this throw with a 'not registered'? + } + + if (it->instanced) { + throw sg_exception("SGSubsystemMgr::create: using non-instanced mode for instanced subsytem"); + } + + SGSubsystemRef ref = it->functor(); + if (!ref) { + throw sg_exception("SGSubsystemMgr::create: functor failed to create subsystem implementation: " + name); + } + + ref->set_name(name); + return ref; +} + +SGSubsystemRef +SGSubsystemMgr::createInstance(const std::string& name, const std::string& instanceName) +{ + auto it = findRegistration(name); + if (it == global_registrations.end()) { + return {}; // or should this throw with a 'not registered'? + } + + if (!it->instanced) { + throw sg_exception("SGSubsystemMgr::create: using instanced mode for non-instanced subsytem"); + } + + SGSubsystemRef ref = it->functor(); + if (!ref) { + throw sg_exception("SGSubsystemMgr::create: functor failed to create an instsance of " + name); + } + + const auto combinedName = name + "-" + instanceName; + ref->set_name(combinedName); + return ref; +} + +void SGSubsystemMgr::Delegate::willChange(SGSubsystem*, State) +{ +} + +void SGSubsystemMgr::Delegate::didChange(SGSubsystem*, State) +{ +} + +void SGSubsystemMgr::addDelegate(Delegate * d) +{ + assert(d); + _delegates.push_back(d); +} + +void SGSubsystemMgr::removeDelegate(Delegate * d) +{ + assert(d); + auto it = std::find(_delegates.begin(), _delegates.end(), d); + if (it == _delegates.end()) { + SG_LOG(SG_GENERAL, SG_DEV_ALERT, "removeDelegate: unknown delegate"); + return; + } + + _delegates.erase(it); +} + +void SGSubsystemMgr::notifyDelegatesWillChange(SGSubsystem* sub, State newState) +{ + std::for_each(_delegates.begin(), _delegates.end(), [sub, newState](Delegate* d) + { d->willChange(sub, newState); }); +} + +void SGSubsystemMgr::notifyDelegatesDidChange(SGSubsystem* sub, State state) +{ + std::for_each(_delegates.begin(), _delegates.end(), [sub, state](Delegate* d) + { d->didChange(sub, state); }); +} + + // end of subsystem_mgr.cxx diff --git a/simgear/structure/subsystem_mgr.hxx b/simgear/structure/subsystem_mgr.hxx index 1142e366..6b9d5888 100644 --- a/simgear/structure/subsystem_mgr.hxx +++ b/simgear/structure/subsystem_mgr.hxx @@ -47,7 +47,10 @@ public: const SGTimeStamp& getTime() const { return time; } }; +// forward decls class SampleStatistic; +class SGSubsystemGroup; +class SGSubsystemMgr; typedef std::vector eventTimeVec; typedef std::vector::iterator eventTimeVecIterator; @@ -125,7 +128,6 @@ typedef void (*SGSubsystemTimingCb)(void* userData, const std::string& name, Sam class SGSubsystem : public SGReferenced { public: - /** * Default constructor. */ @@ -272,14 +274,52 @@ public: */ void stamp(const std::string& name); -protected: + std::string name() const + { return _name; } - bool _suspended; + virtual bool is_group() const + { return false; } + + virtual SGSubsystemMgr* get_manager() const; + + /// get the parent group of this subsystem + SGSubsystemGroup* get_group() const; + + enum class State { + ADD, + BIND, + INIT, + POSTINIT, + REINIT, + SUSPEND, + RESUME, + UNBIND, + SHUTDOWN, + REMOVE + }; + + /** + * debug helper, print a state as a string + */ + static std::string nameForState(State s); +protected: + friend class SGSubsystemMgr; + friend class SGSubsystemGroup; + + void set_name(const std::string& n); + + void set_group(SGSubsystemGroup* group); + + std::string _name; + bool _suspended = false; eventTimeVec timingInfo; static SGSubsystemTimingCb reportTimingCb; static void* reportTimingUserData; + +private: + SGSubsystemGroup* _group = nullptr; }; typedef SGSharedPtr SGSubsystemRef; @@ -290,27 +330,29 @@ typedef SGSharedPtr SGSubsystemRef; class SGSubsystemGroup : public SGSubsystem { public: - SGSubsystemGroup (); virtual ~SGSubsystemGroup (); - virtual void init(); - virtual InitStatus incrementalInit (); - virtual void postinit (); - virtual void reinit (); - virtual void shutdown (); - virtual void bind (); - virtual void unbind (); - virtual void update (double delta_time_sec); - virtual void suspend (); - virtual void resume (); - virtual bool is_suspended () const; + void init() override; + InitStatus incrementalInit () override; + void postinit () override; + void reinit () override; + void shutdown () override; + void bind () override; + void unbind () override; + void update (double delta_time_sec) override; + void suspend () override; + void resume () override; + bool is_suspended () const override; virtual void set_subsystem (const std::string &name, SGSubsystem * subsystem, double min_step_sec = 0); + + void set_subsystem (SGSubsystem * subsystem, double min_step_sec = 0); + virtual SGSubsystem * get_subsystem (const std::string &name); - virtual void remove_subsystem (const std::string &name); + bool remove_subsystem (const std::string &name); virtual bool has_subsystem (const std::string &name) const; /** @@ -335,19 +377,37 @@ public: { return dynamic_cast(get_subsystem(T::subsystemName())); } -private: + bool is_group() const override + { return true; } + + SGSubsystemMgr* get_manager() const override; +private: + void forEach(std::function f); + void reverseForEach(std::function f); + + void notifyWillChange(SGSubsystem* sub, SGSubsystem::State s); + void notifyDidChange(SGSubsystem* sub, SGSubsystem::State s); + + friend class SGSubsystemMgr; + + void set_manager(SGSubsystemMgr* manager); + class Member; Member* get_member (const std::string &name, bool create = false); - - typedef std::vector MemberVec; + + using MemberVec = std::vector; MemberVec _members; double _fixedUpdateTime; double _updateTimeRemainder; /// index of the member we are currently init-ing - unsigned int _initPosition; + int _initPosition; + + /// back-pointer to the manager, for the root groups. (sub-groups + /// will have this as null, and chain via their parent) + SGSubsystemMgr* _manager = nullptr; }; typedef SGSharedPtr SGSubsystemGroupRef; @@ -377,6 +437,7 @@ public: * Types of subsystem groups. */ enum GroupType { + INVALID = -1, INIT = 0, GENERAL, FDM, ///< flight model, autopilot, instruments that run coupled @@ -389,17 +450,17 @@ public: SGSubsystemMgr (); virtual ~SGSubsystemMgr (); - virtual void init (); - virtual InitStatus incrementalInit (); - virtual void postinit (); - virtual void reinit (); - virtual void shutdown (); - virtual void bind (); - virtual void unbind (); - virtual void update (double delta_time_sec); - virtual void suspend (); - virtual void resume (); - virtual bool is_suspended () const; + void init () override; + InitStatus incrementalInit () override; + void postinit () override; + void reinit () override; + void shutdown () override; + void bind () override; + void unbind () override; + void update (double delta_time_sec) override; + void suspend () override; + void resume () override; + bool is_suspended () const override; virtual void add (const char * name, SGSubsystem * subsystem, @@ -407,14 +468,16 @@ public: double min_time_sec = 0); /** - * remove a subsystem, and return a pointer to it. - * returns NULL if the subsystem was not found. + * remove a subsystem, and return true on success + * returns false if the subsystem was not found */ - virtual void remove(const char* name); + bool remove(const char* name); virtual SGSubsystemGroup * get_group (GroupType group); - virtual SGSubsystem* get_subsystem(const std::string &name) const; + SGSubsystem* get_subsystem(const std::string &name) const; + + SGSubsystem* get_subsystem(const std::string &name, const std::string& instanceName) const; void reportTiming(); void setReportTimingCb(void* userData,SGSubsystemTimingCb cb) {reportTimingCb = cb;reportTimingUserData = userData;} @@ -425,13 +488,156 @@ public: return dynamic_cast(get_subsystem(T::subsystemName())); } -private: - std::vector _groups; - unsigned int _initPosition; +// instanced overloads, for both raw char* and std::string +// these concatenate the subsystem type name with the instance name + template + T* get_subsystem(const char* instanceName) const + { + return dynamic_cast(get_subsystem(T::subsystemName(), instanceName)); + } - // non-owning reference - typedef std::map SubsystemDict; - SubsystemDict _subsystem_map; + template + T* get_subsystem(const std::string& instanceName) const + { + return dynamic_cast(get_subsystem(T::subsystemName(), instanceName)); + } + + struct Dependency { + enum Type { + HARD, + SOFT, + PROPERTY + }; + + std::string name; + Type type; + }; + + using DependencyVec = std::vector; + using SubsystemFactoryFunctor = std::function; + + /** + * @brief register a subsytem with the manager + * + */ + static void registerSubsystem(const std::string& name, + SubsystemFactoryFunctor f, + GroupType group, + bool isInstanced = false, + double updateInterval = 0.0, + std::initializer_list deps = {}); + + template + class Registrant + { + public: + Registrant(GroupType group = GENERAL, double updateInterval = 0.0, + std::initializer_list deps = {}) + { + SubsystemFactoryFunctor factory = [](){ return new T; }; + SGSubsystemMgr::registerSubsystem(T::subsystemName(), + factory, group, + false, updateInterval, + deps); + } + + // could implement a dtor to unregister but not needed at the moment + }; + + template + class InstancedRegistrant + { + public: + InstancedRegistrant(GroupType group = GENERAL, + double updateInterval = 0.0, + std::initializer_list deps = {}) + { + SubsystemFactoryFunctor factory = [](){ return new T; }; + SGSubsystemMgr::registerSubsystem(T::subsystemName(), + factory, group, + true, updateInterval, + deps); + } + + // could implement a dtor to unregister but not needed at the moment + }; + + /** + * @brief templated add function, subsystem is deduced automatically + * + */ + template + SGSharedPtr add(GroupType customGroup = INVALID, double customInterval = 0.0) + { + auto ref = create(T::subsystemName()); + + + const GroupType group = (customGroup == INVALID) ? + defaultGroupFor(T::subsystemName()) : customGroup; + const double interval = (customInterval == 0.0) ? + defaultUpdateIntervalFor(T::subsystemName()) : customInterval; + add(ref->name().c_str(), ref.ptr(), group, interval); + return dynamic_cast(ref.ptr()); + } + + /** + * @brief templated creation function, only makes the instance but + * doesn't add to the manager or group heirarchy + */ + template + SGSharedPtr create() + { + auto ref = create(T::subsystemName()); + return dynamic_cast(ref.ptr()); + } + + SGSubsystemRef create(const std::string& name); + + template + SGSharedPtr createInstance(const std::string& instanceName) + { + auto ref = createInstance(T::subsystemName(), instanceName); + return dynamic_cast(ref.ptr()); + } + + SGSubsystemRef createInstance(const std::string& name, const std::string& instanceName); + + static GroupType defaultGroupFor(const char* name); + static double defaultUpdateIntervalFor(const char* name); + static const DependencyVec& dependsFor(const char* name); + + /** + * @brief delegate to recieve notifications when the subsystem + * configuration changes. For any event/state change, a before (will) + * and after (did) change notifications are sent. + */ + class Delegate + { + public: + virtual void willChange(SGSubsystem* sub, SGSubsystem::State newState); + virtual void didChange(SGSubsystem* sub, SGSubsystem::State currentState); + }; + + void addDelegate(Delegate * d); + void removeDelegate(Delegate * d); +private: + friend class SGSubsystem; + friend class SGSubsystemGroup; + + void notifyDelegatesWillChange(SGSubsystem* sub, State newState); + void notifyDelegatesDidChange(SGSubsystem* sub, State statee); + + std::vector _groups; + unsigned int _initPosition = 0; + bool _destructorActive = false; + + // non-owning reference, this is to accelerate lookup + // by name which otherwise needs a full walk of the entire tree + using SubsystemDict = std::map; + mutable SubsystemDict _subsystemNameCache; + + using DelegateVec = std::vector; + DelegateVec _delegates; }; #endif // __SUBSYSTEM_MGR_HXX diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx new file mode 100644 index 00000000..fb27a664 --- /dev/null +++ b/simgear/structure/subsystem_test.cxx @@ -0,0 +1,407 @@ +#include + +#include + +#include +#include +#include +#include + +using std::string; +using std::cout; +using std::cerr; +using std::endl; + +/////////////////////////////////////////////////////////////////////////////// +// sample subsystems + +class MySub1 : public SGSubsystem +{ +public: + static const char* subsystemName() { return "mysub"; } + + void init() override + { + wasInited = true; + } + + void update(double dt) override + { + + } + + bool wasInited = false; +}; + +class AnotherSub : public SGSubsystem +{ +public: + static const char* subsystemName() { return "anothersub"; } + + void init() override + { + + } + + void update(double dt) override + { + lastUpdateTime = dt; + } + + double lastUpdateTime = 0.0; +}; + +class FakeRadioSub : public SGSubsystem +{ +public: + static const char* subsystemName() { return "fake-radio"; } + + void init() override + { + wasInited = true; + } + + void update(double dt) override + { + lastUpdateTime = dt; + } + + bool wasInited = false; + double lastUpdateTime = 0.0; +}; + +class InstrumentGroup : public SGSubsystemGroup +{ +public: + static const char* subsystemName() { return "instruments"; } + + virtual ~InstrumentGroup() + { + } + + void init() override + { + wasInited = true; + SGSubsystemGroup::init(); + } + + void update(double dt) override + { + lastUpdateTime = dt; + SGSubsystemGroup::update(dt); + } + + bool wasInited = false; + double lastUpdateTime = 0.0; +}; + +/////////////////////////////////////////////////////////////////////////////// +// sample delegate + +class RecorderDelegate : public SGSubsystemMgr::Delegate +{ +public: + void willChange(SGSubsystem* sub, SGSubsystem::State newState) override + { + events.push_back({sub->name(), false, newState}); + } + void didChange(SGSubsystem* sub, SGSubsystem::State newState) override + { + events.push_back({sub->name(), true, newState}); + } + + struct Event { + string subsystem; + bool didChange; + SGSubsystem::State event; + + std::string nameForEvent() const + { + return subsystem + (didChange ? "-did-" : "-will-") + SGSubsystem::nameForState(event); + } + }; + + using EventVec = std::vector; + EventVec events; + + EventVec::const_iterator findEvent(const std::string& name) const + { + auto it = std::find_if(events.begin(), events.end(), [name](const Event& ev) + { return ev.nameForEvent() == name; }); + return it; + } + + bool hasEvent(const std::string& name) const + { + return findEvent(name) != events.end(); + } +}; + +/////////////////////////////////////////////////////////////////////////////// + +SGSubsystemMgr::Registrant registrant(SGSubsystemMgr::GENERAL); +SGSubsystemMgr::Registrant registrant2(SGSubsystemMgr::FDM); + +SGSubsystemMgr::Registrant registrant4(SGSubsystemMgr::FDM); + +SGSubsystemMgr::InstancedRegistrant registrant3(SGSubsystemMgr::POST_FDM); + +void testRegistrationAndCreation() +{ + SGSharedPtr manager = new SGSubsystemMgr; + + auto anotherSub = manager->create(); + SG_VERIFY(anotherSub); + SG_CHECK_EQUAL(anotherSub->name(), AnotherSub::subsystemName()); + SG_CHECK_EQUAL(anotherSub->name(), std::string("anothersub")); + + auto radio1 = manager->createInstance("nav1"); + auto radio2 = manager->createInstance("nav2"); + + +} + +void testAddGetRemove() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto anotherSub = manager->add(); + SG_VERIFY(anotherSub); + SG_CHECK_EQUAL(anotherSub->name(), AnotherSub::subsystemName()); + SG_CHECK_EQUAL(anotherSub->name(), std::string("anothersub")); + + SG_VERIFY(d->hasEvent("anothersub-will-add")); + SG_VERIFY(d->hasEvent("anothersub-did-add")); + + auto lookup = manager->get_subsystem(); + SG_CHECK_EQUAL(lookup, anotherSub); + + SG_CHECK_EQUAL(manager->get_subsystem("anothersub"), anotherSub); + + // manual create & add + auto mySub = manager->create(); + manager->add(MySub1::subsystemName(), mySub.ptr(), SGSubsystemMgr::DISPLAY, 0.1234); + + SG_VERIFY(d->hasEvent("mysub-will-add")); + SG_VERIFY(d->hasEvent("mysub-did-add")); + + SG_CHECK_EQUAL(manager->get_subsystem(), mySub); + + bool ok = manager->remove(AnotherSub::subsystemName()); + SG_VERIFY(ok); + SG_VERIFY(d->hasEvent("anothersub-will-remove")); + SG_VERIFY(d->hasEvent("anothersub-did-remove")); + + SG_VERIFY(manager->get_subsystem() == nullptr); + + // lookup after remove + SG_CHECK_EQUAL(manager->get_subsystem(), mySub); + + // re-add of removed, and let's test overriding + auto another2 = manager->add(SGSubsystemMgr::SOUND); + SG_CHECK_EQUAL(another2->name(), AnotherSub::subsystemName()); + + auto soundGroup = manager->get_group(SGSubsystemMgr::SOUND); + SG_CHECK_EQUAL(soundGroup->get_subsystem("anothersub"), another2); + +} + +void testSubGrouping() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto anotherSub = manager->add(); + auto instruments = manager->add(); + + auto radio1 = manager->createInstance("nav1"); + auto radio2 = manager->createInstance("nav2"); + + SG_CHECK_EQUAL(radio1->name(), std::string("fake-radio-nav1")); + SG_CHECK_EQUAL(radio2->name(), std::string("fake-radio-nav2")); + + instruments->set_subsystem(radio1); + instruments->set_subsystem(radio2); + + SG_VERIFY(d->hasEvent("fake-radio-nav1-did-add")); + SG_VERIFY(d->hasEvent("fake-radio-nav1-will-add")); + + // lookup of the group should also work + SG_CHECK_EQUAL(manager->get_subsystem(), instruments); + + manager->init(); + + SG_VERIFY(instruments->wasInited); + SG_VERIFY(radio1->wasInited); + SG_VERIFY(radio2->wasInited); + + SG_VERIFY(d->hasEvent("instruments-will-init")); + SG_VERIFY(d->hasEvent("instruments-did-init")); + SG_VERIFY(d->hasEvent("fake-radio-nav1-will-init")); + SG_VERIFY(d->hasEvent("fake-radio-nav2-did-init")); + + manager->update(0.5); + SG_CHECK_EQUAL_EP(0.5, instruments->lastUpdateTime); + SG_CHECK_EQUAL_EP(0.5, radio1->lastUpdateTime); + SG_CHECK_EQUAL_EP(0.5, radio2->lastUpdateTime); + + SG_CHECK_EQUAL(radio1, instruments->get_subsystem("fake-radio-nav1")); + SG_CHECK_EQUAL(radio2, instruments->get_subsystem("fake-radio-nav2")); + + // type-safe lookup of instanced + SG_CHECK_EQUAL(radio1, manager->get_subsystem("nav1")); + SG_CHECK_EQUAL(radio2, manager->get_subsystem("nav2")); + + bool ok = manager->remove("fake-radio-nav2"); + SG_VERIFY(ok); + SG_VERIFY(instruments->get_subsystem("fake-radio-nav2") == nullptr); + + manager->update(1.0); + SG_CHECK_EQUAL_EP(1.0, instruments->lastUpdateTime); + SG_CHECK_EQUAL_EP(1.0, radio1->lastUpdateTime); + + // should not have been updated + SG_CHECK_EQUAL_EP(0.5, radio2->lastUpdateTime); + + manager->unbind(); + SG_VERIFY(d->hasEvent("instruments-will-unbind")); + SG_VERIFY(d->hasEvent("instruments-did-unbind")); + SG_VERIFY(d->hasEvent("fake-radio-nav1-will-unbind")); + +} + +void testIncrementalInit() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + // place everything into the same group, so incremental init has + // some work to do + auto mySub = manager->add(SGSubsystemMgr::POST_FDM); + auto anotherSub = manager->add(SGSubsystemMgr::POST_FDM); + auto instruments = manager->add(SGSubsystemMgr::POST_FDM); + + auto radio1 = manager->createInstance("nav1"); + auto radio2 = manager->createInstance("nav2"); + instruments->set_subsystem(radio1); + instruments->set_subsystem(radio2); + + for ( ; ; ) { + auto status = manager->incrementalInit(); + if (status == SGSubsystemMgr::INIT_DONE) + break; + } + + for (auto ev : d->events) { + std::cerr << "ev:" << ev.nameForEvent() << std::endl; + } + + SG_VERIFY(mySub->wasInited); + + SG_VERIFY(d->hasEvent("mysub-will-init")); + SG_VERIFY(d->hasEvent("mysub-did-init")); + + SG_VERIFY(d->hasEvent("anothersub-will-init")); + SG_VERIFY(d->hasEvent("anothersub-did-init")); + + // SG_VERIFY(d->hasEvent("instruments-will-init")); + // SG_VERIFY(d->hasEvent("instruments-did-init")); + + + SG_VERIFY(d->hasEvent("fake-radio-nav1-will-init")); + SG_VERIFY(d->hasEvent("fake-radio-nav1-did-init")); + + SG_VERIFY(d->hasEvent("fake-radio-nav2-will-init")); + SG_VERIFY(d->hasEvent("fake-radio-nav2-did-init")); + + + +} + +void testSuspendResume() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto anotherSub = manager->add(); + auto instruments = manager->add(); + + auto radio1 = manager->createInstance("nav1"); + auto radio2 = manager->createInstance("nav2"); + + instruments->set_subsystem(radio1); + instruments->set_subsystem(radio2); + + manager->init(); + manager->update(1.0); + + SG_CHECK_EQUAL_EP(1.0, anotherSub->lastUpdateTime); + SG_CHECK_EQUAL_EP(1.0, instruments->lastUpdateTime); + SG_CHECK_EQUAL_EP(1.0, radio1->lastUpdateTime); + + anotherSub->suspend(); + radio1->resume(); // should be a no-op + + SG_VERIFY(d->hasEvent("anothersub-will-suspend")); + SG_VERIFY(d->hasEvent("anothersub-did-suspend")); + SG_VERIFY(!d->hasEvent("radio1-will-suspend")); + + manager->update(0.5); + + SG_CHECK_EQUAL_EP(1.0, anotherSub->lastUpdateTime); + SG_CHECK_EQUAL_EP(0.5, instruments->lastUpdateTime); + SG_CHECK_EQUAL_EP(0.5, radio1->lastUpdateTime); + + // suspend the whole group + instruments->suspend(); + anotherSub->resume(); + SG_VERIFY(d->hasEvent("anothersub-will-resume")); + SG_VERIFY(d->hasEvent("anothersub-did-resume")); + + SG_VERIFY(d->hasEvent("instruments-will-suspend")); + SG_VERIFY(d->hasEvent("instruments-did-suspend")); + + manager->update(2.0); + + SG_CHECK_EQUAL_EP(2.5, anotherSub->lastUpdateTime); + + // this is significant, since SGSubsystemGroup::update is still + // called in this case + SG_CHECK_EQUAL_EP(2.0, instruments->lastUpdateTime); + SG_CHECK_EQUAL_EP(0.5, radio1->lastUpdateTime); + + // twiddle the state of a radio while its whole group is suspended + // this should not notify! + d->events.clear(); + radio2->suspend(); + SG_VERIFY(d->events.empty()); + + instruments->resume(); + manager->update(3.0); + SG_CHECK_EQUAL_EP(3.0, anotherSub->lastUpdateTime); + SG_CHECK_EQUAL_EP(3.0, instruments->lastUpdateTime); + + // should see all the passed time now + SG_CHECK_EQUAL_EP(5.0, radio1->lastUpdateTime); + SG_CHECK_EQUAL_EP(5.0, radio2->lastUpdateTime); +} + +/////////////////////////////////////////////////////////////////////////////// + + +int main(int argc, char* argv[]) +{ + testRegistrationAndCreation(); + testAddGetRemove(); + testSubGrouping(); + testIncrementalInit(); + testSuspendResume(); + + cout << __FILE__ << ": All tests passed" << endl; + return EXIT_SUCCESS; +} From bf21c0e0999ed9b191d019ea2111a54c9c19d795 Mon Sep 17 00:00:00 2001 From: James Turner Date: Wed, 25 Apr 2018 11:41:40 +0100 Subject: [PATCH 3/4] MSVC build fixes, ooops --- simgear/structure/subsystem_mgr.cxx | 2 ++ simgear/structure/subsystem_mgr.hxx | 1 + simgear/structure/subsystem_test.cxx | 1 + 3 files changed, 4 insertions(+) diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index da180d5b..ac46eeab 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -167,6 +167,8 @@ std::string SGSubsystem::nameForState(State s) case State::SUSPEND: return "suspend"; case State::RESUME: return "resume"; } + + return {}; } //////////////////////////////////////////////////////////////////////// diff --git a/simgear/structure/subsystem_mgr.hxx b/simgear/structure/subsystem_mgr.hxx index 6b9d5888..26674a02 100644 --- a/simgear/structure/subsystem_mgr.hxx +++ b/simgear/structure/subsystem_mgr.hxx @@ -28,6 +28,7 @@ #include #include #include +#include #include #include diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx index fb27a664..cb07e902 100644 --- a/simgear/structure/subsystem_test.cxx +++ b/simgear/structure/subsystem_test.cxx @@ -1,6 +1,7 @@ #include #include +#include #include #include From 48b228f68fcb64f9319fc6b6fad9df823846516b Mon Sep 17 00:00:00 2001 From: James Turner Date: Wed, 25 Apr 2018 17:04:01 +0100 Subject: [PATCH 4/4] Packages: additional test for updating invalid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is part of trying to trace a crash reported on the devel list, unfortunately the test passes but it’s sill good to have it --- simgear/package/CatalogTest.cxx | 31 +++++++++++++++++++++++++++++++ simgear/package/Root.cxx | 2 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/simgear/package/CatalogTest.cxx b/simgear/package/CatalogTest.cxx index 3f603fa4..6a4446c0 100644 --- a/simgear/package/CatalogTest.cxx +++ b/simgear/package/CatalogTest.cxx @@ -861,6 +861,36 @@ void updateValidToInvalid(HTTP::Client* cl) SG_VERIFY(!vectorContains(root->catalogs(), c)); } +void updateInvalidToInvalid(HTTP::Client* cl) +{ + global_catalogVersion = 0; + SGPath rootPath(simgear::Dir::current().path()); + rootPath.append("cat_update_invalid_to_inalid"); + simgear::Dir pd(rootPath); + pd.removeChildren(); + + // first, sync the invalid version + pkg::RootRef root(new pkg::Root(rootPath, "8.1.2")); + root->setHTTPClient(cl); + + pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml"); + waitForUpdateComplete(cl, root); + SG_VERIFY(!c->isEnabled()); + + SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION); + SG_VERIFY(!vectorContains(root->catalogs(), c)); + SG_VERIFY(vectorContains(root->allCatalogs(), c)); + + // now refresh to a different, but still bad one + global_catalogVersion = 3; + c->refresh(); + waitForUpdateComplete(cl, root); + SG_VERIFY(!c->isEnabled()); + SG_VERIFY(c->status() == pkg::Delegate::FAIL_VALIDATION); + SG_VERIFY(!vectorContains(root->catalogs(), c)); + +} + int main(int argc, char* argv[]) { sglog().setLogLevels( SG_ALL, SG_DEBUG ); @@ -894,6 +924,7 @@ int main(int argc, char* argv[]) updateInvalidToValid(&cl); updateValidToInvalid(&cl); + updateInvalidToInvalid(&cl); removeInvalidCatalog(&cl); diff --git a/simgear/package/Root.cxx b/simgear/package/Root.cxx index 24ed0d55..226e632e 100644 --- a/simgear/package/Root.cxx +++ b/simgear/package/Root.cxx @@ -671,7 +671,7 @@ void Root::cancelDownload(InstallRef aInstall) void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason) { - CatalogDict::iterator catIt = d->catalogs.find(aCat->id()); + auto catIt = d->catalogs.find(aCat->id()); d->fireRefreshStatus(aCat, aReason); if (aReason == Delegate::STATUS_IN_PROGRESS) {