-
Notifications
You must be signed in to change notification settings - Fork 222
V25.2.0-IOFreeze: SiteWaterMainsTemperature's new fields #5477
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
Conversation
|
Thanks for this! |
|
CI Results for 8e5857b:
|
| N3, \field Temperature Multiplier | ||
| \note If calculation method is Schedule, this input field is ignored. | ||
| \type real | ||
| \units dimensionless | ||
| \required-field | ||
| \minimum 0 | ||
| N4; \field Temperature Offset | ||
| \note If calculation method is Schedule, this input field is ignored. | ||
| \type real | ||
| \units deltaC | ||
| \required-field |
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.
default 1 and default 0 (respectively) replaced by required-field: good
| EXPECT_TRUE(swmt.isEmpty(4)); // Maximum Difference In Monthly Average Outdoor Air Temperatures | ||
| EXPECT_EQ(1.0, swmt.getDouble(5).get()); // Temperature Multiplier | ||
| EXPECT_EQ(0.0, swmt.getDouble(6).get()); // Temperature Offset |
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.
Ok
| std::string update_3_7_0_to_3_8_0(const IdfFile& idf_3_7_0, const IddFileAndFactoryWrapper& idd_3_8_0); | ||
| std::string update_3_8_0_to_3_9_0(const IdfFile& idf_3_8_0, const IddFileAndFactoryWrapper& idd_3_9_0); | ||
| std::string update_3_9_0_to_3_10_0(const IdfFile& idf_3_9_0, const IddFileAndFactoryWrapper& idd_3_10_0); | ||
| std::string update_3_10_0_to_3_10_1(const IdfFile& idf_3_10_0, const IddFileAndFactoryWrapper& idd_3_10_1); |
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.
Should probably have done that on the v25.2.0-IOFreeze brnach, I hope this doesn't create conflicts too often.
| auto iddObject = idd_3_10_1.getObject(iddname); | ||
| IdfObject newObject(iddObject.get()); | ||
|
|
||
| for (size_t i = 0; i < object.numFields(); ++i) { | ||
| if ((value = object.getString(i))) { | ||
| newObject.setString(i, value.get()); | ||
| } | ||
| } | ||
|
|
||
| newObject.setDouble(5, 1.0); | ||
| newObject.setDouble(6, 0.0); |
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.
ok
| ok = setTemperatureMultiplier(1.0); | ||
| OS_ASSERT(ok); | ||
| ok = setTemperatureOffset(0.0); | ||
| OS_ASSERT(ok); |
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.
all good
| siteWater.setTemperatureMultiplier(1.2); | ||
| siteWater.setTemperatureOffset(-0.5); |
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.
These technically return bool, so you should test that they return true. ANd test that siteWater.setTemperatureMultiplier(-10.0) fails.
Don't care that much, just being thorough though.
Pull request overview
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.