Skip to content

Commit

Permalink
#4074 Followup: add test for logic, and avoid clearing Setpoint2/Sche…
Browse files Browse the repository at this point in the history 
…dule all the time. Left A TODO
  • Loading branch information
@jmarrec
jmarrec committed Sep 15, 2020
1 parent 9d5b2ac commit 361e509
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 23 deletions.
52 changes: 29 additions & 23 deletions src/model/ShadingControl.cpp
View file
Expand Up @@ -311,31 +311,37 @@ namespace detail {
std::string oldControlType = this->shadingControlType();
bool result = setString(OS_ShadingControlFields::ShadingControlType, shadingControlType);
if (result){
if (!ShadingControl_Impl::isControlTypeValueNeedingSetpoint1(shadingControlType)) {
// Not calling reset to avoid double check on whether it's required
// resetSetpoint();
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not require a Setpoint, reseting");
bool test = setString(OS_ShadingControlFields::Setpoint, "");
OS_ASSERT(test);
}
if (!ShadingControl_Impl::isControlTypeValueNeedingSetpoint2(shadingControlType)) {
// Not calling reset to avoid double check on whether it's required
// resetSetpoint2();
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not require a Setpoint2, reseting");
bool test = setString(OS_ShadingControlFields::Setpoint2, "");
OS_ASSERT(test);
}
if (!ShadingControl_Impl::isControlTypeValueAllowingSchedule(shadingControlType)) {
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not allow a Schedule, reseting");
bool test = setString(OS_ShadingControlFields::ScheduleName, "");
OS_ASSERT(test);
test = setString(OS_ShadingControlFields::ShadingControlIsScheduled, "No");
OS_ASSERT(test);
}

// TODO: do we want to force remove this stuff like it did resetSetpoint before? Units/quantity might be oranges and apples when switching
// types, but they could be compatible, and it may be very confusing at least for interface users that are playing with a dropdown to see
// schedulesand setpoints disapear
if (!openstudio::istringEqual(oldControlType, shadingControlType)) {
resetSetpoint();
} else {
if (!ShadingControl_Impl::isControlTypeValueNeedingSetpoint1(shadingControlType)) {
// Not calling reset to avoid double check on whether it's required
// resetSetpoint();
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not require a Setpoint, reseting");
bool test = setString(OS_ShadingControlFields::Setpoint, "");
OS_ASSERT(test);
}
if (!ShadingControl_Impl::isControlTypeValueNeedingSetpoint2(shadingControlType)) {
// Not calling reset to avoid double check on whether it's required
// resetSetpoint2();
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not require a Setpoint2, reseting");
bool test = setString(OS_ShadingControlFields::Setpoint2, "");
OS_ASSERT(test);
}
if (!ShadingControl_Impl::isControlTypeValueAllowingSchedule(shadingControlType)) {
LOG(Info, briefDescription() << " Shading Control Type was changed to '" << shadingControlType << " which does not allow a Schedule, reseting");
bool test = setString(OS_ShadingControlFields::ScheduleName, "");
OS_ASSERT(test);
test = setString(OS_ShadingControlFields::ShadingControlIsScheduled, "No");
OS_ASSERT(test);
}
resetSetpoint(); // For backward compatibility
// resetSetpoint2();
// resetSchedule();
}

}
return result;
}
Expand Down
68 changes: 68 additions & 0 deletions src/model/test/ShadingControl_GTest.cpp
View file
Expand Up @@ -35,6 +35,7 @@
#include "../ShadingControl_Impl.hpp"
#include "../Construction.hpp"
#include "../Schedule.hpp"
#include "../ScheduleConstant.hpp"
#include "../Blind.hpp"
#include "../Blind_Impl.hpp"
#include "../SimpleGlazing.hpp"
Expand Down Expand Up @@ -222,3 +223,70 @@ TEST_F(ModelFixture, ShadingControl_Clone) {
EXPECT_NE(shadingControl.shadingMaterial().get(), shadingControlClone.shadingMaterial().get());
}
}

TEST_F(ModelFixture, ShadingControl_ShadingControlTypeLogic) {

Model m;

Blind b(m);
ShadingControl sc(b);

EXPECT_TRUE(sc.setSetpoint(32.0));
ASSERT_TRUE(sc.setpoint());
EXPECT_EQ(32.0, sc.setpoint().get());

ScheduleConstant sch(m);

// Allows nothing
EXPECT_TRUE(sc.setShadingControlType("AlwaysOn"));
EXPECT_FALSE(sc.isControlTypeValueNeedingSetpoint1());
EXPECT_FALSE(sc.isControlTypeValueNeedingSetpoint2());
EXPECT_FALSE(sc.isControlTypeValueAllowingSchedule());
EXPECT_FALSE(sc.setSetpoint(30.0));
EXPECT_FALSE(sc.setSetpoint2(500.0));
EXPECT_FALSE(sc.setSchedule(sch));
EXPECT_FALSE(sc.setpoint());
EXPECT_FALSE(sc.setpoint2());
EXPECT_FALSE(sc.schedule());

// Allows schedules, required two setpoints
EXPECT_TRUE(sc.setShadingControlType("OnIfHighZoneAirTempAndHighSolarOnWindow"));
EXPECT_TRUE(sc.isControlTypeValueNeedingSetpoint1());
EXPECT_TRUE(sc.isControlTypeValueNeedingSetpoint2());
EXPECT_TRUE(sc.isControlTypeValueAllowingSchedule());
EXPECT_TRUE(sc.setSetpoint(30.0));
EXPECT_TRUE(sc.setSetpoint2(500.0));
EXPECT_TRUE(sc.setSchedule(sch));
ASSERT_TRUE(sc.setpoint());
EXPECT_EQ(30.0, sc.setpoint().get());
ASSERT_TRUE(sc.setpoint2());
EXPECT_EQ(500.0, sc.setpoint2().get());
ASSERT_TRUE(sc.schedule());
EXPECT_EQ(sch, sc.schedule().get());

// Shouldn't allow reseting setpoints since required
sc.resetSetpoint();
ASSERT_TRUE(sc.setpoint());
EXPECT_EQ(30.0, sc.setpoint().get());
ASSERT_TRUE(sc.setpoint2());
EXPECT_EQ(500.0, sc.setpoint2().get());
// Note: I'm not providing resetSetpoint2 on purpose

sc.resetSchedule();
EXPECT_FALSE(sc.schedule());
EXPECT_TRUE(sc.setSchedule(sch));

// OnIfHighGlare: needs setpoint1 only and not Setpoint2, allows schedule
EXPECT_TRUE(sc.setShadingControlType("OnIfHighZoneCooling"));
EXPECT_TRUE(sc.isControlTypeValueNeedingSetpoint1());
EXPECT_FALSE(sc.isControlTypeValueNeedingSetpoint2());
EXPECT_TRUE(sc.isControlTypeValueAllowingSchedule());

// Setpoint 1 has been cleared because we're switching Shading Control Type and that was the historical behavior
EXPECT_FALSE(sc.setpoint2());
// Setpoint2 is cleared because the new Shading Control Type does not support Setpoint 2
EXPECT_FALSE(sc.setpoint2());
// Schedule is kept, user should be responsible to ensure the Schedule still makes sense, instead of always clearing it
EXPECT_TRUE(sc.schedule());

}

0 comments on commit 361e509

Please sign in to comment.