diff --git a/simgear/structure/subsystem_mgr.cxx b/simgear/structure/subsystem_mgr.cxx index 644741a7..474ea3dd 100644 --- a/simgear/structure/subsystem_mgr.cxx +++ b/simgear/structure/subsystem_mgr.cxx @@ -456,13 +456,24 @@ SGSubsystemGroup::set_subsystem (const string &name, SGSubsystem * subsystem, notifyDidChange(subsystem, State::ADD); if (_state != State::INVALID) { - SG_LOG(SG_GENERAL, SG_DEV_WARN, "TODO: implement SGSubsystemGroup transition when adding after init"); // transition to the correct state - // bind + if (_state >= State::BIND) { + notifyWillChange(subsystem, State::BIND); + subsystem->bind(); + notifyDidChange(subsystem, State::BIND); + } - // init + if (_state >= State::INIT) { + notifyWillChange(subsystem, State::INIT); + subsystem->init(); + notifyDidChange(subsystem, State::INIT); + } - // post-init + if (_state >= State::POSTINIT) { + notifyWillChange(subsystem, State::POSTINIT); + subsystem->postinit(); + notifyDidChange(subsystem, State::POSTINIT); + } } } @@ -504,11 +515,17 @@ SGSubsystemGroup::remove_subsystem(const string &name) if (_state != State::INVALID) { // transition out correctly - SG_LOG(SG_GENERAL, SG_DEV_WARN, "TODO: implement SGSubsystemGroup transition when removing before shutdown"); - - // shutdown + if (_state >= State::INIT) { + notifyWillChange(sub, State::SHUTDOWN); + sub->shutdown(); + notifyDidChange(sub, State::SHUTDOWN); + } - // unbind + if (_state >= State::BIND) { + notifyWillChange(sub, State::UNBIND); + sub->unbind(); + notifyDidChange(sub, State::UNBIND); + } } notifyWillChange(sub, State::REMOVE); @@ -1152,10 +1169,9 @@ namespace { group, minTime); - if (arg->getBoolValue("do-bind-init", false)) { - sub->bind(); - sub->init(); - } + // we no longer check for the do-bind-init flag here, since set_subsystem + // tracks the group state and will transition the added subsystem + // automatically return true; } @@ -1170,11 +1186,7 @@ namespace { SG_LOG(SG_GENERAL, SG_ALERT, "do_remove_subsystem: unknown subsytem:" << name); return false; } - - // is it safe to always call these? let's assume so! - instance->shutdown(); - instance->unbind(); - + // unplug from the manager (this also deletes the instance!) manager->remove(name.c_str()); return true; diff --git a/simgear/structure/subsystem_test.cxx b/simgear/structure/subsystem_test.cxx index dfc816d0..ed399351 100644 --- a/simgear/structure/subsystem_test.cxx +++ b/simgear/structure/subsystem_test.cxx @@ -439,6 +439,44 @@ void testPropertyRoot() SG_CHECK_EQUAL(props->getIntValue("anothersub/bar"), 172); } +void testAddRemoveAfterInit() +{ + SGSharedPtr manager = new SGSubsystemMgr; + auto d = new RecorderDelegate; + manager->addDelegate(d); + + auto group = manager->add(); + SG_VERIFY(group); + + auto radio1 = manager->createInstance("nav1"); + group->set_subsystem(radio1); + + manager->bind(); + manager->init(); + + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-bind")); + + auto radio2 = manager->createInstance("nav2"); + group->set_subsystem(radio2); + + SG_VERIFY(d->hasEvent("fake-radio.nav2-will-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav2-did-init")); + SG_VERIFY(d->hasEvent("fake-radio.nav2-did-bind")); + + bool ok = manager->remove("fake-radio.nav1"); + SG_VERIFY(ok); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-shutdown")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-unbind")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-will-remove")); + SG_VERIFY(d->hasEvent("fake-radio.nav1-did-remove")); + + + +} + /////////////////////////////////////////////////////////////////////////////// @@ -450,6 +488,7 @@ int main(int argc, char* argv[]) testIncrementalInit(); testSuspendResume(); testPropertyRoot(); + testAddRemoveAfterInit(); cout << __FILE__ << ": All tests passed" << endl; return EXIT_SUCCESS;