Subsystems: bind/init/unbind on add/remove

When adding/remove subsystems to a group which is already bound or
inited, transition the subsystem automatically. This removes the
need to manually transition the subsystem in various places in
Flightgear.

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;

simgear/structure/subsystem_test.cxx

@@ -439,6 +439,44 @@ void testPropertyRoot()
     SG_CHECK_EQUAL(props->getIntValue("anothersub/bar"), 172);
 }
 
+void testAddRemoveAfterInit()
+{
+    SGSharedPtr<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    auto d = new RecorderDelegate;
+    manager->addDelegate(d);
+    
+    auto group = manager->add<InstrumentGroup>();
+    SG_VERIFY(group);
+
+    auto radio1 = manager->createInstance<FakeRadioSub>("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<FakeRadioSub>("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;