new version: 2018.3.0

This fixes the reset process that was broken by
dfed2184f1.

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")

simgear/canvas/Canvas.cxx

@@ -1,4 +1,5 @@
-// The canvas for rendering with the 2d API
+///@file
+/// The canvas for rendering with the 2d API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/Canvas.hxx

@@ -71,18 +71,18 @@ namespace canvas
         public osg::NodeCallback
       {
         public:
-          CullCallback(const CanvasWeakPtr& canvas);
+          explicit CullCallback(const CanvasWeakPtr& canvas);
 
         private:
           CanvasWeakPtr _canvas;
 
-          virtual void operator()(osg::Node* node, osg::NodeVisitor* nv);
+          void operator()(osg::Node* node, osg::NodeVisitor* nv) override;
       };
       typedef osg::ref_ptr<CullCallback> CullCallbackPtr;
 
-      Canvas(SGPropertyNode* node);
+      explicit Canvas(SGPropertyNode* node);
       virtual ~Canvas();
-      virtual void onDestroy();
+      void onDestroy() override;
 
       void setCanvasMgr(CanvasMgr* canvas_mgr);
       CanvasMgr* getCanvasMgr() const;
@@ -184,11 +184,9 @@ namespace canvas
       bool propagateEvent( EventPtr const& event,
                            EventPropagationPath const& path );
 
-      virtual void childAdded( SGPropertyNode * parent,
-                               SGPropertyNode * child );
-      virtual void childRemoved( SGPropertyNode * parent,
-                                 SGPropertyNode * child );
-      virtual void valueChanged (SGPropertyNode * node);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode * node) override;
 
       osg::Texture2D* getTexture() const;
 
@@ -254,8 +252,8 @@ namespace canvas
 
       static SystemAdapterPtr _system_adapter;
 
-      Canvas(const Canvas&); // = delete;
-      Canvas& operator=(const Canvas&); // = delete;
+      Canvas(const Canvas&) = delete;
+      Canvas& operator=(const Canvas&) = delete;
   };
 
 } // namespace canvas

simgear/canvas/CanvasEvent.cxx

@@ -1,4 +1,5 @@
-// Canvas Event for event model similar to DOM Level 3 Event Model
+///@file
+/// Canvas Event for event model similar to DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -125,10 +126,10 @@ namespace canvas
   //----------------------------------------------------------------------------
   std::string Event::typeToStr(int type)
   {
-    TypeMap const& type_map = getTypeMap();
+    auto const& map_by_id = getTypeMap().by<id>();
 
-    TypeMap::map_by<id>::const_iterator it = type_map.by<id>().find(type);
-    if( it == type_map.by<id>().end() )
+    auto it = map_by_id.find(type);
+    if( it == map_by_id.end() )
       return "unknown";
     return it->second;
   }

simgear/canvas/CanvasEvent.hxx

@@ -65,6 +65,11 @@ namespace canvas
       // of the actual event instances.
       virtual ~Event();
 
+      /**
+       * Clone event and set to the given type (Same type if not specified)
+       */
+      virtual Event* clone(int type = 0) const = 0;
+
       /**
        * Get whether this events support bubbling
        */
@@ -110,7 +115,14 @@ namespace canvas
        */
       bool defaultPrevented() const;
 
+      /**
+       * Register a new type string or get the id of an existing type string
+       *
+       * @param type    Type string
+       * @return Id of the given @a type
+       */
       static int getOrRegisterType(const std::string& type);
+
       static int strToType(const std::string& type);
       static std::string typeToStr(int type);
 

simgear/canvas/CanvasEventManager.cxx

@@ -1,4 +1,5 @@
-// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
+///@file
+/// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,9 +18,11 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasEventManager.hxx"
-#include <simgear/canvas/events/MouseEvent.hxx>
-#include <simgear/canvas/elements/CanvasElement.hxx>
+#include "elements/CanvasElement.hxx"
+#include "events/MouseEvent.hxx"
+
 #include <cmath>
 
 namespace simgear
@@ -114,6 +117,8 @@ namespace canvas
         return handled;
       }
       case Event::DRAG:
+      case Event::DRAG_START:
+      case Event::DRAG_END:
         if( !_last_mouse_down.valid() )
           return false;
         else

simgear/canvas/CanvasEventManager.hxx

@@ -1,4 +1,5 @@
-// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
+///@file
+/// Manage event handling inside a Canvas similar to the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasEventTypes.hxx

@@ -1,4 +1,5 @@
-// Mapping between canvas gui Event types and their names
+///@file
+/// Mapping between canvas gui Event types and their names
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -25,6 +26,8 @@ ENUM_MAPPING(MOUSE_UP,    "mouseup",    MouseEvent)
 ENUM_MAPPING(CLICK,       "click",      MouseEvent)
 ENUM_MAPPING(DBL_CLICK,   "dblclick",   MouseEvent)
 ENUM_MAPPING(DRAG,        "drag",       MouseEvent)
+ENUM_MAPPING(DRAG_START,  "dragstart",  MouseEvent)
+ENUM_MAPPING(DRAG_END,    "dragend",    MouseEvent)
 ENUM_MAPPING(WHEEL,       "wheel",      MouseEvent)
 ENUM_MAPPING(MOUSE_MOVE,  "mousemove",  MouseEvent)
 ENUM_MAPPING(MOUSE_OVER,  "mouseover",  MouseEvent)

simgear/canvas/CanvasEventVisitor.cxx

@@ -1,5 +1,6 @@
-// Visitor for traversing a canvas element hierarchy similar to the traversal
-// of the DOM Level 3 Event Model
+///@file
+/// Visitor for traversing a canvas element hierarchy similar to the traversal
+/// of the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -18,9 +19,10 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasEvent.hxx"
 #include "CanvasEventVisitor.hxx"
-#include <simgear/canvas/elements/CanvasElement.hxx>
+#include "elements/CanvasElement.hxx"
 
 namespace simgear
 {

simgear/canvas/CanvasEventVisitor.hxx

@@ -1,5 +1,6 @@
-// Visitor for traversing a canvas element hierarchy similar to the traversal
-// of the DOM Level 3 Event Model
+///@file
+/// Visitor for traversing a canvas element hierarchy similar to the traversal
+/// of the DOM Level 3 Event Model
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasMgr.cxx

@@ -1,4 +1,5 @@
-// Canvas with 2D rendering API
+///@file
+/// Canvas with 2D rendering API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,12 +18,11 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasMgr.hxx"
 #include "Canvas.hxx"
 #include "CanvasEventManager.hxx"
 
-#include <boost/bind.hpp>
-
 namespace simgear
 {
 namespace canvas

simgear/canvas/CanvasMgr.hxx

@@ -1,4 +1,5 @@
-// Canvas with 2D rendering API
+///@file
+/// Canvas with 2D rendering API
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -61,7 +62,7 @@ namespace canvas
 
     protected:
 
-      virtual void elementCreated(PropertyBasedElementPtr element);
+      void elementCreated(PropertyBasedElementPtr element) override;
   };
 
 } // namespace canvas

simgear/canvas/CanvasObjectPlacement.cxx

@@ -1,4 +1,5 @@
-// Canvas placement for placing a canvas texture onto osg objects.
+///@file
+/// Canvas placement for placing a canvas texture onto osg objects
 //
 // It also provides a SGPickCallback for passing mouse events to the canvas and
 // manages emissive lighting of the placed canvas.

simgear/canvas/CanvasObjectPlacement.hxx

@@ -1,5 +1,5 @@
 ///@file
-/// Placement for putting a canvas texture onto OpenSceneGraph objects.
+/// Placement for putting a canvas texture onto OpenSceneGraph objects
 ///
 /// It also provides a SGPickCallback for passing mouse events to the canvas and
 /// manages emissive lighting of the placed canvas.
@@ -60,7 +60,7 @@ namespace canvas
        */
       void setCaptureEvents(bool enable);
 
-      virtual bool childChanged(SGPropertyNode* child);
+      bool childChanged(SGPropertyNode* child) override;
 
     protected:
       typedef SGSharedPtr<SGPickCallback> PickCallbackPtr;

simgear/canvas/CanvasPlacement.cxx

@@ -1,4 +1,5 @@
-// Base class for canvas placements
+///@file
+/// Base class for canvas placements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/CanvasPlacement.hxx

@@ -1,4 +1,5 @@
-// Base class for canvas placements
+///@file
+/// Base class for canvas placements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -40,9 +41,8 @@ namespace canvas
     protected:
       SGPropertyNode_ptr _node;
 
-    private:
-      Placement(const Placement&) /* = delete */;
-      Placement& operator=(const Placement&) /* = delete */;
+      Placement(const Placement&) = delete;
+      Placement& operator=(const Placement&) = delete;
   };
 
 } // namespace canvas

simgear/canvas/CanvasSystemAdapter.hxx

@@ -1,4 +1,5 @@
-// Adapter for using the canvas with different applications
+///@file
+/// Adapter for using the canvas with different applications
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -29,6 +30,10 @@ namespace HTTP { class Client; }
 namespace canvas
 {
 
+  /**
+   * Provides access to different required systems of the application to the
+   * Canvas
+   */
   class SystemAdapter
   {
     public:

simgear/canvas/CanvasWindow.cxx

@@ -1,4 +1,5 @@
-// Window for placing a Canvas onto it (for dialogs, menus, etc.)
+///@file
+/// Window for placing a Canvas onto it (for dialogs, menus, etc.)
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -212,6 +213,19 @@ namespace canvas
       _resize_left = getRegion().l() + offset.x();
   }
 
+  //----------------------------------------------------------------------------
+  bool Window::handleEvent(const EventPtr& event)
+  {
+    if( auto mouse_event = dynamic_cast<MouseEvent*>(event.get()) )
+    {
+      mouse_event->local_pos =
+      mouse_event->client_pos =
+        mouse_event->screen_pos - toOsg(getPosition());
+    }
+
+    return Image::handleEvent(event);
+  }
+
   //----------------------------------------------------------------------------
   void Window::parseDecorationBorder(const std::string& str)
   {

simgear/canvas/CanvasWindow.hxx

@@ -1,4 +1,5 @@
-// Window for placing a Canvas onto it (for dialogs, menus, etc.)
+///@file
+/// Window for placing a Canvas onto it (for dialogs, menus, etc.)
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -96,6 +97,8 @@ namespace canvas
       void handleResize( uint8_t mode,
                          const osg::Vec2f& offset = osg::Vec2f() );
 
+      bool handleEvent(const EventPtr& event) override;
+
     protected:
 
       enum Attributes

simgear/canvas/ODGauge.cxx

@@ -1,4 +1,5 @@
-// Owner Drawn Gauge helper class
+///@file
+/// Owner Drawn Gauge helper class
 //
 // Written by Harald JOHNSEN, started May 2005.
 //
@@ -6,9 +7,9 @@
 //
 // Ported to OSG by Tim Moore - Jun 2007
 //
-// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April 2012
-// Supports now multisampling/mipmapping, usage of the stencil buffer and placing
-// the texture in the scene by certain filter criteria
+// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April
+// 2012 Supports now multisampling/mipmapping, usage of the stencil buffer and
+// placing the texture in the scene by certain filter criteria.
 //
 // This library is free software; you can redistribute it and/or
 // modify it under the terms of the GNU Library General Public

simgear/canvas/ODGauge.hxx

@@ -1,4 +1,5 @@
-// Owner Drawn Gauge helper class
+///@file
+/// Owner Drawn Gauge helper class
 //
 // Written by Harald JOHNSEN, started May 2005.
 //
@@ -6,9 +7,9 @@
 //
 // Ported to OSG by Tim Moore - Jun 2007
 //
-// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April 2012
-// Supports now multisampling/mipmapping, usage of the stencil buffer and placing
-// the texture in the scene by certain filter criteria
+// Heavily modified to be usable for the 2d Canvas by Thomas Geymayer - April
+// 2012 Supports now multisampling/mipmapping, usage of the stencil buffer and
+// placing the texture in the scene by certain filter criteria.
 //
 // This library is free software; you can redistribute it and/or
 // modify it under the terms of the GNU Library General Public

simgear/canvas/VGInitOperation.cxx

@@ -1,4 +1,5 @@
-// osg::Operation to initialize the OpenVG context used for path rendering
+///@file
+/// osg::Operation to initialize the OpenVG context used for path rendering
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/VGInitOperation.hxx

@@ -1,4 +1,5 @@
-// osg::Operation to initialize the OpenVG context used for path rendering
+///@file
+/// osg::Operation to initialize the OpenVG context used for path rendering
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/canvas_fwd.hxx

@@ -1,4 +1,5 @@
-// Canvas forward declarations
+///@file
+/// Canvas forward declarations
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/CanvasElement.cxx

@@ -1,4 +1,5 @@
-// Interface for 2D Canvas element
+///@file
+/// Interface for 2D Canvas element
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -271,8 +272,7 @@ namespace canvas
   //----------------------------------------------------------------------------
   void Element::setFocus()
   {
-    CanvasPtr canvas = _canvas.lock();
-    if( canvas )
+    if( auto canvas = _canvas.lock() )
       canvas->setFocusElement(this);
   }
 

simgear/canvas/elements/CanvasElement.hxx

@@ -95,7 +95,7 @@ namespace canvas
        *
        */
       virtual ~Element() = 0;
-      virtual void onDestroy();
+      void onDestroy() override;
 
       ElementPtr getParent() const;
       CanvasWeakPtr getCanvas() const;
@@ -105,7 +105,7 @@ namespace canvas
        *
        * @param dt  Frame time in seconds
        */
-      virtual void update(double dt);
+      void update(double dt) override;
 
       bool addEventListener(const std::string& type, const EventListener& cb);
       virtual void clearEventListener();
@@ -154,11 +154,9 @@ namespace canvas
        */
       osg::Vec2f posToLocal(const osg::Vec2f& pos) const;
 
-      virtual void childAdded( SGPropertyNode * parent,
-                               SGPropertyNode * child );
-      virtual void childRemoved( SGPropertyNode * parent,
-                                 SGPropertyNode * child );
-      virtual void valueChanged(SGPropertyNode * child);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode* child) override;
 
       virtual bool setStyle( const SGPropertyNode* child,
                              const StyleInfo* style_info = 0 );
@@ -597,7 +595,7 @@ namespace canvas
 
       osg::ref_ptr<osg::Drawable> _drawable;
 
-      Element(const Element&);// = delete
+      Element(const Element&) = delete;
 
       template<class Derived>
       static Derived& derived_cast(Element& el)

simgear/canvas/elements/CanvasGroup.cxx

@@ -1,4 +1,5 @@
-// A group of 2D Canvas elements
+///@file
+/// A group of 2D Canvas elements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -168,6 +169,7 @@ namespace canvas
     if( !_scene_group.valid() )
       return warnSceneGroupExpired("clearEventListener");
 
+    // TODO should this be recursive?
     for(size_t i = 0; i < _scene_group->getNumChildren(); ++i)
       getChildByIndex(i)->clearEventListener();
   }

simgear/canvas/elements/CanvasGroup.hxx

@@ -1,4 +1,5 @@
-// A group of 2D Canvas elements
+///@file
+/// A group of 2D Canvas elements
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -86,14 +87,15 @@ namespace canvas
        */
       ElementPtr getElementById(const std::string& id);
 
-      virtual void clearEventListener();
+      void clearEventListener() override;
 
-      virtual bool traverse(EventVisitor& visitor);
+      bool traverse(EventVisitor& visitor) override;
 
-      virtual bool setStyle( const SGPropertyNode* child,
-                             const StyleInfo* style_info = 0 );
+      bool setStyle( const SGPropertyNode* child,
+                     const StyleInfo* style_info = 0 ) override;
 
-      virtual osg::BoundingBox getTransformedBounds(const osg::Matrix& m) const;
+      osg::BoundingBox
+      getTransformedBounds(const osg::Matrix& m) const override;
 
     protected:
 
@@ -105,11 +107,11 @@ namespace canvas
        */
       virtual ElementFactory getChildFactory(const std::string& type) const;
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childAdded(SGPropertyNode * child);
-      virtual void childRemoved(SGPropertyNode * child);
-      virtual void childChanged(SGPropertyNode * child);
+      void childAdded(SGPropertyNode * child) override;
+      void childRemoved(SGPropertyNode * child) override;
+      void childChanged(SGPropertyNode * child) override;
 
       void handleZIndexChanged(ElementPtr child, int z_index = 0);
 

simgear/canvas/elements/CanvasImage.cxx

@@ -1,4 +1,5 @@
-// An image on the Canvas
+///@file
+/// An image on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/CanvasImage.hxx

@@ -1,4 +1,5 @@
-// An image on the Canvas
+///@file
+/// An image on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -53,7 +54,7 @@ namespace canvas
              ElementWeakPtr parent = 0 );
       virtual ~Image();
 
-      virtual void valueChanged(SGPropertyNode* child);
+      void valueChanged(SGPropertyNode* child) override;
 
       void setSrcCanvas(CanvasPtr canvas);
       CanvasWeakPtr getSrcCanvas() const;
@@ -93,7 +94,7 @@ namespace canvas
 
       const SGRect<float>& getRegion() const;
 
-      bool handleEvent(const EventPtr& event);
+      bool handleEvent(const EventPtr& event) override;
 
       /**
        *
@@ -108,9 +109,9 @@ namespace canvas
         SRC_CANVAS     = DEST_SIZE << 1
       };
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childChanged(SGPropertyNode * child);
+      void childChanged(SGPropertyNode * child) override;
 
       void setupDefaultDimensions();
       SGRect<int> getTextureDimensions() const;

simgear/canvas/elements/CanvasMap.cxx

@@ -1,5 +1,6 @@
-// A group of 2D Canvas elements which get automatically transformed according
-// to the map parameters.
+///@file
+/// A group of 2D Canvas elements which get automatically transformed according
+/// to the map parameters.
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -199,10 +200,18 @@ namespace canvas
       _projection = std::make_shared<SansonFlamsteedProjection>();
 
     _projection->setWorldPosition(_node->getDoubleValue(REF_LAT),
-                                  _node->getDoubleValue(REF_LON) );
-    _projection->setOrientation(_node->getFloatValue(HDG));
-    _projection->setScreenRange(_node->getDoubleValue(SCREEN_RANGE));
-    _projection->setRange(_node->getDoubleValue(RANGE));
+                                  _node->getDoubleValue(REF_LON));
+
+    // Only set existing properties to prevent using 0 instead of default values
+
+    if( auto heading = _node->getChild(HDG) )
+      _projection->setOrientation(heading->getFloatValue());
+
+    if( auto screen_range = _node->getChild(SCREEN_RANGE) )
+      _projection->setScreenRange(screen_range->getDoubleValue());
+
+    if( auto range = _node->getChild(RANGE) )
+      _projection->setRange(range->getDoubleValue());
 
     _projection_dirty = true;
   }

simgear/canvas/elements/CanvasMap.hxx

@@ -1,5 +1,6 @@
-// A group of 2D Canvas elements which get automatically transformed according
-// to the map parameters.
+///@file
+/// A group of 2D Canvas elements which get automatically transformed according
+/// to the map parameters.
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -46,16 +47,14 @@ namespace canvas
       virtual ~Map();
 
     protected:
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
       void updateProjection(SGPropertyNode* type_node);
 
-      virtual void childAdded( SGPropertyNode* parent,
-                               SGPropertyNode* child );
-      virtual void childRemoved( SGPropertyNode* parent,
-                                 SGPropertyNode* child );
-      virtual void valueChanged(SGPropertyNode* child);
-      virtual void childChanged(SGPropertyNode* child);
+      void childAdded(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void childRemoved(SGPropertyNode* parent, SGPropertyNode* child) override;
+      void valueChanged(SGPropertyNode* child) override;
+      void childChanged(SGPropertyNode* child) override;
 
       using GeoNodes =
         std::unordered_map<SGPropertyNode*, std::shared_ptr<GeoNodePair>>;

simgear/canvas/elements/CanvasPath.cxx

@@ -1,4 +1,5 @@
-// An OpenVG path on the Canvas
+///@file
+/// An OpenVG path on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -230,9 +231,12 @@ namespace canvas
           vgDestroyPaint(_paint_fill);
       }
 
-      virtual const char* className() const { return "PathDrawable"; }
-      virtual osg::Object* cloneType() const { return new PathDrawable(_path_element); }
-      virtual osg::Object* clone(const osg::CopyOp&) const { return new PathDrawable(_path_element); }
+      const char* className() const override
+      { return "PathDrawable"; }
+      osg::Object* cloneType() const override
+      { return new PathDrawable(_path_element); }
+      osg::Object* clone(const osg::CopyOp&) const override
+      { return new PathDrawable(_path_element); }
 
       /**
        * Replace the current path segments with the new ones
@@ -383,7 +387,7 @@ namespace canvas
       /**
        * Draw callback
        */
-      virtual void drawImplementation(osg::RenderInfo& renderInfo) const
+      void drawImplementation(osg::RenderInfo& renderInfo) const override
       {
         if( _attributes_dirty & PATH )
           return;
@@ -555,13 +559,13 @@ namespace canvas
       /**
        * Compute the bounding box
        */
-      virtual osg::BoundingBox
+      osg::BoundingBox
 #if OSG_VERSION_LESS_THAN(3,3,2)
       computeBound()
 #else
       computeBoundingBox()
 #endif
-      const
+      const override
       {
         if( _path == VG_INVALID_HANDLE || (_attributes_dirty & PATH) )
           return osg::BoundingBox();
@@ -667,7 +671,7 @@ namespace canvas
       struct PathUpdateCallback:
         public osg::Drawable::UpdateCallback
       {
-        virtual void update(osg::NodeVisitor*, osg::Drawable* drawable)
+        void update(osg::NodeVisitor*, osg::Drawable* drawable) override
         {
           static_cast<PathDrawable*>(drawable)->update();
         }
@@ -896,8 +900,10 @@ namespace canvas
     else if( name == "coord" )
       _attributes_dirty |= COORDS;
     else if ( name == "svg")
+    {
       _hasSVG = true;
       _attributes_dirty |= SVG;
+    }
   }
 
   //----------------------------------------------------------------------------

simgear/canvas/elements/CanvasPath.hxx

@@ -1,4 +1,5 @@
-// An OpenVG path on the Canvas
+///@file
+/// An OpenVG path on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -40,7 +41,8 @@ namespace canvas
             ElementWeakPtr parent = 0 );
       virtual ~Path();
 
-      virtual osg::BoundingBox getTransformedBounds(const osg::Matrix& m) const;
+      osg::BoundingBox
+      getTransformedBounds(const osg::Matrix& m) const override;
 
       /** Add a segment with the given command and coordinates */
       Path& addSegment(uint8_t cmd, std::initializer_list<float> coords = {});
@@ -86,10 +88,10 @@ namespace canvas
       bool _hasRect : 1;
       SGRectf _rect;
 
-      virtual void updateImpl(double dt);
+      void updateImpl(double dt) override;
 
-      virtual void childRemoved(SGPropertyNode * child);
-      virtual void childChanged(SGPropertyNode * child);
+      void childRemoved(SGPropertyNode * child) override;
+      void childChanged(SGPropertyNode * child) override;
 
       void parseRectToVGPath();
   };

simgear/canvas/elements/CanvasText.cxx

@@ -1,4 +1,5 @@
-// A text on the Canvas
+///@file
+/// A text on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -17,6 +18,7 @@
 // Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301, USA
 
 #include <simgear_config.h>
+
 #include "CanvasText.hxx"
 #include <simgear/canvas/Canvas.hxx>
 #include <simgear/canvas/CanvasSystemAdapter.hxx>
@@ -56,21 +58,20 @@ namespace canvas
 
       SGVec2i sizeForWidth(int w) const;
 
-      virtual osg::BoundingBox
+      osg::BoundingBox
 #if OSG_VERSION_LESS_THAN(3,3,2)
       computeBound()
 #else
       computeBoundingBox()
 #endif
-      const;
+      const override;
 
     protected:
-
       friend class TextLine;
 
       canvas::Text *_text_element;
 
-      virtual void computePositions(unsigned int contextID) const;
+      void computePositions(unsigned int contextID) const override;
   };
 
   class TextLine

simgear/canvas/elements/CanvasText.hxx

@@ -1,4 +1,5 @@
-// A text on the Canvas
+///@file
+/// A text on the Canvas
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/elements/map/projection.hxx

@@ -1,4 +1,5 @@
-// Geographic projections for Canvas map element
+///@file
+/// Geographic projections for Canvas map element
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/events/CustomEvent.cxx

@@ -1,4 +1,5 @@
-// Canvas user defined event
+///@file
+/// Canvas user defined event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -46,6 +47,14 @@ namespace canvas
 //    assert( type_map.find(type_id) != type_map.end() );
   }
 
