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

Support serialization of state #55

Merged
merged 9 commits into from
Feb 17, 2020
Merged

Support serialization of state #55

merged 9 commits into from
Feb 17, 2020

Conversation

markaren
Copy link
Member

Not able to test this using PyFMI as it raises a NotImplementedError.
But works with FMI4j.

Closes #54

Copy link
Collaborator

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

I would separate the (de)serialization action in Python from the state getter/setter.

Should we also raise an error if the is false and the FMU user tries to call those functions?

Side question: regarding the integration test, would it be easy to set a CI job testing the capabilities through FMI4J as it covers area not available through pyfmi?

pythonfmu/fmi2slave.py Outdated Show resolved Hide resolved
pythonfmu/fmi2slave.py Outdated Show resolved Hide resolved
@markaren
Copy link
Member Author

Side question: regarding the integration test, would it be easy to set a CI job testing the capabilities through FMI4J as it covers area not available through pyfmi?

It's possible, but keeping it up to date is not so straightforward... So I dunno. If we get FMPy up and running we could use that.

@markaren markaren changed the base branch from improve-state-handling to master February 16, 2020 16:13
# Conflicts:
#	pythonfmu/fmi2slave.py
@fcollonval
Copy link
Collaborator

Side question: regarding the integration test, would it be easy to set a CI job testing the capabilities through FMI4J as it covers area not available through pyfmi?

It's possible, but keeping it up to date is not so straightforward... So I dunno. If we get FMPy up and running we could use that.

Thanks for the feedback. From my trial on subinterpreter, using FMPy seems really tricky. It may have something to do with how to interact with c code.

@markaren
Copy link
Member Author

How about this?

Copy link
Collaborator

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thank you for the modifications. This fits perfectly what I meant.

There are just two typing changes to revert. But otherwise this is good to go.

pythonfmu/fmi2slave.py Outdated Show resolved Hide resolved
pythonfmu/fmi2slave.py Outdated Show resolved Hide resolved
markaren and others added 2 commits February 17, 2020 07:56
Co-Authored-By: Frédéric Collonval <[email protected]>
Co-Authored-By: Frédéric Collonval <[email protected]>
@markaren markaren merged commit e93211c into master Feb 17, 2020
@markaren markaren deleted the feature/serialize-state branch February 17, 2020 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Support serialize/deserialize state
2 participants