Fix #5121

NREL · Mar 27, 2024 · 8f1a3bf · 8f1a3bf
1 parent 309a5ea
commit 8f1a3bf
Show file tree

Hide file tree

Showing 12 changed files with 185 additions and 57 deletions.
diff --git a/src/model/test/ZoneHVACEquipmentList_GTest.cpp b/src/model/test/ZoneHVACEquipmentList_GTest.cpp
@@ -160,6 +160,10 @@ TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_ModelObject_Is_First)
   Model m;
   ThermalZone z(m);
 
+  auto eqlists = m.getConcreteModelObjects<ZoneHVACEquipmentList>();
+  EXPECT_EQ(1, eqlists.size());
+  auto& eqlist = eqlists.front();
+
   ZoneHVACBaseboardConvectiveElectric bb_delete(m);
 
   ScheduleConstant bb_sch(m);
@@ -174,11 +178,39 @@ TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_ModelObject_Is_First)
   EXPECT_TRUE(bb_delete.addToThermalZone(z));
   EXPECT_TRUE(bb.addToThermalZone(z));
 
-  auto objects = m.getObjectsByName(bb.nameString());
-  EXPECT_EQ(2, objects.size());
-  EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), objects.front().iddObject().type());
+  {
+    EXPECT_EQ(2, eqlist.numExtensibleGroups());
+    auto idf_egs = eqlist.extensibleGroups();
+    for (const auto& idf_eg : idf_egs) {
+      // Using getField so we don't resolve object name, but get the handle string
+      const auto val_ = idf_eg.getField(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+      const auto uid = openstudio::toUUID(*val_);
+      auto obj_ = m.getObject(uid);
+      ASSERT_TRUE(obj_);
+      EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
+    }
+  }
+
+  {
+    auto objects = m.getObjectsByName(bb.nameString());
+    EXPECT_EQ(2, objects.size());
+    EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), objects.front().iddObject().type());
+  }
 
   EXPECT_NO_THROW(bb_delete.remove());
+
+  {
+    EXPECT_EQ(1, eqlist.numExtensibleGroups());
+    auto idf_egs = eqlist.extensibleGroups();
+    for (const auto& idf_eg : idf_egs) {
+      // Using getField so we don't resolve object name, but get the handle string
+      const auto val_ = idf_eg.getField(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+      const auto uid = openstudio::toUUID(*val_);
+      auto obj_ = m.getObject(uid);
+      ASSERT_TRUE(obj_);
+      EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
+    }
+  }
 }
 
 TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_Schedule_Is_First) {
@@ -204,23 +236,39 @@ TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_Schedule_Is_First) {
   EXPECT_TRUE(bb_delete.addToThermalZone(z));
   EXPECT_TRUE(bb.addToThermalZone(z));
 
-  EXPECT_EQ(2, eqlist.numExtensibleGroups());
-  // I deliberately use idfObject.extensibleGroups so the handles aren't resolved to object name
-  auto idf_egs = eqlist.idfObject().extensibleGroups();
-
-  for (const auto& idf_eg : idf_egs) {
-    const std::string val = idf_eg.getString(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment).get();
-    const auto uid = openstudio::toUUID(val);
-    auto obj_ = m.getObject(uid);
-    ASSERT_TRUE(obj_);
-    EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
+  {
+    EXPECT_EQ(2, eqlist.numExtensibleGroups());
+    auto idf_egs = eqlist.extensibleGroups();
+    for (const auto& idf_eg : idf_egs) {
+      // Using getField so we don't resolve object name, but get the handle string
+      const auto val_ = idf_eg.getField(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+      const auto uid = openstudio::toUUID(*val_);
+      auto obj_ = m.getObject(uid);
+      ASSERT_TRUE(obj_);
+      EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
+    }
   }
 
-  auto objects = m.getObjectsByName(bb.nameString());
-  EXPECT_EQ(2, objects.size());
-  EXPECT_EQ(IddObjectType(IddObjectType::OS_Schedule_Constant), objects.front().iddObject().type());
+  {
+    auto objects = m.getObjectsByName(bb.nameString());
+    EXPECT_EQ(2, objects.size());
+    EXPECT_EQ(IddObjectType(IddObjectType::OS_Schedule_Constant), objects.front().iddObject().type());
+  }
 
   EXPECT_NO_THROW(bb_delete.remove());
+
+  {
+    EXPECT_EQ(1, eqlist.numExtensibleGroups());
+    auto idf_egs = eqlist.extensibleGroups();
+    for (const auto& idf_eg : idf_egs) {
+      // Using getField so we don't resolve object name, but get the handle string
+      const auto val_ = idf_eg.getField(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+      const auto uid = openstudio::toUUID(*val_);
+      auto obj_ = m.getObject(uid);
+      ASSERT_TRUE(obj_);
+      EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
+    }
+  }
 }
 
 TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_Schedule_Is_First_OnlyPop) {
@@ -306,10 +354,11 @@ TEST_F(ModelFixture, ZoneHVACEquipmentList_RemoveEquipment_Schedule_OnlyPop_Work
     EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), obj_->iddObject().type());
   }
 
-  eqlist.idfObject().eraseExtensibleGroup(0);
+  auto i = eqlist.idfObject();
+  i.eraseExtensibleGroup(0);
 
-  EXPECT_EQ(1, eqlist.numExtensibleGroups());
-  idf_egs = eqlist.idfObject().extensibleGroups();
+  EXPECT_EQ(1, i.numExtensibleGroups());
+  idf_egs = i.extensibleGroups();
   for (const auto& idf_eg : idf_egs) {
     const std::string val = idf_eg.getString(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment).get();
     const auto uid = openstudio::toUUID(val);

diff --git a/src/utilities/idf/IdfExtensibleGroup.cpp b/src/utilities/idf/IdfExtensibleGroup.cpp
@@ -22,6 +22,17 @@ StringVector IdfExtensibleGroup::fields(bool returnDefault) const {
   return result;
 }
 
+StringVector IdfExtensibleGroup::fieldsWithHandles(bool returnDefault) const {
+  StringVector result;
+  const unsigned n = numFields();
+  result.reserve(n);
+  for (unsigned i = 0; i < n; ++i) {
+    OptionalString str = getField(i, returnDefault);
+    result.push_back(str.get_value_or(""));
+  }
+  return result;
+}
+
 StringVector IdfExtensibleGroup::fieldComments(bool returnDefault) const {
   StringVector result;
   for (unsigned i = 0, n = numFields(); i < n; ++i) {
@@ -57,6 +68,13 @@ OptionalString IdfExtensibleGroup::getString(unsigned fieldIndex, bool returnDef
   return m_impl->getString(mf_toIndex(fieldIndex), returnDefault);
 }
 
+boost::optional<std::string> IdfExtensibleGroup::getField(unsigned index, bool returnDefault) const {
+  if (!isValid(index)) {
+    return boost::none;
+  }
+  return m_impl->getField(mf_toIndex(index), returnDefault);
+}
+
 OptionalDouble IdfExtensibleGroup::getDouble(unsigned fieldIndex, bool returnDefault) const {
   if (!isValid(fieldIndex)) {
     return boost::none;
@@ -169,7 +187,7 @@ IdfExtensibleGroup IdfExtensibleGroup::pushClone() const {
     return {p, 0};
   }
 
-  StringVector values = fields();
+  StringVector values = fieldsWithHandles();
   OS_ASSERT(values.size() == numFields());
   return m_impl->pushExtensibleGroup(values);
 }
@@ -180,7 +198,7 @@ IdfExtensibleGroup IdfExtensibleGroup::insertClone(unsigned groupIndex) const {
     return {p, 0};
   }
 
-  StringVector values = fields();
+  StringVector values = fieldsWithHandles();
   OS_ASSERT(values.size() == numFields());
   return m_impl->insertExtensibleGroup(groupIndex, values);
 }

diff --git a/src/utilities/idf/IdfExtensibleGroup.hpp b/src/utilities/idf/IdfExtensibleGroup.hpp
@@ -40,10 +40,13 @@ class UTILITIES_API IdfExtensibleGroup
   /** @name Getters */
   //@{
 
-  /** Returns this extensible group's fields. Return value will be empty() if this group is
-   *  empty(). */
+  /** Returns this extensible group's fields. Return value will be empty() if this group is empty(). */
   std::vector<std::string> fields(bool returnDefault = false) const;
 
+  /** Returns this extensible group's fields. Return value will be empty() if this group is empty().
+    * Unlike fields(), in the case it's a WorkspaceObject_Impl, it uses handles as string and not the name of the target object */
+  std::vector<std::string> fieldsWithHandles(bool returnDefault = false) const;
+
   /** Returns the comments associated with this extensible group's fields. */
   std::vector<std::string> fieldComments(bool returnDefault = false) const;
 
@@ -54,6 +57,10 @@ class UTILITIES_API IdfExtensibleGroup
    *  exists (isValid(fieldIndex)). */
   boost::optional<std::string> getString(unsigned fieldIndex, bool returnDefault = false) const;
 
+  /** Like getString except for reference fields getString will return the name of the referenced object.
+   * This method, getField, will always return the string value  of the field.   */
+  boost::optional<std::string> getField(unsigned index, bool returnDefault = false) const;
+
   /** Returns true if the field is empty. */
   bool isEmpty(unsigned fieldIndex) const;
 

diff --git a/src/utilities/idf/IdfObject.cpp b/src/utilities/idf/IdfObject.cpp
@@ -209,6 +209,10 @@ namespace detail {
     return boost::none;
   }
 
+  boost::optional<std::string> IdfObject_Impl::getField(unsigned index, bool returnDefault) const {
+    return IdfObject_Impl::getString(index, returnDefault, false);
+  }
+
   boost::optional<double> IdfObject_Impl::getDouble(unsigned index, bool returnDefault) const {
     OptionalDouble result;
     OptionalString value = getString(index, returnDefault, false);
@@ -687,7 +691,7 @@ namespace detail {
       unsigned i = numExtensibleGroups() - 1;
       IdfExtensibleGroup eg = getExtensibleGroup(i);
       OS_ASSERT(!eg.empty());
-      IdfExtensibleGroup temp = pushExtensibleGroup(eg.fields(), checkValidity);
+      IdfExtensibleGroup temp = pushExtensibleGroup(eg.fieldsWithHandles(), checkValidity);
       if (temp.empty()) {
         OS_ASSERT(numFields() == n);
         OS_ASSERT(m_diffs.size() == diffSize);
@@ -701,7 +705,7 @@ namespace detail {
         --i;
         IdfExtensibleGroup peg = getExtensibleGroup(i);
         OS_ASSERT(!peg.empty());
-        ok = eg.setFields(peg.fields(), false);
+        ok = eg.setFields(peg.fieldsWithHandles(), false);
         if (!ok) {
           // roll back
           i += 2;
@@ -710,7 +714,7 @@ namespace detail {
             OS_ASSERT(!peg.empty());
             eg = getExtensibleGroup(i + 1);
             OS_ASSERT(!eg.empty());
-            peg.setFields(eg.fields(), false);
+            peg.setFields(eg.fieldsWithHandles(), false);
             ++i;
           }
           popExtensibleGroup(false);
@@ -734,7 +738,7 @@ namespace detail {
           ++i;
           eg = getExtensibleGroup(i);
           OS_ASSERT(!eg.empty());
-          peg.setFields(eg.fields(), false);
+          peg.setFields(eg.fieldsWithHandles(), false);
           peg = eg;
         }
         popExtensibleGroup(false);
@@ -774,7 +778,7 @@ namespace detail {
       IdfExtensibleGroup egToPop = getExtensibleGroup(numExtensibleGroups() - 1);
       OS_ASSERT(!egToPop.empty());
       UnsignedVector indices = egToPop.mf_indices();
-      result = egToPop.fields();
+      result = egToPop.fieldsWithHandles();
       OS_ASSERT(result.size() == groupSize);
 
       // record diffs for each field going backwards
@@ -825,14 +829,14 @@ namespace detail {
       StringVector temp = result;
       IdfExtensibleGroup eg = getExtensibleGroup(i);
       OS_ASSERT(!eg.empty());
-      result = eg.fields();
+      result = eg.fieldsWithHandles();
       ok = eg.setFields(temp, checkValidity);
       if (!ok) {
         // roll back changes and return
         for (unsigned j = i + 1, n = numExtensibleGroups(); j < n; ++j) {
           eg = getExtensibleGroup(j);
           OS_ASSERT(!eg.empty());
-          result = eg.fields();
+          result = eg.fieldsWithHandles();
           OS_ASSERT(result.size() == temp.size());
           eg.setFields(temp, false);
           temp = result;
@@ -884,7 +888,7 @@ namespace detail {
       if (indices.empty()) {
         indices = eg.mf_indices();
       }
-      rollbackValues.push_back(eg.fields());
+      rollbackValues.push_back(eg.fieldsWithHandles());
       rollbackComments.push_back(eg.fieldComments());
       // try to pop
       StringVector result = popExtensibleGroup(checkValidity);
@@ -2063,6 +2067,10 @@ boost::optional<std::string> IdfObject::getString(unsigned index, bool returnDef
   return m_impl->getString(index, returnDefault, returnUninitializedEmpty);
 }
 
+boost::optional<std::string> IdfObject::getField(unsigned index, bool returnDefault) const {
+  return m_impl->getField(index, returnDefault);
+}
+
 boost::optional<double> IdfObject::getDouble(unsigned index, bool returnDefault) const {
   return m_impl->getDouble(index, returnDefault);
 }

diff --git a/src/utilities/idf/IdfObject.hpp b/src/utilities/idf/IdfObject.hpp
@@ -144,6 +144,11 @@ class UTILITIES_API IdfObject
    */
   boost::optional<std::string> getString(unsigned index, bool returnDefault = false, bool returnUninitializedEmpty = false) const;
 
+  /** Like getString except for reference fields getString will return the name of the referenced object.
+    * This method, getField, will always return the string value of the field.
+    * For IdfObject, this is the same as getString */
+  boost::optional<std::string> getField(unsigned index, bool returnDefault = false) const;
+
   /** Get the value of the field at index, converted to double, if possible. Returns an
    *  uninitialized object if the conversion is unsuccessful for any reason. Logs a warning
    *  if the conversion fails, the field is RealType, and the field is not equal to

diff --git a/src/utilities/idf/IdfObject_Impl.hpp b/src/utilities/idf/IdfObject_Impl.hpp
@@ -101,6 +101,11 @@ namespace detail {
      */
     virtual boost::optional<std::string> getString(unsigned index, bool returnDefault = false, bool returnUninitializedEmpty = false) const;
 
+    /** Like getString except for reference fields getString will return the name of the referenced object.
+     * This method, getField, will always return the string value of the field.
+     * For IdfObject, this is the same as getString */
+    virtual boost::optional<std::string> getField(unsigned index, bool returnDefault = false) const;
+
     /** Get the value of the field at index, converted to double, if possible. Returns an
      *  uninitialized object if the conversion is unsuccessful for any reason. Logs a warning
      *  if the conversion fails, the field is RealType, and the field is not equal to

diff --git a/src/utilities/idf/Test/WorkspaceObject_GTest.cpp b/src/utilities/idf/Test/WorkspaceObject_GTest.cpp
@@ -10,15 +10,21 @@
 #include <utilities/idd/Zone_FieldEnums.hxx>
 #include <utilities/idd/Lights_FieldEnums.hxx>
 #include <utilities/idd/Schedule_Compact_FieldEnums.hxx>
+
 #include <utilities/idd/OS_DaylightingDevice_Shelf_FieldEnums.hxx>
 #include <utilities/idd/OS_SetpointManager_MixedAir_FieldEnums.hxx>
+#include <utilities/idd/OS_ZoneHVAC_EquipmentList_FieldEnums.hxx>
+#include <utilities/idd/OS_ZoneHVAC_Baseboard_Convective_Electric_FieldEnums.hxx>
+#include <utilities/idd/OS_Schedule_Constant_FieldEnums.hxx>
+
 #include <utilities/idd/IddFactory.hxx>
 
 #include "../WorkspaceExtensibleGroup.hpp"
 #include "../IdfFile.hpp"
 #include "../IdfObject.hpp"
 #include "../Workspace.hpp"
 #include "../WorkspaceObject.hpp"
+#include "../WorkspaceObject_Impl.hpp"
 
 #include "../../core/Optional.hpp"
 
@@ -585,6 +591,59 @@ TEST_F(IdfFixture, WorkspaceObject_setName_allObjects) {
   }
 }
 
+TEST_F(IdfFixture, WorkspaceObject_eraseExtensibleGroups) {
+
+  Workspace w(StrictnessLevel::Draft, IddFileType::OpenStudio);
+  WorkspaceObject eqlist = w.addObject(IdfObject(IddObjectType::OS_ZoneHVAC_EquipmentList)).get();
+
+  WorkspaceObject bb_delete = w.addObject(IdfObject(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric)).get();
+  {
+    auto weg = eqlist.pushExtensibleGroup().cast<WorkspaceExtensibleGroup>();
+    EXPECT_TRUE(weg.setPointer(0, bb_delete.handle()));
+    EXPECT_TRUE(weg.setUnsigned(1, 1));
+    EXPECT_TRUE(weg.setUnsigned(2, 1));
+  }
+
+  WorkspaceObject bb = w.addObject(IdfObject(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric)).get();
+
+  WorkspaceObject bb_sch = w.addObject(IdfObject(IddObjectType::OS_Schedule_Constant)).get();
+  EXPECT_TRUE(bb.setPointer(2, bb_sch.handle()));
+
+  {
+    auto weg = eqlist.pushExtensibleGroup().cast<WorkspaceExtensibleGroup>();
+    EXPECT_TRUE(weg.setPointer(0, bb.handle()));
+    EXPECT_TRUE(weg.setUnsigned(1, 1));
+    EXPECT_TRUE(weg.setUnsigned(2, 1));
+  }
+
+  EXPECT_TRUE(bb.setName("Baseboard"));
+  EXPECT_TRUE(bb_sch.setName(bb.nameString()));
+
+  EXPECT_EQ(bb.nameString(), bb_sch.nameString());
+
+  auto objects = w.getObjectsByName(bb.nameString());
+  EXPECT_EQ(2, objects.size());
+  EXPECT_EQ(IddObjectType(IddObjectType::OS_Schedule_Constant), objects.front().iddObject().type());
+
+  EXPECT_EQ(2, eqlist.numExtensibleGroups());
+  for (const auto& eg : eqlist.extensibleGroups()) {
+    const auto weg = eg.cast<WorkspaceExtensibleGroup>();
+    auto target_ = weg.getTarget(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+    ASSERT_TRUE(target_);
+    EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), target_->iddObject().type());
+  }
+
+  eqlist.eraseExtensibleGroup(0);
+
+  EXPECT_EQ(1, eqlist.numExtensibleGroups());
+  for (const auto& eg : eqlist.extensibleGroups()) {
+    const auto weg = eg.cast<WorkspaceExtensibleGroup>();
+    auto target_ = weg.getTarget(OS_ZoneHVAC_EquipmentListExtensibleFields::ZoneEquipment);
+    ASSERT_TRUE(target_);
+    EXPECT_EQ(IddObjectType(IddObjectType::OS_ZoneHVAC_Baseboard_Convective_Electric), target_->iddObject().type());
+  }
+}
+
 TEST_F(IdfFixture, WorkspaceObject_SpecialMembers) {
 
   static_assert(!std::is_trivial<WorkspaceObject>{});