-
Notifications
You must be signed in to change notification settings - Fork 61
Update modelica system #277
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
This needs PR #272 - removed this dependency and solved the dependency problem by a temporary solution - use Exception instead of OMCSessionException for now ... |
784f937
to
34921ee
Compare
2c3ab36
to
92d804a
Compare
Seems this one is next in line. Please rebase it so I can start reviewing it. |
92d804a
to
1433499
Compare
@adeas31 done |
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.
However, the verbose flag calls getErrorString()
which is useful in some cases to show the compiler notifications to the user. For example, if you call this,
loadString("model M annotation (uses(Modelica(version=\"4.0.0\"))); end M;");
You will not get the notifications that the Modelica package is loaded due to uses annotation unless you call ``getErrorString()`.
I suggest we always call getErrorString()
inside sendExpression
and logs it using the logger. To further enhance this, we can use getMessagesStringInternal()
instead of getErrorString()
since getErrorString()
returns string without any structure you don't know if its an error, warning or a notification. If we use getMessagesStringInternal()
we can properly log using error, warning and info.
This is done in PR #278; please check mainly 7658886; I will check the usage of |
Update ModelicaSystem on top of PR #273
Changes: