Skip to content

Relax error check for oa controller actuator node#4715

Merged
jasondegraw merged 5 commits intodevelopfrom
87630504-OAControllerActuatorNode-Issue4272
Feb 13, 2015
Merged

Relax error check for oa controller actuator node#4715
jasondegraw merged 5 commits intodevelopfrom
87630504-OAControllerActuatorNode-Issue4272

Conversation

@mjwitte
Copy link
Contributor

@mjwitte mjwitte commented Feb 5, 2015

Previously if the Controller:OutdoorAir actuator node was not on an OutdoorAir:Node list then this was a fatal error. Now it's just a warning to allow usage such as drawing OA from a preconditioning space such as a thermal labyrinth. Fixes #4272

GetOAControllerInputs was refactored to facilitate unit tests. Part of this routine was moved to ProcessOAControllerInputs. Unit tests were still a bit messy though requiring some additional setup, opening the err file, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjwitte I'm not sure why these are static (and StandardErrorOutput below). Is there a reason these aren't just:

bool ErrorsFound = false;
int ControllerNum = 0;

Maybe I am missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasondegraw Good question. Probably because this is static here (which is what I copied to make this one):

https://github.com/NREL/EnergyPlus/blob/develop/tst/EnergyPlus/unit/HeatBalanceManager.unit.cc#L36

So, why are those static there? Because it's static here and I probably copy/pasted the declaration from here:

https://github.com/NREL/EnergyPlus/blob/develop/src/EnergyPlus/HeatBalanceManager.cc#L327

And it's not clear to me that even the one in HeatBalanceManager needs to be static because GetHeatBalanceInput is only called once (fairly certain about that, but not completely).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjwitte Looking around some it looks like static is getting used a lot elsewhere, too. So maybe the best thing is to get rid of the static modifier here and leave it alone elsewhere. It might be a bigger task to look at all of the uses of static than we want to get involved in right now. How does that sound to you?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasondegraw Sounds good. Done.

@jasondegraw
Copy link
Member

@mjwitte I removed one more static, removed a local scope, and then added some deallocations of globals that @Myoldmopar wanted. I don't foresee any of those changing the all-green CI status, so I expect to merge this once the CI comes back

@mjwitte
Copy link
Contributor Author

mjwitte commented Feb 12, 2015

@jasondegraw Thanks for the cleanup.

jasondegraw added a commit that referenced this pull request Feb 13, 2015
…Issue4272

Relax error check for oa controller actuator node Issue 4272
@jasondegraw jasondegraw merged commit 7f48b25 into develop Feb 13, 2015
@jasondegraw jasondegraw deleted the 87630504-OAControllerActuatorNode-Issue4272 branch February 13, 2015 14:32
@Myoldmopar Myoldmopar changed the title Relax error check for oa controller actuator node Issue 4272 Relax error check for oa controller actuator node Mar 7, 2015
@Myoldmopar Myoldmopar added the Defect Includes code to repair a defect in EnergyPlus label Mar 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Defect Includes code to repair a defect in EnergyPlus

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Thermal Labyrinth Modeling and connecting AHU OA inlet to a zone exhaust node

4 participants