+  //----------------------------------------------------------------------------
+  CustomEvent* CustomEvent::clone(int type) const
+  {
+    auto event = new CustomEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   void CustomEvent::setDetail(StringMap const& data)
   {

simgear/canvas/events/CustomEvent.hxx

@@ -38,6 +38,7 @@ namespace canvas
     public:
 
       /**
+       * @brief Construct a user defined event from a type string
        *
        * @param type_str    Event type name (if name does not exist yet it will
        *                    be registered as new event type)
@@ -49,6 +50,10 @@ namespace canvas
                    StringMap const& data = StringMap() );
 
       /**
+       * @brief Construct a user defined event from a (previously registered)
+       *        type id
+       *
+       * @see getOrRegisterType()
        *
        * @param type_id     Event type id
        * @param bubbles     If this event should take part in the bubbling phase
@@ -58,6 +63,8 @@ namespace canvas
                    bool bubbles = false,
                    StringMap const& data = StringMap() );
 
+      CustomEvent* clone(int type = 0) const override;
+
       /**
        * Set user data
        */
@@ -74,7 +81,7 @@ namespace canvas
        * @see #bubbles
        * @see CustomEvent()
        */
-      virtual bool canBubble() const { return bubbles; }
+      bool canBubble() const override { return bubbles; }
 
       StringMap detail; //!< User data map
       bool bubbles;     //!< Whether the event supports bubbling

simgear/canvas/events/DeviceEvent.cxx

@@ -1,4 +1,5 @@
-// Input device event
+///@file
+/// Input device event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/canvas/events/KeyboardEvent.cxx

@@ -1,4 +1,5 @@
-// Keyboard event
+///@file
+/// Keyboard event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -69,6 +70,14 @@ namespace canvas
 //      // TODO what to do with wrong event type?
   }
 
+  //----------------------------------------------------------------------------
+  KeyboardEvent* KeyboardEvent::clone(int type) const
+  {
+    auto event = new KeyboardEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   void KeyboardEvent::setKey(uint32_t key)
   {

simgear/canvas/events/KeyboardEvent.hxx

@@ -45,6 +45,7 @@ namespace canvas
 
       KeyboardEvent();
       KeyboardEvent(const osgGA::GUIEventAdapter& ea);
+      KeyboardEvent* clone(int type = 0) const override;
 
       void setKey(uint32_t key);
       void setUnmodifiedKey(uint32_t key);

simgear/canvas/events/MouseEvent.cxx

@@ -1,4 +1,5 @@
-// Mouse event
+///@file
+/// Mouse event
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -48,6 +49,14 @@ namespace canvas
       button += 1;
   }
 
+  //----------------------------------------------------------------------------
+  MouseEvent* MouseEvent::clone(int type) const
+  {
+    auto event = new MouseEvent(*this);
+    event->type = type;
+    return event;
+  }
+
   //----------------------------------------------------------------------------
   bool MouseEvent::canBubble() const
   {

simgear/canvas/events/MouseEvent.hxx

@@ -36,8 +36,9 @@ namespace canvas
     public:
       MouseEvent();
       MouseEvent(const osgGA::GUIEventAdapter& ea);
+      MouseEvent* clone(int type = 0) const override;
 
-      virtual bool canBubble() const;
+      bool canBubble() const override;
 
       osg::Vec2f getScreenPos() const { return screen_pos; }
       osg::Vec2f getClientPos() const { return client_pos; }

simgear/canvas/events/input_event_demo.cxx

@@ -1,4 +1,5 @@
-// Keyboard event demo. Press some keys and get some info...
+///@file
+/// Keyboard event demo. Press some keys and get some info...
 //
 // Copyright (C) 2014  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/debug/logstream.cxx

@@ -335,9 +335,7 @@ public:
 
     ~LogStreamPrivate()
     {
-        for (simgear::LogCallback* cb : m_callbacks) {
-            delete cb;
-        }
+        removeCallbacks();
     }
 
     SGMutex m_lock;
@@ -363,6 +361,9 @@ public:
 #endif
     bool m_developerMode = false;
 
+    // test suite mode.
+    bool m_testMode = false;
+
     void startLog()
     {
         SGGuard<SGMutex> g(m_lock);
@@ -443,6 +444,16 @@ public:
         }
     }
 
+    void removeCallbacks()
+    {
+        PauseThread pause(this);
+        for (simgear::LogCallback* cb : m_callbacks) {
+            delete cb;
+        }
+        m_callbacks.clear();
+        m_consoleCallbacks.clear();
+    }
+
     void setLogLevels( sgDebugClass c, sgDebugPriority p )
     {
         PauseThread pause(this);
@@ -455,6 +466,9 @@ public:
 
     bool would_log( sgDebugClass c, sgDebugPriority p ) const
     {
+        // Testing mode, so always log.
+        if (m_testMode) return true;
+
         p = translatePriority(p);
         if (p >= SG_INFO) return true;
         return ((c & m_logClass) != 0 && p >= m_logPriority);
@@ -711,6 +725,13 @@ void logstream::requestConsole()
 #endif
 }
 
+void
+logstream::setTestingMode( bool testMode )
+{
+    d->m_testMode = testMode;
+    if (testMode) d->removeCallbacks();
+}
+
 
 namespace simgear
 {

simgear/debug/logstream.hxx

@@ -158,12 +158,22 @@ public:
      
     void removeCallback(simgear::LogCallback* cb);
 
+    void removeCallbacks();
+
     /**
      * optionally record all entries and submit them to new log callbacks that
      * are added. This allows simplified logging configuration, but still including
      * early startup information in all logs.
      */
     void setStartupLoggingEnabled(bool enabled);
+
+    /**
+     * Set up the logstream for running in test mode.  For example the callbacks
+     * will be unregistered and the behaviour of the would_log() function
+     * sanitized.
+     */
+    void setTestingMode(bool testMode);
+
 private:
     // constructor
     logstream();

simgear/io/HTTPClient.cxx

@@ -242,8 +242,13 @@ void Client::makeRequest(const Request_ptr& r)
     if( r->isComplete() )
       return;
 
+    if (r->url().empty()) {
+        r->setFailure(EINVAL, "no URL specified on request");
+        return;
+    }
+
     if( r->url().find("://") == std::string::npos ) {
-        r->setFailure(EINVAL, "malformed URL");
+        r->setFailure(EINVAL, "malformed URL: '" + r->url() + "'");
         return;
     }
 

simgear/io/HTTPRepository.cxx

@@ -75,6 +75,23 @@ namespace simgear
 
     typedef SGSharedPtr<HTTPRepoGetRequest> RepoRequestPtr;
 
+    std::string innerResultCodeAsString(HTTPRepository::ResultCode code)
+    {
+        switch (code) {
+        case HTTPRepository::REPO_NO_ERROR:     return "no error";
+        case HTTPRepository::REPO_ERROR_NOT_FOUND:  return "not found";
+        case HTTPRepository::REPO_ERROR_SOCKET:     return "socket error";
+        case HTTPRepository::SVN_ERROR_XML:        return "malformed XML";
+        case HTTPRepository::SVN_ERROR_TXDELTA:        return "malformed XML";
+        case HTTPRepository::REPO_ERROR_IO:         return "I/O error";
+        case HTTPRepository::REPO_ERROR_CHECKSUM:         return "checksum verification error";
+        case HTTPRepository::REPO_ERROR_FILE_NOT_FOUND: return "file not found";
+        case HTTPRepository::REPO_ERROR_HTTP:   return "HTTP-level error";
+        case HTTPRepository::REPO_ERROR_CANCELLED:   return "cancelled";
+        case HTTPRepository::REPO_PARTIAL_UPDATE:   return "partial update (incomplete)";
+        }
+    }
+    
 class HTTPRepoPrivate
 {
 public:
@@ -274,9 +291,7 @@ public:
             if (!cp.exists()) {
                 continue;
             }
-
-            SG_LOG(SG_TERRASYNC, SG_BULK, "new child, copying existing file" << cp << p);
-
+            
             SGBinaryFile src(cp);
             SGBinaryFile dst(p);
             src.open(SG_IO_IN);
@@ -316,22 +331,20 @@ public:
 
             ChildInfoList::iterator c = findIndexChild(it->file());
             if (c == children.end()) {
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "is orphan '" << it->file() << "'" );
-
-				orphans.push_back(it->file());
+                orphans.push_back(it->file());
             } else if (c->hash != hash) {
+#if 0
                 SG_LOG(SG_TERRASYNC, SG_DEBUG, "hash mismatch'" << it->file() );
                 // file exists, but hash mismatch, schedule update
                 if (!hash.empty()) {
                     SG_LOG(SG_TERRASYNC, SG_DEBUG, "file exists but hash is wrong for:" << it->file() );
                     SG_LOG(SG_TERRASYNC, SG_DEBUG, "on disk:" << hash << " vs in info:" << c->hash);
                 }
-
+#endif
                 toBeUpdated.push_back(it->file() );
             } else {
                 // file exists and hash is valid. If it's a directory,
                 // perform a recursive check.
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "file exists hash is good:" << it->file() );
                 if (c->type == ChildInfo::DirectoryType) {
                     HTTPDirectory* childDir = childDirectory(it->file());
                     childDir->updateChildrenBasedOnHash();
@@ -386,7 +399,6 @@ public:
                 continue;
             }
 
-            SG_LOG(SG_TERRASYNC,SG_DEBUG, "scheduling update for " << *it );
             if (cit->type == ChildInfo::FileType) {
                 _repository->updateFile(this, *it, cit->sizeInBytes);
             } else {
@@ -481,11 +493,13 @@ private:
 
             if( typeData == "version" ) {
                 if( tokens.size() < 2 ) {
-                    SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: missing version number in line '" << line << "'" );
+                    SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: missing version number in line '" << line << "'"
+                           << "\n\tparsing:" << p.utf8Str());
                     break;
                 }
                 if( tokens[1] != "1" ) {
-                    SG_LOG(SG_TERRASYNC, SG_WARN, "invalid .dirindex file: wrong version number '" << tokens[1] << "' (expected 1)" );
+                    SG_LOG(SG_TERRASYNC, SG_WARN, "invalid .dirindex file: wrong version number '" << tokens[1] << "' (expected 1)"
+                           << "\n\tparsing:" << p.utf8Str());
                     break;
                 }
                 continue; // version is good, continue
@@ -496,24 +510,27 @@ private:
             }
 
             if( typeData == "time" && tokens.size() > 1 ) {
-                SG_LOG(SG_TERRASYNC, SG_INFO, ".dirindex at '" << p.str() << "' timestamp: " << tokens[1] );
+               // SG_LOG(SG_TERRASYNC, SG_INFO, ".dirindex at '" << p.str() << "' timestamp: " << tokens[1] );
                 continue;
             }
 
             if( tokens.size() < 3 ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: not enough tokens in line '" << line << "' (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: not enough tokens in line '" << line << "' (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
             if (typeData != "f" && typeData != "d" ) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid type in line '" << line << "', expected 'd' or 'f', (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid type in line '" << line << "', expected 'd' or 'f', (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
             // security: prevent writing outside the repository via ../../.. filenames
             // (valid filenames never contain / - subdirectories have their own .dirindex)
             if ((tokens[1] == "..") || (tokens[1].find_first_of("/\\") != std::string::npos)) {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid filename in line '" << line << "', (ignoring line)" );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "malformed .dirindex file: invalid filename in line '" << line << "', (ignoring line)"
+                       << "\n\tparsing:" << p.utf8Str());
                 continue;
             }
 
@@ -656,6 +673,11 @@ void HTTPRepository::setInstalledCopyPath(const SGPath& copyPath)
     _d->installedCopyPath = copyPath;
 }
 
+std::string HTTPRepository::resultCodeAsString(ResultCode code)
+{
+    return innerResultCodeAsString(code);
+}
+    
 HTTPRepository::ResultCode
 HTTPRepository::failure() const
 {
@@ -668,7 +690,7 @@ HTTPRepository::failure() const
 
     void HTTPRepoGetRequest::cancel()
     {
-        _directory->repository()->http->cancelRequest(this, "Reposiotry cancelled");
+        _directory->repository()->http->cancelRequest(this, "Repository cancelled");
         _directory = 0;
     }
 
@@ -690,7 +712,7 @@ HTTPRepository::failure() const
                 file.reset(new SGBinaryFile(pathInRepo));
                 if (!file->open(SG_IO_OUT)) {
                   SG_LOG(SG_TERRASYNC, SG_WARN, "unable to create file " << pathInRepo);
-                  _directory->repository()->http->cancelRequest(this, "Unable to create output file");
+                    _directory->repository()->http->cancelRequest(this, "Unable to create output file:" + pathInRepo.utf8Str());
                 }
 
                 sha1_init(&hashContext);
@@ -706,12 +728,12 @@ HTTPRepository::failure() const
             if (responseCode() == 200) {
                 std::string hash = strutils::encodeHex(sha1_result(&hashContext), HASH_LENGTH);
                 _directory->didUpdateFile(fileName, hash, contentSize());
-                SG_LOG(SG_TERRASYNC, SG_DEBUG, "got file " << fileName << " in " << _directory->absolutePath());
             } else if (responseCode() == 404) {
                 SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file not found on server: " << fileName << " for " << _directory->absolutePath());
                 _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_FILE_NOT_FOUND);
             } else {
-                SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file download error on server: " << fileName << " for " << _directory->absolutePath() << ": " << responseCode() );
+                SG_LOG(SG_TERRASYNC, SG_WARN, "terrasync file download error on server: " << fileName << " for " << _directory->absolutePath() <<
+                       "\n\tserver responded: " << responseCode() << "/" << responseReason());
                 _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_HTTP);
             }
 
@@ -720,13 +742,18 @@ HTTPRepository::failure() const
 
         virtual void onFail()
         {
+            HTTPRepository::ResultCode code = HTTPRepository::REPO_ERROR_SOCKET;
+            if (responseCode() == -1) {
+                code = HTTPRepository::REPO_ERROR_CANCELLED;
+            }
+            
             file.reset();
             if (pathInRepo.exists()) {
                 pathInRepo.remove();
             }
 
             if (_directory) {
-                _directory->didFailToUpdateFile(fileName, HTTPRepository::REPO_ERROR_SOCKET);
+                _directory->didFailToUpdateFile(fileName, code);
                 _directory->repository()->finishedRequest(this);
             }
         }
@@ -1121,13 +1148,15 @@ HTTPRepository::failure() const
             RequestVector copyOfActive(activeRequests);
             RequestVector::iterator rq;
             for (rq = copyOfActive.begin(); rq != copyOfActive.end(); ++rq) {
-                //SG_LOG(SG_TERRASYNC, SG_DEBUG, "cancelling request for:" << (*rq)->url());
-                http->cancelRequest(*rq, "Repository updated failed");
+                http->cancelRequest(*rq, "Repository updated failed due to checksum error");
             }
 
-
             SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update repository:" << baseUrl
-                   << ", possibly modified during sync");
+                   << "\n\tchecksum failure for: " << relativePath 
+                   << "\n\tthis typically indicates the remote repository is corrupt or was being updated during the sync");
+        } else if (fileStatus == HTTPRepository::REPO_ERROR_CANCELLED) {
+            // if we were cancelled, don't report or log
+            return;
         }
 
         Failure f;
@@ -1135,7 +1164,8 @@ HTTPRepository::failure() const
         f.error = fileStatus;
         failures.push_back(f);
 
-        SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update entry:" << relativePath << " code:" << fileStatus);
+        SG_LOG(SG_TERRASYNC, SG_WARN, "failed to update entry:" << relativePath << " status/code: "
+               << innerResultCodeAsString(fileStatus) << "/" << fileStatus);
     }
 
 } // of namespace simgear

simgear/io/HTTPRepository.hxx

@@ -42,6 +42,7 @@ public:
         REPO_ERROR_CHECKSUM,
         REPO_ERROR_FILE_NOT_FOUND,
         REPO_ERROR_HTTP,
+        REPO_ERROR_CANCELLED,
         REPO_PARTIAL_UPDATE
     };
 
