Skip to content
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

Use logging in modelica system syntron #228

Merged

Conversation

syntron
Copy link
Contributor

@syntron syntron commented Nov 5, 2024

Update ModelicaSystem to use Exceptions / the python logging interface (see #227)

It is based on #226 due to the develop history; if needed it could be rebased or parts could be extracted

@adeas31
Copy link
Member

adeas31 commented Nov 6, 2024

It is based on #226 due to the develop history; if needed it could be rebased or parts could be extracted

Yes, please rebase this on master so the external dll path commits are not included.

@syntron syntron force-pushed the use_logging_in_ModelicaSystem_syntron branch from 3e16a13 to 7916694 Compare November 7, 2024 18:11
@syntron
Copy link
Contributor Author

syntron commented Nov 7, 2024

Rebased to master (as #226 is included now)

## Show notification or warnings to the user when verbose=True OR if some error occurred i.e., not result
if verbose or not result:
print(self.requestApi('getErrorString'))
if not result:
Copy link
Member

Choose a reason for hiding this comment

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

Check for self._verbose flag here. loadLibrary raises some warnings and notifications that are good to know specially when verbose is set.

if verbose:
print("| info | setParameters() failed : It is not possible to set the following signal " + "\"" + name + "\"" + ", It seems to be structural, final, protected or evaluated or has a non-constant binding, use sendExpression(setParameterValue("+ self.modelName + ", " + name + ", " + value + "), parsed=false)" + " and rebuild the model using buildModel() API")
if self._verbose:
logger.warning("setParameters() failed : It is not possible to set " +
Copy link
Member

Choose a reason for hiding this comment

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

Should be logger.info

if self._raiseerrors:
raise ModelicaSystemError("OM error: {}".format(errstr))
else:
logger.warning(errstr)
Copy link
Member

Choose a reason for hiding this comment

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

This should be logger.error

@syntron syntron force-pushed the use_logging_in_ModelicaSystem_syntron branch from 8d412f2 to f23c7f6 Compare November 11, 2024 21:24
@syntron
Copy link
Contributor Author

syntron commented Nov 11, 2024

@adeas31 update based on your comments

Copy link
Member

@adeas31 adeas31 left a comment

Choose a reason for hiding this comment

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

LGTM

loadMsg = self.getconn.sendExpression(loadFileExp)
## Show notification or warnings to the user when verbose=True OR if some error occurred i.e., not result
if verbose or not loadMsg:
return print(self.getconn.sendExpression("getErrorString()"))
if not loadMsg:
Copy link
Member

@adeas31 adeas31 Nov 12, 2024

Choose a reason for hiding this comment

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

Forgot to add check for self._verbose flag here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in an added commit; see below ...

@adeas31
Copy link
Member

adeas31 commented Nov 12, 2024

@arun3688 can you please review and test it.

@adeas31 adeas31 merged commit d293943 into OpenModelica:master Nov 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants