-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #4911 - Avoid extra warning message in ShadingControl #4914
Conversation
@@ -406,7 +394,7 @@ namespace model { | |||
|
|||
void ShadingControl_Impl::resetSetpoint2() { | |||
std::string shadingControlType = this->shadingControlType(); | |||
if (ShadingControl_Impl::isControlTypeValueNeedingSetpoint1(shadingControlType)) { | |||
if (ShadingControl_Impl::isControlTypeValueNeedingSetpoint2(shadingControlType)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bug found by @brgix
// 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(); // For backward compatibility | ||
// resetSetpoint2(); | ||
// resetSchedule(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this block that was a TODO and issues an unnecessary Warn
Testing the code from the #4911 with this branch. First test code, warning is removed [1] OS-build-release(main)> model = OpenStudio::Model::Model.new
=> #<OpenStudio::Model::Model:0x000055d047110f40 @__swigtype__="_p_openstudio__model__Model">
[2] OS-build-release(main)> shd = OpenStudio::Model::Shade.new(model)
=> #<OpenStudio::Model::Shade:0x000055d04708b1d8 @__swigtype__="_p_openstudio__model__Shade">
[3] OS-build-release(main)> ctl = OpenStudio::Model::ShadingControl.new(shd)
=> #<OpenStudio::Model::ShadingControl:0x000055d047019f88 @__swigtype__="_p_openstudio__model__ShadingControl">
[4] OS-build-release(main)> ctl.setSetpoint(18)
=> true
[5] OS-build-release(main)> ctl.setShadingControlType("OnIfHighOutdoorAirTempAndHighSolarOnWindow")
=> true Here you still get the warning that tells you you are setting a Setpoint2 when it's not needed. Which I think is fine IMHO. [7] OS-build-release(main)> model = OpenStudio::Model::Model.new
=> #<OpenStudio::Model::Model:0x000055d046e2a308 @__swigtype__="_p_openstudio__model__Model">
[8] OS-build-release(main)> shd = OpenStudio::Model::Shade.new(model)
=> #<OpenStudio::Model::Shade:0x000055d046da9140 @__swigtype__="_p_openstudio__model__Shade">
[9] OS-build-release(main)> ctl = OpenStudio::Model::ShadingControl.new(shd)
=> #<OpenStudio::Model::ShadingControl:0x000055d046d23ef0 @__swigtype__="_p_openstudio__model__ShadingControl">
[10] OS-build-release(main)> ctl.setSetpoint(18)
=> true
[11] OS-build-release(main)> ctl.setSetpoint2(100)
[openstudio.model.ShadingControl] <0> Object of type 'OS:ShadingControl' and named 'Shading Control 1' has a Shading Control Type 'OnIfHighSolarOnWindow' which does not require a Setpoint2
=> false
[12] OS-build-release(main)> ctl.setShadingControlType("OnIfHighOutdoorAirTempAndHighSolarOnWindow")
=> true Test 1
-[openstudio.model.ShadingControl] <0> Object of type 'OS:ShadingControl' and named 'Shading Control 1' has a Shading Control Type 'OnIfHighOutdoorAirTempAndHighSolarOnWindow' which does require a Setpoint, not resetting it
Test 2
[openstudio.model.ShadingControl] <0> Object of type 'OS:ShadingControl' and named 'Shading Control 1' has a Shading Control Type 'OnIfHighSolarOnWindow' which does not require a Setpoint2
-[openstudio.model.ShadingControl] <0> Object of type 'OS:ShadingControl' and named 'Shading Control 1' has a Shading Control Type 'OnIfHighOutdoorAirTempAndHighSolarOnWindow' which does require a Setpoint, not resetting it |
CI Results for c92ab5f:
|
Pull request overview
FYI @brgix
Pull Request Author
src/model/test
)src/energyplus/Test
)src/osversion/VersionTranslator.cpp
)Labels:
IDDChange
APIChange
Pull Request - Ready for CI
so that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.