@@ -70,6 +71,9 @@ public:
      * repository. When a file is missing it will be copied from this tree.
      */
     void setInstalledCopyPath(const SGPath& copyPath);
+    
+    static std::string resultCodeAsString(ResultCode code);
+
 private:
     bool isBare() const;
 

simgear/io/http_repo_sync.cxx

@@ -5,8 +5,6 @@
 #include <signal.h>
 
 #include <iostream>
-#include <boost/foreach.hpp>
-
 
 #include <simgear/io/sg_file.hxx>
 #include <simgear/io/HTTPClient.hxx>

simgear/misc/strutils.hxx

@@ -72,10 +72,10 @@ namespace simgear {
 	std::string strip( const std::string& s );
 
 	/**
-         * Return a new string with any trailing \r and \n characters removed.
+         * Return a new string with any trailing \\r and \\n characters removed.
          * Typically useful to clean a CR-terminated line obtained from
-         * std::getline() which, upon reading CRLF (\r\n), discards the Line
-         * Feed character (\n) but leaves the Carriage Return (\r) in the
+         * std::getline() which, upon reading CRLF (\\r\\n), discards the Line
+         * Feed character (\\n) but leaves the Carriage Return (\\r) in the
          * string.
          * @param s Input string
          * @return The cleaned string
@@ -83,7 +83,7 @@ namespace simgear {
         std::string stripTrailingNewlines(const std::string& s);
 
 	/**
-         * Strip any trailing \r and \n characters from a string.
+         * Strip any trailing \\r and \\n characters from a string.
          * Should have slightly less overhead than stripTrailingNewlines().
          * @param s Input string (modified in-place)
          */
@@ -112,11 +112,12 @@ namespace simgear {
 	 * Produces a result similar to the perl and python functions of the
 	 * same name.
 	 * 
-	 * @param s The string to split into words,
-	 * @param sep Word delimiters.  If not specified then any whitespace is a separator,
-	 * @param maxsplit If given, splits at no more than maxsplit places,
-	 * resulting in at most maxsplit+1 words.
-	 * @return Array of words.
+	 * @param s         The string to split into words
+	 * @param sep       Word delimiters. If not specified then any whitespace is
+	 *                  a separator
+	 * @param maxsplit  If given, splits at no more than maxsplit places,
+	 *                  resulting in at most maxsplit+1 words
+	 * @return Array of words
 	 */
 	string_list
 	split( const std::string& s,
@@ -124,11 +125,11 @@ namespace simgear {
 	       int maxsplit = 0 );
 
       /**
-       * split a string on any of several characters. Commonly used to deal
+       * Split a string on any of several characters. Commonly used to deal
        * with strings containing whitespace, newlines. To parse CSS style
-       * string, use with '\n\t ,' as the seperator list.
+       * string, use with '\\n\\t ,' as the separator list.
        *
-       * Note consecutive seperators will not produce empty entries in the
+       * @note Consecutive separators will not produce empty entries in the
        * the result, i.e splitting 'a,b,,c,d' with a ',' will produce a result
        * with four entries, not five.
        */
@@ -226,14 +227,22 @@ namespace simgear {
     bool to_bool(const std::string& s);
 
     /**
-     * Like strcmp(), but for dotted versions strings NN.NN.NN
-     * any number of terms are supported.
-     * @return 0 if versions match, -ve number if v1 is lower, +ve if v1
-     * is greater
-     * @param maxComponents is the maximum number of components to look at.
-     * This can be used to ignore (say) the patch level by setting it to 2
+     * Compare dotted versions strings NN.NN.NN (analogous to strcmp())
+     *
+     * @note Any number of terms are supported.
+     *
+     * @param v1            First version
+     * @param v2            Second version
+     * @param maxComponents The maximum number of components to look at. This
+     *                      can be used to ignore (say) the patch level by
+     *                      setting it to 2
+     * @return 0 if versions match,
+     *         -ve number if @a v1 is lower,
+     *         +ve if @a v1 is greater
      */
-    int compare_versions(const std::string& v1, const std::string& v2, int maxComponents = 0);
+    int compare_versions( const std::string& v1,
+                          const std::string& v2,
+                          int maxComponents = 0 );
 
     /**
      * Convert a string to upper case.

simgear/package/Catalog.cxx

@@ -84,7 +84,7 @@ bool checkVersion(const std::string& aVersion, SGPropertyNode_ptr props)
 
 std::string redirectUrlForVersion(const std::string& aVersion, SGPropertyNode_ptr props)
 {
-    BOOST_FOREACH(SGPropertyNode* v, props->getChildren("alternate-version")) {
+    for (SGPropertyNode* v : props->getChildren("alternate-version")) {
         std::string s(v->getStringValue("version"));
         if (checkVersionString(aVersion, s)) {
             return  v->getStringValue("url");;
@@ -108,12 +108,12 @@ public:
     }
 
 protected:
-    virtual void gotBodyData(const char* s, int n)
+    void gotBodyData(const char* s, int n) override
     {
         m_buffer += std::string(s, n);
     }
 
-    virtual void onDone()
+    void onDone() override
     {
         if (responseCode() != 200) {
             Delegate::StatusCode code = Delegate::FAIL_DOWNLOAD;
@@ -139,7 +139,7 @@ protected:
 
         std::string ver(m_owner->root()->applicationVersion());
         if (!checkVersion(ver, props)) {
-            SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url() << ", version required " << ver);
+            SG_LOG(SG_GENERAL, SG_WARN, "downloaded catalog " << m_owner->url() << ", but version required " << ver);
 
             // check for a version redirect entry
             std::string url = redirectUrlForVersion(ver, props);
@@ -157,6 +157,13 @@ protected:
 
             return;
         } // of version check failed
+        
+        // validate what we downloaded, in case it's now corrupted
+        // (i.e someone uploaded bad XML data)
+        if (!m_owner->validatePackages()) {
+            m_owner->refreshComplete(Delegate::FAIL_VALIDATION);
+            return;
+        }
 
         // cache the catalog data, now we have a valid install root
         Dir d(m_owner->installRoot());
@@ -169,7 +176,13 @@ protected:
         m_owner->writeTimestamp();
         m_owner->refreshComplete(Delegate::STATUS_REFRESHED);
     }
-
+    
+    void onFail() override
+    {
+        // network level failure
+        SG_LOG(SG_GENERAL, SG_WARN, "catalog network failure for:" << m_owner->url());
+        m_owner->refreshComplete(Delegate::FAIL_DOWNLOAD);
+    }
 private:
 
     CatalogRef m_owner;
@@ -215,7 +228,8 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
 
     bool versionCheckOk = checkVersion(aRoot->applicationVersion(), props);
     if (!versionCheckOk) {
-        SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: need" << aRoot->applicationVersion());
+        SG_LOG(SG_GENERAL, SG_INFO, "catalog at:" << aPath << " failed version check: need version: "
+               << aRoot->applicationVersion());
         // keep the catalog but mark it as needing an update
     } else {
         SG_LOG(SG_GENERAL, SG_DEBUG, "creating catalog from:" << aPath);
@@ -226,7 +240,9 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
     c->parseProps(props);
     c->parseTimestamp();
 
-    if (versionCheckOk) {
+    if (!c->validatePackages()) {
+        c->changeStatus(Delegate::FAIL_VALIDATION);
+    } else if (versionCheckOk) {
         // parsed XML ok, mark status as valid
         c->changeStatus(Delegate::STATUS_SUCCESS);
     } else {
@@ -235,25 +251,44 @@ CatalogRef Catalog::createFromPath(Root* aRoot, const SGPath& aPath)
 
     return c;
 }
+    
+bool Catalog::validatePackages() const
+{
+    for (auto pack : packages()) {
+        if (!pack->validate()) {
+            SG_LOG(SG_GENERAL, SG_WARN, "Catalog " << id() << " failed validation due to invalid package:" << pack->id());
+            return false;
+        }
+    }
+    
+    return true;
+}
 
 bool Catalog::uninstall()
 {
     bool ok;
     bool atLeastOneFailure = false;
 
-    BOOST_FOREACH(PackageRef p, installedPackages()) {
-        ok = p->existingInstall()->uninstall();
-        if (!ok) {
-            SG_LOG(SG_GENERAL, SG_WARN, "uninstall of package " <<
-                p->id() << " failed");
-            // continue trying other packages, bailing out here
-            // gains us nothing
-            atLeastOneFailure = true;
+    try {
+        // clean uninstall of each airacft / package in turn. This is
+        // slightly overkill since we then nuke the entire catalog
+        // directory anyway
+        for (PackageRef p : installedPackages()) {
+            ok = p->existingInstall()->uninstall();
+            if (!ok) {
+                SG_LOG(SG_GENERAL, SG_WARN, "uninstall of package " <<
+                    p->id() << " failed");
+                // continue trying other packages, bailing out here
+                // gains us nothing
+                atLeastOneFailure = true;
+            }
         }
+    } catch (sg_exception& e) {
+        SG_LOG(SG_GENERAL, SG_WARN, "uninstall of catalog failed " << e.getMessage() << ", will clean-up directory");
+        atLeastOneFailure = true;
     }
 
-    Dir d(m_installRoot);
-    ok = d.remove(true /* recursive */);
+    ok = removeDirectory();
     if (!ok) {
         atLeastOneFailure = true;
     }
@@ -263,6 +298,15 @@ bool Catalog::uninstall()
     return ok;
 }
 
+bool Catalog::removeDirectory()
+{
+    Dir d(m_installRoot);
+    if (!m_installRoot.exists())
+        return true;
+    
+    return d.remove(true /* recursive */);
+}
+    
 PackageList const&
 Catalog::packages() const
 {
@@ -540,6 +584,20 @@ Delegate::StatusCode Catalog::status() const
     return m_status;
 }
 
+bool Catalog::isEnabled() const
+{
+    switch (m_status) {
+    case Delegate::STATUS_SUCCESS:
+    case Delegate::STATUS_REFRESHED:
+    case Delegate::STATUS_IN_PROGRESS:
+    // this is important so we can use Catalog aircraft in offline mode
+    case Delegate::FAIL_DOWNLOAD:
+        return true;
+    default:
+        return false;
+    }
+}
+    
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Catalog.hxx

@@ -134,6 +134,12 @@ public:
     SGPropertyNode* properties() const;
 
     Delegate::StatusCode status() const;
+    
+    /**
+     * is this Catalog usable? This may be false if the catalog is currently
+     * failing a version check or cannot be updated
+     */
+    bool isEnabled() const;
 
     typedef boost::function<void(Catalog*)> Callback;
 
@@ -149,7 +155,8 @@ private:
 
     class Downloader;
     friend class Downloader;
-
+    friend class Root;
+    
     void parseProps(const SGPropertyNode* aProps);
 
     void refreshComplete(Delegate::StatusCode aReason);
@@ -157,6 +164,17 @@ private:
     void parseTimestamp();
     void writeTimestamp();
 
+    /**
+     * @brief wipe the catalog directory from the disk
+     */
+    bool removeDirectory();
+    
+    /**
+     * @brief Helper to ensure all packages are at least somewhat valid, in terms
+     * of an ID, name and directory.
+     */
+    bool validatePackages() const;
+    
     std::string getLocalisedString(const SGPropertyNode* aRoot, const char* aName) const;
 
     void changeStatus(Delegate::StatusCode newStatus);

simgear/package/CatalogTest.cxx

@@ -61,6 +61,7 @@ std::string readFileIntoString(const SGPath& path)
 
 SGPath global_serverFilesRoot;
 unsigned int global_catalogVersion = 0;
+bool global_failRequests = false;
 
 class TestPackageChannel : public TestServerChannel
 {
@@ -71,6 +72,10 @@ public:
         state = STATE_IDLE;
         SGPath localPath(global_serverFilesRoot);
 
+        if (global_failRequests) {
+            closeWhenDone();
+            return;
+        }
 
         if (path == "/catalogTest1/catalog.xml") {
             if (global_catalogVersion > 0) {
@@ -79,6 +84,19 @@ public:
                 path = ss.str();
             }
         }
+        
+        if (path == "/catalogTestInvalid/catalog.xml") {
+            if (global_catalogVersion > 0) {
+                std::stringstream ss;
+                ss << "/catalogTestInvalid/catalog-v" << global_catalogVersion << ".xml";
+                path = ss.str();
+            }
+        }
+        
+        // return zip data for this computed URL
+        if (path.find("/catalogTest1/movies") == 0) {
+            path = "/catalogTest1/movies-data.zip";
+        }
 
         localPath.append(path);
 
@@ -119,6 +137,12 @@ void waitForUpdateComplete(HTTP::Client* cl, pkg::Root* root)
     std::cerr << "timed out" << std::endl;
 }
 
+template<class T>
+bool vectorContains(const std::vector<T>& vec, const T value)
+{
+    return std::find(vec.begin(), vec.end(), value) != vec.end();
+}
+
 int parseTest()
 {
     SGPath rootPath = simgear::Dir::current().path();
@@ -133,7 +157,7 @@ int parseTest()
     SG_CHECK_EQUAL(cat->description(), "First test catalog");
 
 // check the packages too
-    SG_CHECK_EQUAL(cat->packages().size(), 4);
+    SG_CHECK_EQUAL(cat->packages().size(), 5);
 
     pkg::PackageRef p1 = cat->packages().front();
     SG_CHECK_EQUAL(p1->catalog(), cat.ptr());
@@ -307,7 +331,7 @@ void testAddCatalog(HTTP::Client* cl)
     p.append("org.flightgear.test.catalog1");
     p.append("catalog.xml");
     SG_VERIFY(p.exists());
-    SG_CHECK_EQUAL(root->allPackages().size(), 4);
+    SG_CHECK_EQUAL(root->allPackages().size(), 5);
     SG_CHECK_EQUAL(root->catalogs().size(), 1);
 
     pkg::PackageRef p1 = root->getPackageById("alpha");
@@ -540,6 +564,374 @@ void testInstallTarPackage(HTTP::Client* cl)
     SG_VERIFY(p.exists());
 }
 
+void testInstallArchiveType(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("pkg_install_archive_type");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    // specify a test dir
+    root->setHTTPClient(cl);
+    
+    pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+    waitForUpdateComplete(cl, root);
+    
+    pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.movies");
+    SG_CHECK_EQUAL(p1->id(), "movies");
+    pkg::InstallRef ins = p1->install();
+    
+    SG_VERIFY(ins->isQueued());
+    
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(p1->isInstalled());
+    SG_VERIFY(p1->existingInstall() == ins);
+    
+    // verify on disk state
+    SGPath p(rootPath);
+    p.append("org.flightgear.test.catalog1");
+    p.append("Aircraft");
+    p.append("movies");
+    
+    SG_CHECK_EQUAL(p, ins->path());
+    
+    p.append("movie-list.json");
+    SG_VERIFY(p.exists());
+}
+
+void testDisableDueToVersion(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_disable_at_version");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    // bump version and refresh
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "9.1.2"));
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_CHECK_EQUAL(root->allCatalogs().size(), 1);
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(root->catalogs().size(), 0);
+
+        root->setHTTPClient(cl);
+        root->refresh();
+        waitForUpdateComplete(cl, root);
+        SG_CHECK_EQUAL(root->allCatalogs().size(), 1);
+
+        
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VERSION);
+        SG_VERIFY(!cat->isEnabled());
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it == enabledCats.end());
+        SG_CHECK_EQUAL(enabledCats.size(), 0);
+        
+        auto allCats = root->allCatalogs();
+        auto j = std::find(allCats.begin(), allCats.end(), cat);
+        SG_VERIFY(j != allCats.end());
+
+        SG_CHECK_EQUAL(allCats.size(), 1);
+
+        // ensure existing package is still installed but not directly list
+        
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k == packs.end());
+    }
+}
+
+void testVersionMigrate(HTTP::Client* cl)
+{
+    global_catalogVersion = 2; // version which has migration info
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_migrate_version");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+        
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    // bump version and refresh
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "10.1.2"));
+        root->setHTTPClient(cl);
+        
+        // this should cause auto-migration
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::STATUS_REFRESHED);
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        SG_CHECK_EQUAL(cat->url(), "http://localhost:2000/catalogTest1/catalog-v10.xml");
+
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it != enabledCats.end());
+        
+        // ensure existing package is still installed
+        
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k != packs.end());
+    }
+}
+
+void testOfflineMode(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_offline_mode");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        root->setHTTPClient(cl);
+        
+        pkg::CatalogRef c = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(c->isEnabled());
+        
+        // install a package
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        pkg::InstallRef ins = p1->install();
+        SG_VERIFY(ins->isQueued());
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(p1->isInstalled());
+    }
+    
+    global_failRequests = true;
+    
+    {
+        pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+        SG_CHECK_EQUAL(root->catalogs().size(), 1);
+
+        root->setHTTPClient(cl);
+        root->refresh(true);
+        waitForUpdateComplete(cl, root);
+        SG_CHECK_EQUAL(root->catalogs().size(), 1);
+
+        pkg::CatalogRef cat = root->getCatalogById("org.flightgear.test.catalog1");
+        SG_VERIFY(cat->isEnabled());
+        SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_DOWNLOAD);
+        SG_CHECK_EQUAL(cat->id(), "org.flightgear.test.catalog1");
+        
+        auto enabledCats = root->catalogs();
+        auto it = std::find(enabledCats.begin(), enabledCats.end(), cat);
+        SG_VERIFY(it != enabledCats.end());
+        
+        // ensure existing package is still installed
+        pkg::PackageRef p1 = root->getPackageById("org.flightgear.test.catalog1.b737-NG");
+        SG_VERIFY(p1 !=  pkg::PackageRef());
+        SG_CHECK_EQUAL(p1->id(), "b737-NG");
+        
+        auto packs = root->allPackages();
+        auto k = std::find(packs.begin(), packs.end(), p1);
+        SG_VERIFY(k != packs.end());
+    }
+    
+    global_failRequests = false;
+}
+
+int parseInvalidTest()
+{
+    SGPath rootPath = simgear::Dir::current().path();
+    rootPath.append("testRoot");
+    pkg::Root* root = new pkg::Root(rootPath, "8.1.12");
+    pkg::CatalogRef cat = pkg::Catalog::createFromPath(root, SGPath(SRC_DIR "/catalogTestInvalid"));
+    SG_VERIFY(cat.valid());
+
+    SG_CHECK_EQUAL(cat->status(), pkg::Delegate::FAIL_VALIDATION);
+ 
+    return 0;
+}
+
+void removeInvalidCatalog(HTTP::Client* cl)
+{
+    global_catalogVersion = 0; // fetch the good version
+    
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_remove_invalid");
+    simgear::Dir pd(rootPath);
+    pd.removeChildren();
+    
+    pkg::RootRef root(new pkg::Root(rootPath, "8.1.2"));
+    root->setHTTPClient(cl);
+    
+    // another catalog so the dicts are non-empty
+    pkg::CatalogRef anotherCat = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTest1/catalog.xml");
+
+    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 remove it
+    root->removeCatalog(c);
+    SG_VERIFY(!vectorContains(root->catalogs(), c));
+    SG_VERIFY(!vectorContains(root->allCatalogs(), c));
+    c.clear(); // drop the catalog
+
+    // re-add it again, and remove it again
+    {
+        pkg::CatalogRef c2 = pkg::Catalog::createFromUrl(root.ptr(), "http://localhost:2000/catalogTestInvalid/catalog.xml");
+        waitForUpdateComplete(cl, root);
+        SG_VERIFY(!c2->isEnabled());
+        SG_VERIFY(c2->status() == pkg::Delegate::FAIL_VALIDATION);
+        SG_VERIFY(!vectorContains(root->catalogs(), c2));
+        SG_VERIFY(vectorContains(root->allCatalogs(), c2));
+        
+        // now remove it
+        root->removeCatalog(c2);
+        SG_VERIFY(!vectorContains(root->catalogs(), c2));
+        SG_VERIFY(!vectorContains(root->allCatalogs(), c2));
+    }
+    
+    // only the other catalog (testCatalog should be left)
+    SG_VERIFY(root->allCatalogs().size() == 1);
+    SG_VERIFY(root->catalogs().size() == 1);
+    
+    SG_LOG(SG_GENERAL, SG_INFO, "Remove invalid catalog test passeed");
+}
+
+void updateInvalidToValid(HTTP::Client* cl)
+{
+    global_catalogVersion = 0;
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_update_invalid_to_valid");
+    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 refrsh the good one
+    global_catalogVersion = 2;
+    c->refresh();
+    waitForUpdateComplete(cl, root);
+    SG_VERIFY(c->isEnabled());
+    SG_VERIFY(c->status() == pkg::Delegate::STATUS_REFRESHED);
+    SG_VERIFY(vectorContains(root->catalogs(), c));
+
+}
+
+void updateValidToInvalid(HTTP::Client* cl)
+{
+    global_catalogVersion = 2; // fetch the good version
+    
+    SGPath rootPath(simgear::Dir::current().path());
+    rootPath.append("cat_update_valid_to_invalid");
+    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::STATUS_REFRESHED);
+    SG_VERIFY(vectorContains(root->catalogs(), c));
+    SG_VERIFY(vectorContains(root->allCatalogs(), c));
+    
+    // now refrsh the 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));
+}
+
+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[])
 {
@@ -554,6 +946,8 @@ int main(int argc, char* argv[])
 
     parseTest();
 
+    parseInvalidTest();
+    
     testInstallPackage(&cl);
 
     testUninstall(&cl);
@@ -563,7 +957,21 @@ int main(int argc, char* argv[])
     testRefreshCatalog(&cl);
 
     testInstallTarPackage(&cl);
-
-    std::cout << "Successfully passed all tests!" << std::endl;
+    
+    testInstallArchiveType(&cl);
+    
+    testDisableDueToVersion(&cl);
+    
+    testOfflineMode(&cl);
+    
+    testVersionMigrate(&cl);
+    
+    updateInvalidToValid(&cl);
+    updateValidToInvalid(&cl);
+    updateInvalidToInvalid(&cl);
+    
+    removeInvalidCatalog(&cl);
+    
+    SG_LOG(SG_GENERAL, SG_INFO, "Successfully passed all tests!");
     return EXIT_SUCCESS;
 }

