Feature/160 fmuproxy integration #162
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 14 commits
January 29, 2019 16:08
missing tests and code-formatting
markaren
requested review from
kyllingstad,
jornsv,
ljamt,
hplatou and
eidekrist
|
The warnings from the thrift headers are (MSVC):
Ok to suppress? |
|
Server executable are now built as part of the CI run. |
|
I took the liberty to run ClangFormat on all "non-generated" files. |
hplatou
approved these changes
Mar 20, 2019
There was a problem hiding this comment.
This seems to work fine for some fmu's but I get errors when loading others(all FMI v2):
../fmuproxy/fmuproxy_helper.hpp:44: Internal error: Failed to parse variability: ''
As far as I remember variability has a default value of 'continuous'. I also suggest to give a better error message in case of trying to load an v1 FMU. Currently you just get an xml-parser error.
Also, as it is now, it is not possible to use fmuproxy through the C-API/SSP. Nevertheless I suggest we merge this PR and later create a new one for exposing through C-API/SSP-config when needed.
I can fix this. It should default to continious if it's empty. |
|
As this branch has many commits, I would suggest to use the "Squash and merge" option when merging 👍 |
but only for release, and don't run tests
markaren
changed the title
|
Getting a weird error on Jenkins windows build during debug test: |
|
We have been trying to run FMU-proxy and the test in this PR according to the guide on confluence, and the test fails. FMU-proxy prints the following message: We have tried on multiple machines with the same result, using both |
|
Probably a version mismatch. It needs master, but apperantly artifacts from
circlrci are not available for others. How did you get it? When Halvor
tested it, I sent a matching build to him.
Den tor. 4. apr. 2019, 09:55 skrev Lars T. Kyllingstad <
[email protected]>:
… We have been trying to run FMU-proxy and the test in this PR according to
the guide on confluence, and the test fails. FMU-proxy prints the following
message:
> java -jar fmu-proxy.jar -thrift/tcp=9090
INFO [main] (GrpcFmuServer.kt:78) - GrpcFmuServer listening for connections on port: 57012
INFO [main] (ThriftFmuServer.kt:83) - ThriftFmuSocketServer listening for connections on port: 9090
INFO [main] (Log.java:193) - Logging initialized @2904ms to org.eclipse.jetty.util.log.Slf4jLog
INFO [main] (ThriftFmuServer.kt:83) - ThriftFmuServlet listening for connections on port: 57063
INFO [Thread-5] (Server.java:374) - jetty-9.4.z-SNAPSHOT; built: 2018-06-05T18:24:03.829Z; git: d5fc0523cfa96bfebfbda19606cad384d772f04c; jvm 1.8.0_101-b13
INFO [main] (RpcHttpServer.kt:60) - FmuProxyJsonHttpServer for connections on port: 57070
INFO [main] (RpcWebSocketServer.kt:57) - FmuProxyJsonWsServer listening for connections on port: 57073
INFO [main] (RpcTcpServer.kt:54) - FmuProxyJsonTcpServer listening for connections on port: 57074
INFO [Thread-5] (AbstractConnector.java:289) - Started ***@***.***{HTTP/1.1,[http/1.1]}{0.0.0.0:57063}
INFO [Thread-5] (Server.java:411) - Started @3077ms
INFO [main] (RpcZmqServer.kt:86) - FmuProxyJsonZmqServer listening for connections on port: 57077
Press any key to exit..
WARN [Thread-2] (AbstractNonblockingServer.java:544) - Got an IOException in internalRead!
java.io.IOException: An existing connection was forcibly closed by the remote host
at sun.nio.ch.SocketDispatcher.read0(Native Method)
at sun.nio.ch.SocketDispatcher.read(Unknown Source)
at sun.nio.ch.IOUtil.readIntoNativeBuffer(Unknown Source)
at sun.nio.ch.IOUtil.read(Unknown Source)
at sun.nio.ch.SocketChannelImpl.read(Unknown Source)
at org.apache.thrift.transport.TNonblockingSocket.read(TNonblockingSocket.java:141)
at org.apache.thrift.server.AbstractNonblockingServer$FrameBuffer.internalRead(AbstractNonblockingServer.java:539)
at org.apache.thrift.server.AbstractNonblockingServer$FrameBuffer.read(AbstractNonblockingServer.java:338)
at org.apache.thrift.server.AbstractNonblockingServer$AbstractSelectThread.handleRead(AbstractNonblockingServer.java:203)
at org.apache.thrift.server.TThreadedSelectorServer$SelectorThread.select(TThreadedSelectorServer.java:591)
at org.apache.thrift.server.TThreadedSelectorServer$SelectorThread.run(TThreadedSelectorServer.java:546)
We have tried on multiple machines with the same result, using both
file:// and http:// FMU URLs. Any ideas what could be wrong?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#162 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFuhJ8NiASrqpSNE-997SQR7tg-e4IaKks5vda_ggaJpZM4abzDo>
.
|
I got it from @hplatou. And he has also tried this today, and got the same error message. |
|
Check out the commit where he applied clng format. Fmu proxy has gone snake
case since then
Den tor. 4. apr. 2019, 10:25 skrev Lars T. Kyllingstad <
[email protected]>:
… Probably a version mismatch. It needs master, but apperantly artifacts
from circlrci are not available for others. How did you get it? When Halvor
tested it, I sent a matching build to him.
I got it from @hplatou <https://github.com/hplatou>. And he has also
tried this today, and got the same error message.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#162 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFuhJxH_5e0XlfLJsZEcxeWzkzZ78sBUks5vdbbwgaJpZM4abzDo>
.
|
|
Great, that fixed it. If you have a version of the fmu-proxy server that is compatible with the most recent commits, it would be great if you could post it on Slack at some point in the near future. :) |
|
I will probably make a new GitHub release next week. That version will of
course be compatible with this integration
Den tor. 4. apr. 2019, 10:56 skrev Lars T. Kyllingstad <
[email protected]>:
… Great, that fixed it. If you have a version of the fmu-proxy server that
is compatible with the most recent commits, it would be great if you could
post it on Slack at some point in the near future. :)
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#162 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFuhJ9f_ZQfkd5JfJBdNfKfKdnWaJSjSks5vdb44gaJpZM4abzDo>
.
|
kyllingstad
reviewed
Apr 4, 2019
|
I don't now if it will work or not, but it has been commented by both Conan
and vcpkg people that the debug build does not work or that it isn't built
at all. But I have not looked to much into it.
Den tor. 4. apr. 2019, 14:31 skrev Lars T. Kyllingstad <
[email protected]>:
… ***@***.**** commented on this pull request.
------------------------------
In cmake/FindThrift.cmake
<#162 (comment)>
:
> @@ -0,0 +1,36 @@
+# Find Thrift
+#
+# Find the native Thrift headers and libraries.
+#
+# THRIFT_INCLUDE_DIRS - where to find thrift/thrift.h
+# THRIFT_LIBRARIES - List of libraries when using Thrift.
+# THRIFT_FOUND - True if Thrift found.
+#
+
+find_path(THRIFT_INCLUDE_DIR NAMES thrift/Thrift.h)
+mark_as_advanced(THRIFT_INCLUDE_DIR)
+
+find_library(THRIFT_LIBRARY NAMES thrift thriftmd)
This will work with Conan, because Conan installs release and debug
libraries in entirely different directory trees. Not sure it will play nice
with vcpkg. Ideally, you should search for thriftmd and thriftmdd
separately and use target properties like IMPORTED_LOCATION_DEBUG and
IMPORTED_LOCATION_RELEASE below.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#162 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFuhJwox_E_x_qwuxI1kjOYVEKutxg-Xks5vdfCdgaJpZM4abzDo>
.
|
…y-integration # Conflicts: # conanfile.py
|
This PR cannot be merged before the Jenkins error is fixed. See error in comment above. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #160 and #164 .
Adds integration with fmu-proxy using thrift.
Some remarks:
conanandvcpkgversion.CSECORE_WITH_FMUPROXYNote: Source files are not formatted and probably do not adhere to the new header convention.
And yeah, you need to add a conan remote to get thrift:
conan remote add helmesjo "https://api.bintray.com/conan/helmesjo/public-conan"UPDATE:
Setup instructions