gbXML to HB minor bug re: loading constructions/materials into HB libraries
Closed this issue · 6 comments
Heads up @chriswmackey:
I just updated and tested the updated gbXML to HB component, and it looks like the latest update doesn't force all the constructions/materials to be in upper case, which means it's still failing to be found in subsequent HB components.
I've fixed the problem on my end, so I should be able to send you a PR with my fix end of day.
@saeranv ,
As you see in my comments on the PR, I believe that @mostaphaRoudsari has already addressed this now.
Let me know if you feel otherwise.
@chriswmackey yeap it works. I suppose I didn't notice the syntax error, and rewrote it with the getter to resolve the issue. In theory, if the name object in OpenStudio isn't initialized, my method of using the get() method is the correct solution. However I don't know that for sure, so I'm happy to close this PR, and if anyone else has an issue, we can revist this theory and see if the getter is the issue.
@saeranv, sorry that I didn't see this PR sooner. This addresses the issue as well. I just jumped to the solution after seeing the issue on the forum without checking your PR. Sorry!
No problem @mostaphaRoudsari !
@saeranv ,
Admittedly, using get() AFTER checking whether a parameter is initialized is probably what the OpenStudio developers would recommend. However, using get() when a parameter is not initialized will result in an uncaught exception that causes all of Rhino to crash (a painful experience that I have endured many times while developing for OpenStudio). For this reason, it is important to check if a parameter is initialized first before using get() and is possibly why your code here might be a bit less than ideal.
Also admittedly, there are a lot of places across the OpenStudio component code where get() is used without checking whether the parameter is initialized. This is a poor practice that I meant to correct at some point but, now that we are embarking on a long stint of [+] development, it seems better to just implement it correctly there rather than spending a lot of time going back.
@chriswmackey oh yes, I forgot if should be:
if obj.is_initialized(): obj.get()
Yeah, I've definitely hard crashed making this mistake.
S