-
Notifications
You must be signed in to change notification settings - Fork 61
Finish error handling #278
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
5377e62
to
47cd01b
Compare
Rebase this one and see if you can use |
@adeas31 do you have a simple way / an example to force an error? I would like to check how to evaluate the output of |
Run the following script with omc, loadString("model M Real foo = \"str\"; annotation (uses(Modelica(version=\"4.0.0\"))); end M;");
getMessagesStringInternal();
checkModel(M);
getMessagesStringInternal(); equivalent OMPython code is, from OMPython import OMCSessionZMQ
omc = OMCSessionZMQ()
omc.sendExpression("model M Real foo = \"str\"; annotation (uses(Modelica(version=\"4.0.0\"))); end M;")
omc.sendExpression("getMessagesStringInternal()")
omc.sendExpression("checkModel(M)")
omc.sendExpression("getMessagesStringInternal()") The first call to You can also see how |
8f6d05d
to
31ef902
Compare
Let me know once its ready for review. |
31ef902
to
fe5f3e3
Compare
@adeas31 all done including Please use the following order for review: PR #281, PR #282 (needs a simple conflict solution), PR #278 PR #279 is currently not updated as it would be on top of PR #278 Besides these, there are some smaller cleanups pending |
08d1007
to
5d9b182
Compare
All the dependent PRs are merged. Lets rebase this one now. |
5d9b182
to
4d8a0e7
Compare
@adeas31 ready to go ... |
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.
LGTM.
@@ -325,6 +326,9 @@ def __init__(self, timeout=10.00, | |||
# connect to the running omc instance using ZMQ | |||
self._connect_to_omc(timeout) | |||
|
|||
self._re_log_entries = None | |||
self._re_log_raw = None |
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.
Do we really need to make them class members?
error_raw = self._omc.recv_string() | ||
# run error handling only if there is something to check | ||
if error_raw != "{}\n": | ||
if not self._re_log_entries: |
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.
self._re_log_entries
and self._re_log_raw
can be local variables here.
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 defined them as class variables to do the initialisation once (if needed at all). Later, the compiled re struture is just reused ... However, if the expected numbers of calls to this part is low, it makes sense to run this each time.
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.
The actual re compilation is run each time, its just they are initialized to none only once. I don't think initializing them to none locally is expensive.
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.
in OMCSession.py line 584ff (definition of sendExpression()):
if not self._re_log_entries:
self._re_log_entries = re.compile(...)
if not self._re_log_raw:
self._re_log_raw = re.compile(...)
Compilation is done only once (on first call; self._re* class variables are None); after that, the variables are set (not None) and, thus, the compiled version is used ...
This reverts commit 2c3ab36.
needs the preparation / additional changes in OMCSession* and ModelicaSystem
this is handled in OMCSessionZMQ.sendExpression()
…rors for each command using sendExpression()
4d8a0e7
to
94ec48b
Compare
Finish update of error handling in OMSession.py and ModelicaSystem.py
Based on PR #277, #273 and #270
Changes