-
Notifications
You must be signed in to change notification settings - Fork 222
V25.2.0-IOFreeze: ExternalInterface's optional Initial Value field #5481
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
|
CI Results for e4d1e32:
|
jmarrec
left a comment
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.
Looks good to me. In case someone wonders why there is no FT change necessary here, it's because the FT is written in old/poor style to begin with and puts everything in an optional already.
Lines 54 to 57 in 2b0e50e
| d = modelObject.initialValue(); | |
| if (d.is_initialized()) { | |
| idfObject.setDouble(ExternalInterface_FunctionalMockupUnitExport_To_ActuatorFields::InitialValue, d.get()); | |
| } |
Lines 49 to 52 in 2b0e50e
| d = modelObject.initialValue(); | |
| if (d.is_initialized()) { | |
| idfObject.setDouble(ExternalInterface_FunctionalMockupUnitExport_To_ScheduleFields::InitialValue, d.get()); | |
| } |
| * [#5481](https://github.com/NREL/OpenStudio/pull/5481) - ExternalInterface's optional Initial Value field | ||
| * Field `Initial Value` is made optional for `ExternalInterface:FunctionalMockupUnitExport:To:Schedule` and `ExternalInterface:FunctionalMockupUnitExport:To:Actuator` | ||
| * API-breaking change for `ExternalInterface:FunctionalMockupUnitExport:To:Schedule` and `ExternalInterface:FunctionalMockupUnitExport:To:Actuator`: | ||
| * `initialValue` (`double` to `boost::optional<double>`) |
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.
Related E+ PR is
Thanks for remembering to add the API break to the release notes.
| \retaincase | ||
| N1 ; \field Initial Value | ||
| \type real | ||
| \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.
I'm not 100% sure this is correct on E+ side, asking for clarification in
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.