simgear/package/Delegate.hxx

@@ -54,6 +54,7 @@ public:
         FAIL_VERSION,       ///< version check mismatch
         FAIL_NOT_FOUND,     ///< package URL returned a 404
         FAIL_HTTP_FORBIDDEN, ///< URL returned a 403. Marked specially to catch rate-limiting
+        FAIL_VALIDATION,    ///< catalog or package failed to validate
         STATUS_REFRESHED,
         USER_CANCELLED
     } StatusCode;

simgear/package/Install.cxx

@@ -57,6 +57,10 @@ public:
             throw sg_exception("no package download URLs");
         }
 
+        if (m_owner->package()->properties()->hasChild("archive-type")) {
+            setArchiveTypeFromExtension(m_owner->package()->properties()->getStringValue("archive-type"));
+        }
+        
         // TODO randomise order of m_urls
 
         m_extractPath = aOwner->path().dir();
@@ -165,7 +169,12 @@ protected:
 
         // build a path like /path/to/packages/org.some.catalog/Aircraft/extract_xxxx/MyAircraftDir
         SGPath extractedPath = m_extractPath;
-        extractedPath.append(m_owner->package()->dirName());
+        if (m_owner->package()->properties()->hasChild("archive-path")) {
+            extractedPath.append(m_owner->package()->properties()->getStringValue("archive-path"));
+        } else {
+            extractedPath.append(m_owner->package()->dirName());
+        }
+
 
         // rename it to path/to/packages/org.some.catalog/Aircraft/MyAircraftDir
         bool ok = extractedPath.rename(m_owner->path());
@@ -175,6 +184,8 @@ protected:
         }
 
         // extract_xxxx directory is now empty, so remove it
+        // (note it might not be empty if the archive contained some other
+        // files, but we delete those in such a case
         if (m_extractPath.exists()) {
             simgear::Dir(m_extractPath).remove();
         }
@@ -196,7 +207,22 @@ protected:
     }
 
 private:
