[Flightgear-devel] Subsystem refactoring.

[Edward d'A. <tru...@gm...>]

Now that the 2018.2.1 release has past, James, when you have time
would you be able to have a look at my subsystem refactoring?
Torsten, as I have refactored your CommRadio and NavRadio subsystem
code, could you also please have a look at these?  Cheers!

I have pushed the changes to these two branches on my repository forks:

    flightgear subsystem_dependencies/r4:
https://sourceforge.net/u/edauvergne/flightgear/ci/subsystem_dependencies/r4/~/log/
    simgear subsystem_dependencies/r5:
https://sourceforge.net/u/edauvergne/simgear/ci/subsystem_dependencies/r5/~/log/

The simgear branch is rebased on the PropertyObject/r2 branch, as can
be seen in the web links.  A summary of the changes is:

1)  SGSubsystemMgr:  I have created two new functions
SGSubsystemMgr::add() and SGSubsystemMgr::addInstance().  These
provide a new interface for the subsystem manager to create subsystems
itself.

Note, the addInstance() function supports the "node" argument to allow
for a subsystem specific property tree node to be specified during
creation.  The node is however added via the new virtual
SGSubsystem::set_instance_node() function and stored as the private
_instanceNode variable.  I've overridden this function only in the
CommRadio to allow for the node to be also passed into
OutputProperties (via the new setRoot() function).


2)  Standardisation of the subsystem interface throughout the code
base.  Subsystems are only added via the new add() or addInstance()
functions (there are still some to convert as ctor arguments are
fatal).  And the templated SGSubsystemMgr::get_subsystem() function is
now used for all fetching of subsystems.


3)  FGInstrumentMgr:  I've used this as a test for conversion of
subsystem shells/managers/groups/instance creators/etc.  This is most
of the commits in the flightgear subsystem_dependencies/r4 branch.
There are two major points with this:


3a)  I've eliminated the need for ctor arguments.  The property tree
node is now handled by SGSubsystem::set_instance_node() and has been
removed from the ctor.  This sometimes requires blocks of ctor code to
be shifted to the start of bind() or init() via a simple cut and paste
operation.

For other non-standard ctor arguments, I have added setter functions
within the specific subsystem.  In a block, you then call
addInstance(), get_subsystem<name>(instanceId), and then the setter
function.  Again any ctor code requiring the variable is cut and paste
into bind() or init().


3b)  The location of the subsystems has changed.  Instead of being
stored in the MemberVec of the FGInstrumentMgr subsystem group, the
subsystem manager packs the new instanced subsystems in the same base
MemberVec as the FGInstrumentMgr (using the global registration
information).  The instrument manager then uses
get_manager()->get_subsystem(subsystemClassId, subsystemInstanceId)
without recasting to obtain the subsystem it manages.

This type of design removes the need for SGSubsystemGroup from
everywhere but the subsystem manager itself.  Hence all shells,
managers, groups, instance creators, etc. can operate as simple
subsystems.  The subsystem dependency vector (or initialisation order)
will ensure that the managed subsystems are always after them in the
vector.


One question I have is about the shifting of the ctor subsystem
initialisation code into the first subsystem API function called
(usually either bind() or init()).  I have a felling that this would
be cleaner if we introduced a preinit() subsystem API, so that the
startup sequence would be preinit() -> bind() -> init() -> postinit().
A lot of the code going into bind() really does not fit into the
concept of bind().  See my changes to CommRadio::bind() for example
[1].  What do you think?  Is there another way to organise this so
that the code isn't randomly shifted into bind() or init()?

Regards,

Edward


[1]  https://sourceforge.net/u/edauvergne/flightgear/ci/7d017c7ad567200f405b8777ef50706624cb9267/