-
Notifications
You must be signed in to change notification settings - Fork 460
Removal of Unused Availability Manager for AirLoopHVAC:OutdoorAirSystem #8884
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
Code, IDD, and IDF (many) changes to get rid of the unused last input field for the AirLoopHVAC:OutdoorAirSystem. Also had to change many unit tests to account for this change. Added a unit very basic unit test to read the input.
This commit includes changes to the transition and expand objects programs, modified documentation, and many more idfs that were missed in Part 1.
Correction to two more input files. Hopefully the last ones.
|
Anyone have any problem with this change? Removing an unused field is acceptable to me, I just don't want to accept this and someone immediately mention that they are about to implement the availability manager logic to make the field used... I'll hold on this for now, but if I don't hear anything soon then I'll get this pulled up with develop for final review. |
|
@Myoldmopar I posted this on Slack just over a week ago and the only reply I got was from @mjwitte who was okay with getting rid of it. If you want to let this sit for a little while longer, that's fine. |
Oh I don't. 😄 I'll take a final look and test it and get it merged in. I knew I had seen a conversation about it, but I didn't see it here on the PR so I thought I was making it up... |
Myoldmopar
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.
👍 Not much to say, this is good to go.
| \type object-list | ||
| \object-list ControllerLists | ||
| A3, \field Outdoor Air Equipment List Name | ||
| A3; \field Outdoor Air Equipment List Name |
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.
👍
| PSZ-AC:1_OA_Controllers, !- Controller List Name | ||
| PSZ-AC:1_OA_Equipment, !- Outdoor Air Equipment List Name | ||
| PSZ-AC:1 Availability Manager List; !- Availability Manager List Name | ||
| PSZ-AC:1_OA_Equipment; !- Outdoor Air Equipment List Name |
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.
👍
| state, CurrentModuleObject + " = \"" + AlphArray(1) + "\" invalid " + cAlphaFields(4) + "=\"" + AlphArray(4) + "\" not found."); | ||
| ErrorsFound = true; | ||
| } | ||
| } |
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.
👍
| CALL AddToObjFld('Controller List Name', base + vsAirHandlerNameOff,' OA System Controllers') | ||
| CALL AddToObjFld('Outdoor Air Equipment List Name', base + vsAirHandlerNameOff,' OA System Equipment') | ||
| CALL AddToObjFld('Availability Manager List Name', base + vsAirHandlerNameOff,' Availability Managers',.TRUE.) | ||
| CALL AddToObjFld('Outdoor Air Equipment List Name', base + vsAirHandlerNameOff,' OA System Equipment',.TRUE.) |
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.
Set to true to end the object and remove the now unneeded line 👍
| CALL GetNewObjectDefInIDD(ObjectName,NwNumArgs,NwAorN,NwReqFld,NwObjMinFlds,NwFldNames,NwFldDefaults,NwFldUnits) | ||
| nodiff=.false. | ||
| OutArgs(1:3)=InArgs(1:3) | ||
| CurArgs = CurArgs - 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.
Simple transition here. Good.
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.
@RKStrand I don't see this rule added to \src\Transition\InputRulesFiles\Rules9-5-0-to-9-6-0.md Since this is merged, you can slip that into one of your other PRs.
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.
@mjwitte Whoops! Totally forgot about that. I'll attach it to the next defect that I work on. I'll also update that file because currently it is "blank" but it talks about V9.4 to V9.5 not V9.5 to V9.6.
|
1050 lines of "code" removed from the repo. I'll take it. Thanks @RKStrand ! |
Pull request overview
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.