-
+    void setArchiveTypeFromExtension(const std::string& ext)
+    {
+        if (ext.empty())
+            return;
+        
+        if (ext == "zip") {
+            m_archiveType = ZIP;
+            return;
+        }
+        
+        if ((ext == "tar.gz") || (ext == "tgz")) {
+            m_archiveType = TAR_GZ;
+            return;
+        }
+    }
+    
     void extractCurrentFile(unzFile zip, char* buffer, size_t bufferSize)
     {
         unz_file_info fileInfo;
@@ -262,14 +288,22 @@ private:
     {
         const std::string u(url());
         const size_t ul(u.length());
-        if (u.rfind(".zip") == (ul - 4)) {
-            return extractUnzip();
+        
+        if (m_archiveType == AUTO_DETECT) {
+            if (u.rfind(".zip") == (ul - 4)) {
+                m_archiveType = ZIP;
+            } else if (u.rfind(".tar.gz") == (ul - 7)) {
+                m_archiveType = TAR_GZ;
+            }
+            // we will fall through to the error case now
         }
-
-        if (u.rfind(".tar.gz") == (ul - 7)) {
+        
+        if (m_archiveType == ZIP) {
+            return extractUnzip();
+        } else if (m_archiveType == TAR_GZ) {
             return extractTar();
         }
-
+        
         SG_LOG(SG_IO, SG_WARN, "unsupported archive format:" << u);
         return false;
     }
@@ -330,7 +364,14 @@ private:
         m_owner->installResult(aReason);
     }
 
+    enum ArchiveType {
+        AUTO_DETECT = 0,
+        ZIP,
+        TAR_GZ
+    };
+    
     InstallRef m_owner;
+    ArchiveType m_archiveType = AUTO_DETECT;
     string_list m_urls;
     SG_MD5_CTX m_md5;
     std::string m_buffer;

simgear/package/Package.cxx

@@ -497,6 +497,23 @@ Package::PreviewVec Package::previewsFromProps(const SGPropertyNode_ptr& ptr) co
     return result;
 }
 
+bool Package::validate() const
+{
+    if (m_id.empty())
+        return false;
+    
+    std::string nm(m_props->getStringValue("name"));
+    if (nm.empty())
+        return false;
+    
+    std::string dir(m_props->getStringValue("dir"));
+    if (dir.empty())
+        return false;
+    
+    return true;
+}
+
+    
 } // of namespace pkg
 
 } // of namespace simgear

simgear/package/Package.hxx

@@ -99,10 +99,8 @@ public:
     /**
      * human-readable name - note this is probably not localised,
      * although this is not ruled out for the future.
-     *
-     * Deprecated - please use nameForVariant
      */
-    SG_DEPRECATED(std::string name() const);
+    std::string name() const;
 
     /**
      * Human readable name of a variant
@@ -112,12 +110,9 @@ public:
     std::string nameForVariant(const unsigned int vIndex) const;
 
     /**
-     * syntactic sugar to get the localised description
-     *
-     * Deprecated - please use getLocalisedProp to get the variant-specific
-     * description.
+     * syntactic sugar to get the localised description of the main aircraft
      */
-    SG_DEPRECATED(std::string description() const);
+    std::string description() const;
 
     /**
      * access the raw property data in the package
@@ -225,6 +220,11 @@ private:
 
     void updateFromProps(const SGPropertyNode* aProps);
 
+    /**
+     * @brief check the Package passes some basic consistence checks
+     */
+    bool validate() const;
+    
     std::string getLocalisedString(const SGPropertyNode* aRoot, const char* aName) const;
 
     PreviewVec previewsFromProps(const SGPropertyNode_ptr& ptr) const;

simgear/package/Root.cxx

@@ -408,15 +408,13 @@ Root::Root(const SGPath& aPath, const std::string& aVersion) :
     }
     
     for (SGPath c : dir.children(Dir::TYPE_DIR | Dir::NO_DOT_OR_DOTDOT)) {
-        CatalogRef cat = Catalog::createFromPath(this, c);
-        if (cat) {
-            if (cat->status() == Delegate::STATUS_SUCCESS) {
-                d->catalogs[cat->id()] = cat;
-            } else {
-                // catalog has problems, such as needing an update
-                // keep it out of the main collection for now
-                d->disabledCatalogs.push_back(cat);
-            }
+        // note this will set the catalog status, which will insert into
+        // disabled catalogs automatically if necesary
+        auto cat = Catalog::createFromPath(this, c);
+        if (cat && cat->isEnabled()) {
+            d->catalogs.insert({cat->id(), cat});
+        } else if (cat) {
+            SG_LOG(SG_GENERAL, SG_WARN, "Package-Root init: catalog is disabled: " << cat->id());
         }
     } // of child directories iteration
 }
@@ -438,9 +436,15 @@ std::string Root::applicationVersion() const
 
 CatalogRef Root::getCatalogById(const std::string& aId) const
 {
-    CatalogDict::const_iterator it = d->catalogs.find(aId);
+    auto it = d->catalogs.find(aId);
     if (it == d->catalogs.end()) {
-        return NULL;
+        // check disabled catalog list
+        auto j = std::find_if(d->disabledCatalogs.begin(), d->disabledCatalogs.end(),
+                              [aId](const CatalogRef& cat) { return cat->id() == aId; });
+        if (j != d->disabledCatalogs.end()) {
+            return *j;
+        }
+        return nullptr;
     }
 
     return it->second;
@@ -484,6 +488,13 @@ CatalogList Root::catalogs() const
 
     return r;
 }
+    
+CatalogList Root::allCatalogs() const
+{
+    CatalogList r = catalogs();
+    r.insert(r.end(), d->disabledCatalogs.begin(), d->disabledCatalogs.end());
+    return r;
+}
 
 PackageList
 Root::allPackages() const
@@ -544,12 +555,9 @@ void Root::refresh(bool aForce)
 
     toRefresh.insert(toRefresh.end(), d->disabledCatalogs.begin(),
                      d->disabledCatalogs.end());
-
-
-    CatalogList::iterator j = toRefresh.begin();
-    for (; j != toRefresh.end(); ++j) {
-        (*j)->refresh();
-        didStartAny =  true;
+    for (auto cat : toRefresh) {
+        cat->refresh();
+        didStartAny = true;
     }
 
     if (!didStartAny) {
@@ -663,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) {
@@ -677,23 +685,20 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
         d->catalogs.insert(catIt, CatalogDict::value_type(aCat->id(), aCat));
 
         // catalog might have been previously disabled, let's remove in that case
-        CatalogList::iterator j = std::find(d->disabledCatalogs.begin(),
-                                            d->disabledCatalogs.end(),
-                                            aCat);
+        auto j = std::find(d->disabledCatalogs.begin(),
+                           d->disabledCatalogs.end(),
+                           aCat);
         if (j != d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_INFO, "re-enabling disabled catalog:" << aCat->id());
             d->disabledCatalogs.erase(j);
         }
     }
 
-    if ((aReason != Delegate::STATUS_REFRESHED) &&
-        (aReason != Delegate::STATUS_IN_PROGRESS) &&
-        (aReason != Delegate::STATUS_SUCCESS))
-    {
+    if (!aCat->isEnabled()) {
         // catalog has errors, disable it
-        CatalogList::iterator j = std::find(d->disabledCatalogs.begin(),
-                                            d->disabledCatalogs.end(),
-                                            aCat);
+        auto j = std::find(d->disabledCatalogs.begin(),
+                           d->disabledCatalogs.end(),
+                           aCat);
         if (j == d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_INFO, "disabling catalog:" << aCat->id());
             d->disabledCatalogs.push_back(aCat);
@@ -703,13 +708,39 @@ void Root::catalogRefreshStatus(CatalogRef aCat, Delegate::StatusCode aReason)
         if (catIt != d->catalogs.end()) {
             d->catalogs.erase(catIt);
         }
-    } // of catalog has errors case
+    } // of catalog is disabled
 
     if (d->refreshing.empty()) {
         d->fireRefreshStatus(CatalogRef(), Delegate::STATUS_REFRESHED);
         d->firePackagesChanged();
     }
 }
