Update IsolationNode to fit new Chip Data File design

Change-Id: I3671585c4ab7a9c6c5f4b05e13782c0ea698ad12
Signed-off-by: Zane Shelley <zshelle@us.ibm.com>
diff --git a/src/hei_includes.hpp b/src/hei_includes.hpp
index 71abb4c..e927074 100644
--- a/src/hei_includes.hpp
+++ b/src/hei_includes.hpp
@@ -11,6 +11,7 @@
 #include <stdint.h>
 #include <stdlib.h>
 
+#include <algorithm>
 #include <map>
 #include <memory>
 #include <vector>
@@ -21,7 +22,7 @@
 #include <hei_user_interface.hpp>
 
 // Common macros used throughout this library
-#define HEI_ASSERT(expression) assert(expression);
+#define HEI_ASSERT(expression) assert(expression)
 
 #define HEI_ERR(...)                                                           \
     {                                                                          \
diff --git a/src/hei_signature.hpp b/src/hei_signature.hpp
index d4b40f4..5fd2792 100644
--- a/src/hei_signature.hpp
+++ b/src/hei_signature.hpp
@@ -7,8 +7,8 @@
 {
 
 /**
- * @brief A signature represents an active attention from a single bit on a
- *        register within a chip.
+ * @brief A signature represents an active attention from a single bit position
+ *        from an isolation node within a chip.
  *
  * The isolator will gather a list of all active attentions on a chip and return
  * a list of signatures to the user application. The user application should be
@@ -20,12 +20,12 @@
     /**
      * @brief Constructor from components.
      * @param i_chip     The chip containing this register.
-     * @param i_id       The register ID.
+     * @param i_id       The node ID.
      * @param i_instance The instance of this register.
      * @param i_bit      The target bit within this register.
      * @param i_attnType The attention type reported by this bit.
      */
-    Signature(const Chip& i_chip, RegisterId_t i_id, Instance_t i_instance,
+    Signature(const Chip& i_chip, NodeId_t i_id, Instance_t i_instance,
               BitPosition_t i_bit, AttentionType_t i_attnType) :
         iv_chip(i_chip),
         iv_id(i_id), iv_instance(i_instance), iv_bit(i_bit),
@@ -43,7 +43,7 @@
 
   private:
     Chip iv_chip;                ///< Chip containing this register.
-    RegisterId_t iv_id;          ///< Register ID.
+    NodeId_t iv_id;              ///< Node ID.
     Instance_t iv_instance;      ///< Instance of this register.
     BitPosition_t iv_bit;        ///< Target bit within this register.
     AttentionType_t iv_attnType; ///< Attention type reported by this bit.
@@ -56,7 +56,7 @@
     }
 
     /** @return The register ID. */
-    RegisterId_t getId() const
+    NodeId_t getId() const
     {
         return iv_id;
     }
diff --git a/src/hei_types.hpp b/src/hei_types.hpp
index 7f29f02..3f88a4d 100644
--- a/src/hei_types.hpp
+++ b/src/hei_types.hpp
@@ -30,6 +30,24 @@
 using ChipType_t = uint32_t;
 
 /**
+ * Each isolation node within a chip must have a unique ID. These IDs (combined
+ * with other information) will be passed back to the user application to
+ * identify all of the active errors reported by this chip. Note that some
+ * isolation nodes will have multiple instances within a chip. An ID will be
+ * used for all instances of a node. See enum Instance_t for details on the
+ * instance value.
+ *
+ * Values:
+ *   The isolator does not need to know the possible values because the values
+ *   are passed from the Chip Data Files to the user application.
+ *
+ * Range:
+ *   This is a 2-byte field, which should be sufficient to support all the
+ *   isolation nodes on a typical chip.
+ */
+using NodeId_t = uint16_t;
+
+/**
  * Different chips will contain different types of registers. Also, a single
  * chip may also support multiple types of registers. These enum values are
  * used to communicate to the user application which type of register access is
diff --git a/src/isolator/hei_isolation_chip.hpp b/src/isolator/hei_isolation_chip.hpp
index d4acddf..4fabea1 100644
--- a/src/isolator/hei_isolation_chip.hpp
+++ b/src/isolator/hei_isolation_chip.hpp
@@ -47,7 +47,7 @@
     const ChipType_t iv_chipType;
 
     /** Root nodes for this chip type. */
-    RootNodeMap iv_rootNodes;
+    std::map<AttentionType_t, const IsolationNodePtr> iv_rootNodes;
 
   public: // Member functions
     /**
diff --git a/src/isolator/hei_isolation_node.cpp b/src/isolator/hei_isolation_node.cpp
index f5c58db..d41bdee 100644
--- a/src/isolator/hei_isolation_node.cpp
+++ b/src/isolator/hei_isolation_node.cpp
@@ -1,4 +1,5 @@
 #include <isolator/hei_isolation_node.hpp>
+#include <util/hei_bit_string.hpp>
 
 namespace libhei
 {
@@ -54,18 +55,16 @@
                 // io_isoData. If there are no other active attentions, the user
                 // application could use this signature to help determine, and
                 // circumvent, the isolation problem.
-                io_isoData.addSignature(Signature{i_chip, iv_hwReg.getId(),
-                                                  iv_hwReg.getInstance(), bit,
-                                                  i_attnType});
+                io_isoData.addSignature(
+                    Signature{i_chip, iv_id, iv_instance, bit, i_attnType});
             }
         }
         else
         {
             // We have reached a leaf in the isolation tree. Add this bit's
             // signature to io_isoData.
-            io_isoData.addSignature(Signature{i_chip, iv_hwReg.getId(),
-                                              iv_hwReg.getInstance(), bit,
-                                              i_attnType});
+            io_isoData.addSignature(
+                Signature{i_chip, iv_id, iv_instance, bit, i_attnType});
         }
     }
 
@@ -77,47 +76,43 @@
 
 //------------------------------------------------------------------------------
 
-void IsolationNode::addRule(AttentionType_t i_attnType, const Register* i_rule)
+void IsolationNode::addRule(AttentionType_t i_attnType, RegisterPtr i_rule)
 {
-    // A rule for this attention type should not already exist.
-    HEI_ASSERT(iv_rules.end() == iv_rules.find(i_attnType));
+    HEI_ASSERT(i_rule); // should not be null
 
-    // The rule should not be null.
-    HEI_ASSERT(nullptr != i_rule);
+    auto ret = iv_rules.emplace(i_attnType, i_rule);
 
-    // Add the new rule.
-    iv_rules[i_attnType] = i_rule;
+    // If an entry already existed, it must point to the same object.
+    HEI_ASSERT(ret.second || (ret.first->second == i_rule));
 }
 
 //------------------------------------------------------------------------------
 
-void IsolationNode::addChild(uint8_t i_bit, const IsolationNode* i_child)
+void IsolationNode::addChild(uint8_t i_bit, IsolationNodePtr i_child)
 {
-    // An entry for this bit should not already exist.
-    HEI_ASSERT(iv_children.end() == iv_children.find(i_bit));
+    HEI_ASSERT(i_child); // should not be null
 
-    // The child register should not be null.
-    HEI_ASSERT(nullptr != i_child);
+    auto ret = iv_children.emplace(i_bit, i_child);
 
-    // Add the new rule.
-    iv_children[i_bit] = i_child;
+    // If an entry already existed, it must point to the same object.
+    HEI_ASSERT(ret.second || (ret.first->second == i_child));
 }
 
 //------------------------------------------------------------------------------
 
-std::vector<const IsolationNode*> IsolationNode::cv_isolationStack{};
+std::vector<IsolationNodePtr> IsolationNode::cv_isolationStack{};
 
 //------------------------------------------------------------------------------
 
 void IsolationNode::pushIsolationStack() const
 {
     // Ensure this node does not already exist in cv_isolationStack.
-    auto itr =
-        std::find(cv_isolationStack.begin(), cv_isolationStack.end(), this);
+    auto itr = std::find(cv_isolationStack.begin(), cv_isolationStack.end(),
+                         IsolationNodePtr(this));
     HEI_ASSERT(cv_isolationStack.end() == itr);
 
     // Push to node to the stack.
-    cv_isolationStack.push_back(this);
+    cv_isolationStack.push_back(IsolationNodePtr(this));
 }
 
 //------------------------------------------------------------------------------
diff --git a/src/isolator/hei_isolation_node.hpp b/src/isolator/hei_isolation_node.hpp
index 4cf44dd..8d7cfac 100644
--- a/src/isolator/hei_isolation_node.hpp
+++ b/src/isolator/hei_isolation_node.hpp
@@ -2,46 +2,45 @@
 
 #include <hei_includes.hpp>
 #include <hei_isolation_data.hpp>
-#include <register/hei_hardware_register.hpp>
 #include <register/hei_register.hpp>
-#include <util/hei_bit_string.hpp>
-#include <util/hei_flyweight.hpp>
 
 namespace libhei
 {
 
 /**
- * @brief This class contains the isolation rules and bit definition of a
- *        HardwareRegister used for error isolation.
+ * @brief This class contains the isolation rules and bit definition for a node
+ *        in a chip's error reporting structure.
  *
- * These objects are linked together as a tree. Any active bits in the
- * associated register will either be a true active attention (leaf node) or
- * indicate one or more active attentions occurred in a child node.
+ * These objects are linked together to form a tree with a single root node. Any
+ * active bits found in a node will either indicate an active attention or that
+ * the attention originated in a child node.
  *
  * The primary function of this class is analyze(), which will do a depth-first
- * search of the tree to find all leaves and add their signatures to the
- * returned isolation data.
+ * search of the tree to find all active attentions and add their signatures to
+ * the returned isolation data.
  *
  * The tree structure is built from information in the Chip Data Files. It is
  * possible that the tree could be built with loop in the isolation. This would
  * be bug in the Chip Data Files. This class will keep track of all nodes that
  * have been analyzed to prevent cyclic isolation (an infinite loop).
  *
- * Each isolation register will have a rule for each supported attention type.
- * These rules are a combination of HardwareRegisters and operator registers to
- * define rules like "REG & ~MASK & CNFG", which reads "return all bits in REG
- * that are not in MASK and set in CNFG". See the definition of the Register
- * class for details on how this works.
+ * Each node instance will represent a register, or set of registers, that can
+ * be configured to represent one or more attention types. These configuration
+ * rules are a combination of hardware register objects and operator registers
+ * objects to define rules like "REG & ~MASK & CNFG", which reads "return all
+ * bits in REG that are not in MASK and set in CNFG". See the definition of the
+ * Register class for details on how this works.
  */
 class IsolationNode
 {
   public: // Constructors, destructor, assignment
     /**
      * @brief Constructor from components.
-     * @param i_hwReg A reference to the HardwareRegister targeted for
-     *                isolation.
+     * @param i_id       Unique ID for all instances of this node.
+     * @param i_instance Instance of this node.
      */
-    explicit IsolationNode(const HardwareRegister& i_hwReg) : iv_hwReg(i_hwReg)
+    IsolationNode(NodeId_t i_id, Instance_t i_instance) :
+        iv_id(i_id), iv_instance(i_instance)
     {}
 
     /** @brief Destructor. */
@@ -55,12 +54,14 @@
     IsolationNode& operator=(const IsolationNode&) = delete;
 
   private: // Instance variables
+    /** The unique ID for all instances of this node. */
+    const NodeId_t iv_id;
+
     /**
-     * This is a reference to the HardwareRegister targeted for isolation by
-     * this instance of the class. The reference is required to maintain
-     * polymorphism.
+     * A node may have multiple instances. All of which will have the same ID.
+     * This variable is used to distinguish between each instance of the node.
      */
-    const HardwareRegister& iv_hwReg;
+    const Instance_t iv_instance;
 
     /**
      * This register could report multiple types of attentions. We can use a
@@ -69,21 +70,22 @@
      * HardwareRegister objects and virtual operator registers (all children
      * of the Register class).
      */
-    std::map<AttentionType_t, const Register*> iv_rules;
+    std::map<AttentionType_t, const RegisterPtr> iv_rules;
 
     /**
      * Each bit (key) in this map indicates that an attention was driven from
      * another register (value).
      */
-    std::map<BitPosition_t, const IsolationNode*> iv_children;
+    std::map<BitPosition_t, const std::shared_ptr<const IsolationNode>>
+        iv_children;
 
   public: // Member functions
     /**
-     * @brief  Finds all active attentions on this register. If an active bit is
-     *         a leaf in the isolation tree, the bit's signature is added to the
+     * @brief  Finds all active attentions on this node. If an active bit is a
+     *         leaf in the isolation tree, the bit's signature is added to the
      *         isolation data. Otherwise, this function is recursively called
-     *         to analyze the child register that is driving the attention in
-     *         this register.
+     *         to analyze the child node that is driving the attention in this
+     *         node.
      * @param  i_chip     The target chip for isolation.
      * @param  i_attnType The target attention type to analyze on this register.
      *                    Will assert a rule must exist for this attention type.
@@ -95,48 +97,63 @@
     bool analyze(const Chip& i_chip, AttentionType_t i_attnType,
                  IsolationData& io_isoData) const;
 
-    // TODO: The next two functions are only intended to be used during
-    //       initialization of the isolator. Consider, making them private and
-    //       make the Chip Data File code friends of this class. So that it has
-    //       access to these init functions.
-
     /**
      * @brief Adds a register rule for the given attention type. See iv_rules
      *        for details.
      *
      * This is only intended to be used during initialization of the isolator.
-     * Will assert that nothing has already been defined for this rule.
+     * Will assert that a rule has not already been defined for this type.
      *
      * @param The target attention type.
      * @param The rule for this attention type.
      */
-    void addRule(AttentionType_t i_attnType, const Register* i_rule);
+    void addRule(AttentionType_t i_attnType, RegisterPtr i_rule);
 
     /**
-     * @brief Adds a child register to analyze for the given bit in this
-     *        register. See iv_children for details.
+     * @brief Adds a child node to analyze for the given bit position in this
+     *        node. See iv_children for details.
      *
      * This is only intended to be used during initialization of the isolator.
      * Will assert that nothing has already been defined for this bit.
      *
-     * @param The target bit on this register.
-     * @param The child register to analyze for the given bit.
+     * @param The target bit on this node.
+     * @param The child node to analyze for the given bit.
      */
-    void addChild(BitPosition_t i_bit, const IsolationNode* i_child);
+    void addChild(BitPosition_t i_bit,
+                  std::shared_ptr<const IsolationNode> i_child);
+
+    /** @return The node ID. */
+    NodeId_t getId() const
+    {
+        return iv_id;
+    }
+
+    /** @return The node instance. */
+    Instance_t getInstance() const
+    {
+        return iv_instance;
+    }
 
   public: // Operators
     /** @brief Equals operator. */
     bool operator==(const IsolationNode& i_r) const
     {
-        // iv_hwReg should be unique per IsolationNode.
-        return (iv_hwReg == i_r.iv_hwReg);
+        return (iv_id == i_r.iv_id) && (iv_instance == i_r.iv_instance);
     }
 
     /** @brief Less than operator. */
     bool operator<(const IsolationNode& i_r) const
     {
-        // iv_hwReg should be unique per IsolationNode.
-        return (iv_hwReg < i_r.iv_hwReg);
+        if (iv_id < i_r.iv_id)
+        {
+            return true;
+        }
+        else if (iv_id == i_r.iv_id)
+        {
+            return (iv_instance < i_r.iv_instance);
+        }
+
+        return false;
     }
 
   private: // Isolation stack and supporting functions.
@@ -152,7 +169,7 @@
      *  this node can be popped off the top of the stack. Once all the recursive
      *  calls have returned back to the root node the stack should be empty.
      */
-    static std::vector<const IsolationNode*> cv_isolationStack;
+    static std::vector<std::shared_ptr<const IsolationNode>> cv_isolationStack;
 
     /**
      * @brief Pushes this node to the top of the stack. Will assert that this
@@ -167,10 +184,7 @@
     }
 };
 
-/** Pointer management for isolation nodes. */
-using IsolationNodePtr = std::shared_ptr<IsolationNode>;
-
-/** Simple map to ensure only one root IsolationNode per attention type. */
-using RootNodeMap = std::map<AttentionType_t, const IsolationNodePtr>;
+/** Pointer management for IsolationNode objects. */
+using IsolationNodePtr = std::shared_ptr<const IsolationNode>;
 
 } // end namespace libhei
diff --git a/src/register/hei_register.hpp b/src/register/hei_register.hpp
index ff8999c..7cc28eb 100644
--- a/src/register/hei_register.hpp
+++ b/src/register/hei_register.hpp
@@ -44,4 +44,7 @@
 // Pure virtual destructor must be defined.
 inline Register::~Register() {}
 
+/** Pointer management for Register objects. */
+using RegisterPtr = std::shared_ptr<const Register>;
+
 } // end namespace libhei