+    
+bool Root::removeCatalog(CatalogRef cat)
+{
+    if (!cat)
+        return false;
+    
+    // normal remove path
+    if (!cat->id().empty()) {
+        return removeCatalogById(cat->id());
+    }
+    
+    if (!cat->removeDirectory()) {
+        SG_LOG(SG_GENERAL, SG_WARN, "removeCatalog: failed to remove directory " << cat->installRoot());
+    }
+    auto it = std::find(d->disabledCatalogs.begin(),
+                       d->disabledCatalogs.end(),
+                       cat);
+    if (it != d->disabledCatalogs.end()) {
+        d->disabledCatalogs.erase(it);
+    }
+    
+    // notify that a catalog is being removed
+    d->firePackagesChanged();
+    
+    return true;
+}
 
 bool Root::removeCatalogById(const std::string& aId)
 {
@@ -718,13 +749,8 @@ bool Root::removeCatalogById(const std::string& aId)
     CatalogDict::iterator catIt = d->catalogs.find(aId);
     if (catIt == d->catalogs.end()) {
         // check the disabled list
-        CatalogList::iterator j = d->disabledCatalogs.begin();
-        for (; j != d->disabledCatalogs.end(); ++j) {
-            if ((*j)->id() == aId) {
-                break;
-            }
-        }
-
+        auto j = std::find_if(d->disabledCatalogs.begin(), d->disabledCatalogs.end(),
+                              [aId](const CatalogRef& cat) { return cat->id() == aId; });
         if (j == d->disabledCatalogs.end()) {
             SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: no catalog with id:" << aId);
             return false;
@@ -738,10 +764,10 @@ bool Root::removeCatalogById(const std::string& aId)
         d->catalogs.erase(catIt);
     }
 
-    bool ok = cat->uninstall();
+    bool ok = cat->removeDirectory();
     if (!ok) {
         SG_LOG(SG_GENERAL, SG_WARN, "removeCatalogById: catalog :" << aId
-            << "failed to uninstall");
+            << "failed to remove directory");
     }
 
     // notify that a catalog is being removed
@@ -752,6 +778,11 @@ bool Root::removeCatalogById(const std::string& aId)
 
 void Root::requestThumbnailData(const std::string& aUrl)
 {
+    if (aUrl.empty()) {
+        SG_LOG(SG_GENERAL, SG_DEV_WARN, "requestThumbnailData: empty URL requested");
+        return;
+    }
+
     auto it = d->thumbnailCache.find(aUrl);
     if (it == d->thumbnailCache.end()) {
         bool cachedOnDisk = d->checkPersistentCache(aUrl);

simgear/package/Root.hxx

@@ -69,7 +69,13 @@ public:
     std::string getLocale() const;
     
     CatalogList catalogs() const;
-        
+    
+    /**
+     * retrive all catalogs, including currently disabled ones
+     */
+    CatalogList allCatalogs() const;
+    
+    
     void setMaxAgeSeconds(unsigned int seconds);
     unsigned int maxAgeSeconds() const;
     
@@ -134,6 +140,13 @@ public:
      */
     bool removeCatalogById(const std::string& aId);
     
+    /**
+     * remove a catalog by reference (used when abandoning installs, since
+     * there may not be a valid catalog Id)
+     */
+    bool removeCatalog(CatalogRef cat);
+
+    
     /**
      * request thumbnail data from the cache / network
      */

simgear/package/catalogTest1/catalog-v10.xml

@@ -0,0 +1,178 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog1</id>
+    <description>First test catalog</description>
+    <url>http://localhost:2000/catalogTest1/catalog-v10.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>10.0.*</version>
+    <version>10.1.*</version>
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">8</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+
+    </package>
+
+    <package>
+        <id>c172p</id>
+        <name>Cessna 172-P</name>
+        <dir>c172p</dir>
+        <description>A plane made by Cessna on Jupiter</description>
+        <revision type="int">42</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+        <author>Standard author</author>
+
+        <tag>cessna</tag>
+        <tag>ga</tag>
+        <tag>piston</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">3</FDM>
+          <systems type="int">4</systems>
+          <model type="int">5</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <!-- local dependency -->
+        <depends>
+            <id>org.flightgear.test.catalog1.common-sounds</id>
+            <revision>10</revision>
+        </depends>
+
+        <preview>
+          <type>exterior</type>
+          <path>thumb-exterior.png</path>
+          <url>http://foo.bar.com/thumb-exterior.png</url>
+        </preview>
+
+        <preview>
+          <type>panel</type>
+          <path>thumb-panel.png</path>
+          <url>http://foo.bar.com/thumb-panel.png</url>
+        </preview>
+
+        <preview>
+          <path>thumb-something.png</path>
+          <url>http://foo.bar.com/thumb-something.png</url>
+        </preview>
+
+        <variant>
+            <id>c172p-2d-panel</id>
+            <name>C172 with 2d panel only</name>
+        </variant>
+
+        <variant>
+            <id>c172p-floats</id>
+            <name>C172 with floats</name>
+            <description>A plane with floats</description>
+            <author>Floats variant author</author>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-floats.png</path>
+              <url>http://foo.bar.com/thumb-exterior-floats.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+
+            <thumbnail>http://foo.bar.com/thumb-floats.png</thumbnail>
+            <thumbnail-path>thumb-floats.png</thumbnail-path>
+        </variant>
+
+        <variant>
+            <id>c172p-skis</id>
+            <name>C172 with skis</name>
+            <description>A plane with skis</description>
+
+            <variant-of>c172p</variant-of>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-skis.png</path>
+              <url>http://foo.bar.com/thumb-exterior-skis.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r</id>
+            <name>C172R</name>
+            <description>Equally good version of the C172</description>
+            <variant-of>_package_</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r-floats</id>
+            <name>C172R-floats</name>
+            <description>Equally good version of the C172 with floats</description>
+            <variant-of>c172r</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <md5>ec0e2ffdf98d6a5c05c77445e5447ff5</md5>
+        <url>http://localhost:2000/catalogTest1/c172p.zip</url>
+
+        <thumbnail>http://foo.bar.com/thumb-exterior.png</thumbnail>
+        <thumbnail-path>exterior.png</thumbnail-path>
+    </package>
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/catalogTest1/b737.tar.gz</url>
+      </package>
+
+    <package>
+        <id>common-sounds</id>
+        <name>Common sound files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>sounds</dir>
+        <url>http://localhost:2000/catalogTest1/common-sounds.zip</url>
+        <file-size-bytes>360</file-size-bytes>
+        <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
+    </package>
+</PropertyList>

simgear/package/catalogTest1/catalog-v2.xml

@@ -10,6 +10,11 @@
     <version>8.0.0</version>
     <version>8.2.0</version>
 
+    <alternate-version>
+      <version>10.*.*</version>
+      <url>http://localhost:2000/catalogTest1/catalog-v10.xml</url>
+    </alternate-version>
+    
     <package>
         <id>alpha</id>
         <name>Alpha package</name>

simgear/package/catalogTest1/catalog-v9.xml

@@ -0,0 +1,178 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog1</id>
+    <description>First test catalog</description>
+    <url>http://localhost:2000/catalogTest1/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>9.1.*</version>
+
+    <package>
+        <id>alpha</id>
+        <name>Alpha package</name>
+        <revision type="int">8</revision>
+        <file-size-bytes type="int">593</file-size-bytes>
+
+        <md5>a469c4b837f0521db48616cfe65ac1ea</md5>
+        <url>http://localhost:2000/catalogTest1/alpha.zip</url>
+
+        <dir>alpha</dir>
+
+    </package>
+
+    <package>
+        <id>c172p</id>
+        <name>Cessna 172-P</name>
+        <dir>c172p</dir>
+        <description>A plane made by Cessna on Jupiter</description>
+        <revision type="int">42</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+        <author>Standard author</author>
+
+        <tag>cessna</tag>
+        <tag>ga</tag>
+        <tag>piston</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">3</FDM>
+          <systems type="int">4</systems>
+          <model type="int">5</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <!-- local dependency -->
+        <depends>
+            <id>org.flightgear.test.catalog1.common-sounds</id>
+            <revision>10</revision>
+        </depends>
+
+        <preview>
+          <type>exterior</type>
+          <path>thumb-exterior.png</path>
+          <url>http://foo.bar.com/thumb-exterior.png</url>
+        </preview>
+
+        <preview>
+          <type>panel</type>
+          <path>thumb-panel.png</path>
+          <url>http://foo.bar.com/thumb-panel.png</url>
+        </preview>
+
+        <preview>
+          <path>thumb-something.png</path>
+          <url>http://foo.bar.com/thumb-something.png</url>
+        </preview>
+
+        <variant>
+            <id>c172p-2d-panel</id>
+            <name>C172 with 2d panel only</name>
+        </variant>
+
+        <variant>
+            <id>c172p-floats</id>
+            <name>C172 with floats</name>
+            <description>A plane with floats</description>
+            <author>Floats variant author</author>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-floats.png</path>
+              <url>http://foo.bar.com/thumb-exterior-floats.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+
+            <thumbnail>http://foo.bar.com/thumb-floats.png</thumbnail>
+            <thumbnail-path>thumb-floats.png</thumbnail-path>
+        </variant>
+
+        <variant>
+            <id>c172p-skis</id>
+            <name>C172 with skis</name>
+            <description>A plane with skis</description>
+
+            <variant-of>c172p</variant-of>
+
+            <preview>
+              <type>exterior</type>
+              <path>thumb-exterior-skis.png</path>
+              <url>http://foo.bar.com/thumb-exterior-skis.png</url>
+            </preview>
+
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r</id>
+            <name>C172R</name>
+            <description>Equally good version of the C172</description>
+            <variant-of>_package_</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <variant>
+            <id>c172r-floats</id>
+            <name>C172R-floats</name>
+            <description>Equally good version of the C172 with floats</description>
+            <variant-of>c172r</variant-of>
+            <preview>
+              <type>panel</type>
+              <path>thumb-panel.png</path>
+              <url>http://foo.bar.com/thumb-panel.png</url>
+            </preview>
+        </variant>
+
+        <md5>ec0e2ffdf98d6a5c05c77445e5447ff5</md5>
+        <url>http://localhost:2000/catalogTest1/c172p.zip</url>
+
+        <thumbnail>http://foo.bar.com/thumb-exterior.png</thumbnail>
+        <thumbnail-path>exterior.png</thumbnail-path>
+    </package>
+
+    <package>
+        <id>b737-NG</id>
+        <name>Boeing 737 NG</name>
+        <dir>b737NG</dir>
+        <description>A popular twin-engined narrow body jet</description>
+        <revision type="int">111</revision>
+        <file-size-bytes type="int">860</file-size-bytes>
+
+        <tag>boeing</tag>
+        <tag>jet</tag>
+        <tag>ifr</tag>
+
+        <rating>
+          <FDM type="int">5</FDM>
+          <systems type="int">5</systems>
+          <model type="int">4</model>
+          <cockpit type="int">4</cockpit>
+        </rating>
+
+        <md5>a94ca5704f305b90767f40617d194ed6</md5>
+        <url>http://localhost:2000/catalogTest1/b737.tar.gz</url>
+      </package>
+
+    <package>
+        <id>common-sounds</id>
+        <name>Common sound files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>sounds</dir>
+        <url>http://localhost:2000/catalogTest1/common-sounds.zip</url>
+        <file-size-bytes>360</file-size-bytes>
+        <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
+    </package>    
+</PropertyList>

simgear/package/catalogTest1/catalog.xml

@@ -177,4 +177,18 @@
         <file-size-bytes>360</file-size-bytes>
         <md5>acf9eb89cf396eb42f8823d9cdf17584</md5>
     </package>
+
+
+    <package>
+        <id>movies</id>
+        <name>movies files for test catalog aircraft</name>
+        <revision>10</revision>
+        <dir>movies</dir>
+        <!-- url has no file extension, instead we set arcive-type -->
+        <url>http://localhost:2000/catalogTest1/movies?wierd=foo;bar=thing</url>
+        <archive-type>zip</archive-type>
+        <archive-path>movies_6789</archive-path>
+        <file-size-bytes>232</file-size-bytes>
+        <md5>e5f89c3f1ed1bdda16174c868f3c7b30</md5>
+    </package>
 </PropertyList>

simgear/package/catalogTest1/movies_6789/movie-list.json

@@ -0,0 +1,3 @@
+{
+    "id": "awesomething"
+}

simgear/package/catalogTestInvalid/catalog-v2.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog-invalid</id>
+    <description>Invalid test catalog</description>
+    <url>http://localhost:2000/catalogTestInvalid/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>8.1.*</version>
+    <version>8.0.0</version>
+    <version>8.2.0</version>
+
+    <package>
+        <id>alpha</id>
+        <!-- fixed now -->
+        <name>Alpha aircraft</name>
+        <dir>alpha</dir>
+
+    </package>
+
+   
+</PropertyList>

simgear/package/catalogTestInvalid/catalog-v3.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog-invalid</id>
+    <description>Invalid test catalog</description>
+    <url>http://localhost:2000/catalogTestInvalid/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>8.1.*</version>
+    <version>8.0.0</version>
+    <version>8.2.0</version>
+
+    <package>
+        <id>alpha</id>
+        <!-- missing name -->
+        <dir>alpha</dir>
+
+    </package>
+
+   
+</PropertyList>

simgear/package/catalogTestInvalid/catalog.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0"?>
+
+<PropertyList>
+    <id>org.flightgear.test.catalog-invalid</id>
+    <description>Invalid test catalog</description>
+    <url>http://localhost:2000/catalogTestInvalid/catalog.xml</url>
+    <catalog-version>4</catalog-version>
+
+    <version>8.1.*</version>
+    <version>8.0.0</version>
+    <version>8.2.0</version>
+
+    <package>
+        <id>alpha</id>
+        <!-- missing name -->
+        <dir>alpha</dir>
+
+    </package>
+
+   
+</PropertyList>

simgear/props/PropertyBasedMgr.cxx

@@ -1,4 +1,5 @@
-// Base class for property controlled subsystems
+///@file
+/// Base class for property controlled subsystems
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //

simgear/props/PropertyBasedMgr.hxx

@@ -1,4 +1,5 @@
-// Base class for property controlled subsystems
+///@file
+/// Base class for property controlled subsystems
 //
 // Copyright (C) 2012  Thomas Geymayer <tomgey@gmail.com>
 //
@@ -33,10 +34,10 @@ namespace simgear
     public SGPropertyChangeListener
   {
     public:
-      virtual void init();
-      virtual void shutdown();
+      void init() override;
+      void shutdown() override;
 
-      virtual void update (double delta_time_sec);
+      void update (double delta_time_sec) override;
 
       /**
        * Create a new PropertyBasedElement
@@ -88,10 +89,10 @@ namespace simgear
                         ElementFactory element_factory );
       virtual ~PropertyBasedMgr() = 0;
 
-      virtual void childAdded( SGPropertyNode * parent,
-                               SGPropertyNode * child );
-      virtual void childRemoved( SGPropertyNode * parent,
-                                 SGPropertyNode * child );
+      void childAdded( SGPropertyNode * parent,
+                       SGPropertyNode * child ) override;
+      void childRemoved( SGPropertyNode * parent,
+                         SGPropertyNode * child ) override;
 
       virtual void elementCreated(PropertyBasedElementPtr element) {}
 

simgear/sound/sample.hxx

@@ -23,7 +23,7 @@
 // $Id$
 
 /**
- * \file audio sample.hxx
+ * \file
  * Provides a audio sample encapsulation
  */
 

simgear/sound/soundmgr_aeonwave.cxx

@@ -140,6 +140,7 @@ SGSoundMgr::SGSoundMgr() :
     _changed(true),
     _volume(0.0),
     _velocity(SGVec3d::zeros()),
+    _bad_doppler(false),
     _renderer("unknown"),
     _vendor("unknown")
 {

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)

simgear/structure/event_mgr.cxx

@@ -10,6 +10,11 @@ void SGEventMgr::add(const std::string& name, SGCallback* cb,
                      double interval, double delay,
                      bool repeat, bool simtime)
 {
+    // Prevent Nasal from attempting to add timers after the subsystem has been
+    // shut down.
+    if (_shutdown)
+        return;
+
     // Clamp the delay value to 1 usec, so that user code can use
     // "zero" as a synonym for "next frame".
     if(delay <= 0) delay = 1e-6;
@@ -39,14 +44,15 @@ void SGTimer::run()
 }
 
 SGEventMgr::SGEventMgr() :
-    _inited(false)
+    _inited(false),
+    _shutdown(false)
 {
     
 }
 
 SGEventMgr::~SGEventMgr()
 {
-    
+    _shutdown = true;
 }
 
 void SGEventMgr::unbind()
@@ -63,13 +69,17 @@ void SGEventMgr::init()
 		return;
     }
 	
+    // The event manager dtor and ctor are not called on reset, so reset the flag here.
+    _shutdown = false;
+
     _inited = true;
 }
 
 void SGEventMgr::shutdown()
 {
     _inited = false;
-    
+    _shutdown = true;
+
     _simQueue.clear();
     _rtQueue.clear();
 }

simgear/structure/event_mgr.hxx

@@ -130,7 +130,7 @@ private:
     SGPropertyNode_ptr _rtProp;
     SGTimerQueue _rtQueue; 
     SGTimerQueue _simQueue;
-    bool _inited;
+    bool _inited, _shutdown;
 };
 
 #endif // _SG_EVENT_MGR_HXX

simgear/structure/subsystem_mgr.hxx

@@ -28,10 +28,12 @@
 #include <string>
 #include <map>
 #include <vector>
+#include <functional>
 
 #include <simgear/timing/timestamp.hxx>
 #include <simgear/structure/SGSharedPtr.hxx>
 #include <simgear/misc/strutils.hxx>
+#include <simgear/props/propsfwd.hxx>
 
 class TimingInfo
 {
@@ -47,7 +49,10 @@ public:
     const SGTimeStamp& getTime() const { return time; }
 };
 
+// forward decls
 class SampleStatistic;
+class SGSubsystemGroup;
+class SGSubsystemMgr;
 
 typedef std::vector<TimingInfo> eventTimeVec;
 typedef std::vector<TimingInfo>::iterator eventTimeVecIterator;
@@ -125,7 +130,6 @@ typedef void (*SGSubsystemTimingCb)(void* userData, const std::string& name, Sam
 class SGSubsystem : public SGReferenced
 {
 public:
-
   /**
    * Default constructor.
    */
@@ -272,14 +276,71 @@ public:
    */
   void stamp(const std::string& name);
 
-protected:
+    /**
+     * composite name for this subsystem (type name & optional instance name)
+     */
+    std::string name() const
+    { return _name; }
 
-  bool _suspended;
+    /**
+     * @brief the type (class)-specific part of the subsystem name.
+     */
+    std::string typeName() const;
+    
+    /**
+     * @brief the instance part of the subsystem name. Empty if this
+     * subsystem is not instanced
+     */
+    std::string instanceName() const;
+    
+    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 {
+        INVALID = -1,
+        ADD = 0,
+        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);
+    
+    /// composite name for the subsystem (type name and instance name if this
+    /// is an instanced subsystem. (Since this member was originally defined as
+    /// protected, not private, we can't rename it easily)
+    std::string _name;
+    
+  bool _suspended = false;
 
   eventTimeVec timingInfo;
 
   static SGSubsystemTimingCb reportTimingCb;
   static void* reportTimingUserData;
+    
+private:
+    SGSubsystemGroup* _group = nullptr;
 };
 
 typedef SGSharedPtr<SGSubsystem> SGSubsystemRef;
@@ -290,27 +351,29 @@ typedef SGSharedPtr<SGSubsystem> 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 +398,40 @@ public:
     {
         return dynamic_cast<T*>(get_subsystem(T::subsystemName()));
     }
-private:
 
+    bool is_group() const override
+    { return true; }
+    
+    SGSubsystemMgr* get_manager() const override;
+private:
+    void forEach(std::function<void(SGSubsystem*)> f);
+    void reverseForEach(std::function<void(SGSubsystem*)> 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<Member *> MemberVec;
+    
+    using MemberVec = std::vector<Member*>;
     MemberVec _members;
 
+    // track the state of this group, so we can transition added/removed
+    // members correctly
+    SGSubsystem::State _state = SGSubsystem::State::INVALID;
     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<SGSubsystemGroup> SGSubsystemGroupRef;
@@ -372,11 +456,14 @@ typedef SGSharedPtr<SGSubsystemGroup> SGSubsystemGroupRef;
 class SGSubsystemMgr : public SGSubsystem
 {
 public:
+    SGSubsystemMgr(const SGSubsystemMgr&) = delete;
+    SGSubsystemMgr& operator=(const SGSubsystemMgr&) = delete;
 
     /**
      * Types of subsystem groups.
      */
     enum GroupType {
+        INVALID = -1,
         INIT = 0,
         GENERAL,
         FDM,        ///< flight model, autopilot, instruments that run coupled
@@ -389,17 +476,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,31 +494,196 @@ 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;}
 
+    /**
+     * @brief set the root property node for this subsystem manager
+     * subsystems can retrieve this value during init/bind (or at any time)
+     * to base their own properties trees off of.
+     */
+    void set_root_node(SGPropertyNode_ptr node);
+    
+    /**
+     * @brief retrieve the root property node for this subsystem manager
+     */
+    SGPropertyNode_ptr root_node() const;
+    
     template<class T>
     T* get_subsystem() const
     {
         return dynamic_cast<T*>(get_subsystem(T::subsystemName()));
     }
 
-private:
-    std::vector<SGSubsystemGroupRef> _groups;
-    unsigned int _initPosition;
+// instanced overloads, for both raw char* and std::string
+// these concatenate the subsystem type name with the instance name
+    template<class T>
+    T* get_subsystem(const char* instanceName) const
+    {
+        return dynamic_cast<T*>(get_subsystem(T::subsystemName(), instanceName));
+    }
 
-    // non-owning reference
-    typedef std::map<std::string, SGSubsystem*> SubsystemDict;
-    SubsystemDict _subsystem_map;
+    template<class T>
+    T* get_subsystem(const std::string& instanceName) const
+    {
+        return dynamic_cast<T*>(get_subsystem(T::subsystemName(), instanceName));
+    }
+
+    struct Dependency {
+        enum Type {
+            HARD,
+            SOFT,
+            PROPERTY
+        };
+        
+        std::string name;
+        Type type;
+    };
+    
+    using DependencyVec = std::vector<SGSubsystemMgr::Dependency>;
+    using SubsystemFactoryFunctor = std::function<SGSubsystemRef()>;
+
+    /**
+     * @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<Dependency> deps = {});
+
+    template<class T>
+    class Registrant 
+    {
+    public:
+        Registrant(GroupType group = GENERAL, double updateInterval = 0.0,
+                   std::initializer_list<Dependency> 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 T>
+    class InstancedRegistrant
+    {
+    public:
+        InstancedRegistrant(GroupType group = GENERAL,
+                            double updateInterval = 0.0,
+                            std::initializer_list<Dependency> 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 <class T>
+    SGSharedPtr<T> 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<T*>(ref.ptr());
+    }
+    
+    /**
+     * @brief templated creation function, only makes the instance but
+     * doesn't add to the manager or group heirarchy
+     */
+    template <class T>
+    SGSharedPtr<T> create()
+    {
+        auto ref = create(T::subsystemName());
+        return dynamic_cast<T*>(ref.ptr());
+    }
+    
+    SGSubsystemRef create(const std::string& name);
+    
+    template <class T>
+    SGSharedPtr<T> createInstance(const std::string& instanceName)
+    {
+        auto ref = createInstance(T::subsystemName(), instanceName);
+        return dynamic_cast<T*>(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);
+    
+    /**
+     * @brief return a particular subsystem manager by name. Passing an
+     * empty string retrived the default/global subsystem manager, assuming it
+     * has been created.
+     */
+    static SGSubsystemMgr* getManager(const std::string& id);
+private:
+    friend class SGSubsystem;
+    friend class SGSubsystemGroup;
+    
+    void notifyDelegatesWillChange(SGSubsystem* sub, State newState);
+    void notifyDelegatesDidChange(SGSubsystem* sub, State statee);
+    
+    std::vector<SGSubsystemGroupRef> _groups;
+    unsigned int _initPosition = 0;
+    bool _destructorActive = false;
+    SGPropertyNode_ptr _rootNode;
+    
+    // non-owning reference, this is to accelerate lookup
+    // by name which otherwise needs a full walk of the entire tree
+    using SubsystemDict = std::map<std::string, SGSubsystem*>;
+    mutable SubsystemDict _subsystemNameCache;
+
+    using DelegateVec = std::vector<Delegate*>;
+    DelegateVec _delegates;
 };
 
 #endif // __SUBSYSTEM_MGR_HXX

simgear/structure/subsystem_test.cxx

@@ -0,0 +1,456 @@
+#include <simgear_config.h>
+
+#include <cstdio>
+#include <algorithm>
+
+#include <simgear/compiler.h>
+#include <simgear/constants.h>
+#include <simgear/structure/subsystem_mgr.hxx>
+#include <simgear/misc/test_macros.hxx>
+#include <simgear/props/props.hxx>
+
+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 bind() override
+    {
+        auto node = get_manager()->root_node();
+        if (node)
+            node->setIntValue("mysub/foo", 42);
+    }
+    
+    void update(double dt) override
+    {
+        
+    }
+    
+    bool wasInited = false;
+};
+
+class AnotherSub : public SGSubsystem
+{
+public:
+    static const char* subsystemName() { return "anothersub"; }
+
+    void init() override
+    {
+        
+    }
+    
+    void bind() override
+    {
+        auto node = get_manager()->root_node();
+        if (node)
+            node->setIntValue("anothersub/bar", 172);
+    }
+    
+    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<Event>;
+    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<MySub1> registrant(SGSubsystemMgr::GENERAL);
+SGSubsystemMgr::Registrant<AnotherSub> registrant2(SGSubsystemMgr::FDM);
+
+SGSubsystemMgr::Registrant<InstrumentGroup> registrant4(SGSubsystemMgr::FDM);
+
+SGSubsystemMgr::InstancedRegistrant<FakeRadioSub> registrant3(SGSubsystemMgr::POST_FDM);
+
+void testRegistrationAndCreation()
+{
+    SGSharedPtr<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    
+    auto anotherSub = manager->create<AnotherSub>();
+    SG_VERIFY(anotherSub);
+    SG_CHECK_EQUAL(anotherSub->name(), AnotherSub::subsystemName());
+    SG_CHECK_EQUAL(anotherSub->name(), std::string("anothersub"));
+    SG_CHECK_EQUAL(anotherSub->typeName(), std::string("anothersub"));
+    SG_CHECK_EQUAL(anotherSub->instanceName(), std::string());
+
+    auto radio1 = manager->createInstance<FakeRadioSub>("nav1");
+    auto radio2 = manager->createInstance<FakeRadioSub>("nav2");
+
+    
+}
+
+void testAddGetRemove()
+{
+    SGSharedPtr<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    auto d = new RecorderDelegate;
+    manager->addDelegate(d);
+    
+    auto anotherSub = manager->add<AnotherSub>();
+    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<AnotherSub>();
+    SG_CHECK_EQUAL(lookup, anotherSub);
+    
+    SG_CHECK_EQUAL(manager->get_subsystem("anothersub"), anotherSub);
+    
+    // manual create & add
+    auto mySub = manager->create<MySub1>();
+    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<MySub1>(), 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<AnotherSub>() == nullptr);
+    
+    // lookup after remove
+    SG_CHECK_EQUAL(manager->get_subsystem<MySub1>(), mySub);
+    
+    // re-add of removed, and let's test overriding
+    auto another2 = manager->add<AnotherSub>(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<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    auto d = new RecorderDelegate;
+    manager->addDelegate(d);
+    
+    auto anotherSub = manager->add<AnotherSub>();
+    auto instruments = manager->add<InstrumentGroup>();
+    
+    auto radio1 = manager->createInstance<FakeRadioSub>("nav1");
+    auto radio2 = manager->createInstance<FakeRadioSub>("nav2");
+    
+    SG_CHECK_EQUAL(radio1->name(), std::string("fake-radio.nav1"));
+    SG_CHECK_EQUAL(radio2->name(), std::string("fake-radio.nav2"));
+    SG_CHECK_EQUAL(radio1->typeName(), std::string("fake-radio"));
+    SG_CHECK_EQUAL(radio2->instanceName(), std::string("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<InstrumentGroup>(), instruments);
+    
+    manager->bind();
+    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(0, instruments->get_subsystem("fake-radio"));
+
+    
+    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<FakeRadioSub>("nav1"));
+    SG_CHECK_EQUAL(radio2, manager->get_subsystem<FakeRadioSub>("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<SGSubsystemMgr> 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<MySub1>(SGSubsystemMgr::POST_FDM);
+    auto anotherSub = manager->add<AnotherSub>(SGSubsystemMgr::POST_FDM);
+    auto instruments = manager->add<InstrumentGroup>(SGSubsystemMgr::POST_FDM);
+    
+    auto radio1 = manager->createInstance<FakeRadioSub>("nav1");
+    auto radio2 = manager->createInstance<FakeRadioSub>("nav2");
+    instruments->set_subsystem(radio1);
+    instruments->set_subsystem(radio2);
+    
+    manager->bind();
+    for ( ; ; ) {
+        auto status = manager->incrementalInit();
+        if (status == SGSubsystemMgr::INIT_DONE)
+            break;
+    }
+
+    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<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    auto d = new RecorderDelegate;
+    manager->addDelegate(d);
+    
+    auto anotherSub = manager->add<AnotherSub>();
+    auto instruments = manager->add<InstrumentGroup>();
+    
+    auto radio1 = manager->createInstance<FakeRadioSub>("nav1");
+    auto radio2 = manager->createInstance<FakeRadioSub>("nav2");
+    
+    instruments->set_subsystem(radio1);
+    instruments->set_subsystem(radio2);
+    
+    manager->bind();
+    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("fake-radio.nav1-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);
+}
+
+void testPropertyRoot()
+{
+    SGSharedPtr<SGSubsystemMgr> manager = new SGSubsystemMgr;
+    SGPropertyNode_ptr props(new SGPropertyNode);
+    manager->set_root_node(props);
+    
+    auto d = new RecorderDelegate;
+    manager->addDelegate(d);
+
+    manager->add<MySub1>();
+    auto anotherSub = manager->add<AnotherSub>();
+    auto instruments = manager->add<InstrumentGroup>();
+    
+    auto radio1 = manager->createInstance<FakeRadioSub>("nav1");
+    auto radio2 = manager->createInstance<FakeRadioSub>("nav2");
+    
+    instruments->set_subsystem(radio1);
+    instruments->set_subsystem(radio2);
+
+    manager->bind();
+    manager->init();
+
+    SG_CHECK_EQUAL(props->getIntValue("mysub/foo"), 42);
+    SG_CHECK_EQUAL(props->getIntValue("anothersub/bar"), 172);
+}
+
+///////////////////////////////////////////////////////////////////////////////
+
+
+int main(int argc, char* argv[])
+{
+    testRegistrationAndCreation();
+    testAddGetRemove();
+    testSubGrouping();
+    testIncrementalInit();
+    testSuspendResume();
+    testPropertyRoot();
+    
+    cout << __FILE__ << ": All tests passed" << endl;
+    return EXIT_SUCCESS;
+}

simgear/timing/sg_time.cxx

@@ -72,10 +72,14 @@ void SGTime::init( const SGGeod& location, const SGPath& root, time_t init_time
 	cur_time = time(NULL); 
     }
 
+    char* gmt = asctime(gmtime(&cur_time));
+    char* local = asctime(localtime(&cur_time));
+    gmt[strlen(gmt) - 1] = '\0';
+    local[strlen(local) - 1] = '\0';
     SG_LOG( SG_EVENT, SG_DEBUG,
-                "Current greenwich mean time = " << asctime(gmtime(&cur_time)));
+                "Current greenwich mean time = " << gmt);
     SG_LOG( SG_EVENT, SG_DEBUG,
-             "Current local time          = " << asctime(localtime(&cur_time)));
+             "Current local time          = " << local);
 
     if ( !root.isNull()) {
         if (!static_tzContainer.get()) {

version

@@ -1 +1 @@
-2018.2.0
+2018.3.0