WEKIT-ECS / MIRAGE-XR

MirageXR is a reference implementation of an XR training system. MirageXR enables experts and learners to share experience via XR and wearables using ghost tracks, realtime feedback, and anchored instruction.
Other
27 stars 4 forks source link

Orkestra integration #173

Closed wekitecs closed 2 years ago

wekitecs commented 3 years ago

In GitLab by @fwild on Jan 5, 2021, 11:37

We need to create a CommunicationManager that can be configured to work with Orkestra.

wekitecs commented 3 years ago

In GitLab by @fwild on Jan 14, 2021, 13:06

Do first integration simple - just see if it can be added and show how a 'sharing' call works? then we can investigate further with these two tickets:

wekitecs commented 3 years ago

In GitLab by @wild on Feb 16, 2021, 18:04

@adominguez @itamayo How are things progressing? Does Orkestra now pass the test battery? and the builds?

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 09:29

We wrote for help a 3 weeks ago but not ones answers to the question, so I think we're at the same point ..

212

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 11:30

I have looked into this and left some comments. Maybe you can join on Thursday the dev jour fixe in case this is still a problem? Looks to me that this is not a code problem, but rather a package dependency problem - that the socket lib may not be available at the time it is needed, or may not be available at all for cross platform?

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 11:36

Here are also some alternatives: https://github.com/IBM/socket-io

This one here cuts a corner? https://github.com/floatinghotpot/socket.io-unity

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 12:18

hi fridolin,

I've done test with different libraries of websockets but the best in terms of compatibility and performance is the one we're using. It seems that there is some conflicts with MirageXR's arch, but I am not sure where is that problem. I've tried as nudget package, as an assembly library and other ways but not nothing works. Seem that installs well but at runtime it doesn't resolve the dependencies. I getting married this Friday so I won't be at dev meetings, any way I'll keep trying some other solutions.

The strange things is whether I install it manually using Nudget package it's works, I am not expert on unity so I don't know very well howto resolve this problem.

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 13:24

I believe it must be listed in the dependencies? Or @william.guest are you able to look into the windows runner to see if the package can be installed in the container image?

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 13:24

And: Congratulations to your wedding :)

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 17, 2021, 17:47

i agree that the first option it to include it in the dependencies (Packages/manifest.json).

@itamayo i have a couple of questions, to better understand where the issue may lie: which socketIO implementation are we talking about, and which version? could you provide a link to the repository or nuget package?

when you say that, after installation with npm, it works, does this mean it works in unity, or also when built and deployed onto the HoloLens?

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 18:20

Thanks for the answer, we have developed code in C# but we don't have it packaged for unity. In our projects, we added the libraries to unity and it works correctly, but this project is much more complex than we usually do. We have not tested with HoloLens, as it is not our intention for it to work everywhere either. I understand that being C# it shouldn't have big problems but on that I don't have much idea anymore.

In the end, I have added the libraries manually and it seems that it works, and it passes the tests, but of course for your purpose I understand that it is not a very clean solution.

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 18:26

@william.guest the problem is that using as nudget package it installed correctly:

DisplayProgressbar: Installing SocketIOClient 2.1.3 DisplayProgressbar: Installing Newtonsoft.Json 12.0.3 Refresh completed in 0.000018 seconds. DisplayProgressbar: Installing System.Collections 4.3.0 Refresh completed in 0.000009 seconds. DisplayProgressbar: Installing WebSocketSharp-netstandard 1.0.1 Refresh completed in 0.000004 seconds. DisplayProgressbar: Installing SocketIOClient 2.1.3 Refresh completed in 0.000010 seconds.

But at compiling time it doesn't found the references:

Assets/MirageXR/Common/orkestraLib/plugins/MappingService.cs(7,7): error CS0246: The type or namespace name 'SocketIOClient' could not be found (are you missing a using directive or an assembly reference?)

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 18:32

@itamayo What do you mean by "I have added the libraries manually" - is that that you put the DLLs into the project? Have you checked if listing the package as a dependency helps?

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 17, 2021, 18:36

bit of a long shot, but maybe this is of some use:

https://forum.unity.com/threads/newtonsoft-included-by-unity-package-dependencies-creates-assembly-conflict.960539/

here it says: "The Entities package dependencies include the Performance testing API, which in turn depends on Newtonsoft Json. The latest version of Unity's Newtonsoft Json package appears to include Newtonsoft.Json version 12.0.0."

i am wondering here if there is some conflict between the standard 12.0.0 version of the newtonsoft.json and the expected 12.0.3

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 18:39

@fwild Yes, I put the DLLs into the branch =/ I tried to added to the list as dependency but it doesn't work. @william.guest I think that can be the problem, but I don't know, at least you have a version that works at this branch XD

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 17, 2021, 19:06

i notice that the branch has 2 'packages.config' files, one in assets and one in the root folder. the reference to socketioclient is in the one in the root folder.

could it be that these should be merged? or maybe it is not being found because it is not inside 'assets'?

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 19:16

Good point, I'll try it later, thanks !

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 17, 2021, 19:58

As you mentioned before the problem seems related with dependencies conflicts with Newtonsoft:

PrecompiledAssemblyException: Multiple precompiled assemblies with the same name Newtonsoft.Json.dll included or the current platform. Only one assembly with the same name is allowed per platform. Assembly paths: 4304 /builds/wekit-ecs/mirage-xr/Library/PackageCache/jillejr.newtonsoft.json-for-unity@d276148f40/Plugins/Newtonsoft.Json Editor/Newtonsoft.Json.dll 4305 Assets/Packages/Newtonsoft.Json.12.0.3/lib/net45/Newtonsoft.Json.dll

I try to create own SocketIO pacakge with the same dependency to NewtonSoft...

wekitecs commented 3 years ago

In GitLab by @wild on Feb 17, 2021, 22:20

@a85jafari can you take a look? You recently talked about adding NewtonSoft

wekitecs commented 3 years ago

In GitLab by @a85jafari on Feb 18, 2021, 11:15

@fwild The branch I used Newtonsoft is a seprated branch which is not merged yet. So The problem could not be caused by Moodle integration!

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 18, 2021, 11:40

@fwild can you check out CI pipeline stack ? It's stucked ... thanks

wekitecs commented 3 years ago

In GitLab by @wild on Feb 18, 2021, 12:03

@a85jafari I know - but the question is, have stumbled onto the same issue? It seems the library may be added twice, causing a compile conflict with the CI build pipeline?

wekitecs commented 3 years ago

In GitLab by @wild on Feb 18, 2021, 12:12

I've cancelled it - tried to restart, but it seems to not pass the get activation file script. Can you try to commit again and see if it passes now (or throws a proper error message)?

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 18, 2021, 12:20

It's says:

This job is stuck because the project doesn't have any runners online assigned to it. Go to Runners page

wekitecs commented 3 years ago

In GitLab by @BenediktHensen on Feb 18, 2021, 12:54

Hi, we are currently experiencing a connection loss to the assigned CI runner server. This is why the job is currently stuck and will time out after one hour.

wekitecs commented 3 years ago

In GitLab by @itamayo on Feb 18, 2021, 12:58

Ok, thanks by let me know it!

wekitecs commented 3 years ago

In GitLab by @wild on Feb 18, 2021, 17:34

Runner is up again!

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 19, 2021, 09:54

according to my CI settings, the docker runner is offline. it says the last communication was 17 hours ago, so i think it may have restarted and gone offline again

wekitecs commented 3 years ago

In GitLab by @wild on Feb 19, 2021, 10:56

For me they show up both as green and running?

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 19, 2021, 10:58

yes, back up now :)

wekitecs commented 3 years ago

In GitLab by @adominguez on Feb 24, 2021, 09:29

Hi! We have been testing the code and we had no problem in building our local version. We also saw that the last commit of Iñigo passed the CI. Then, should we try another push? Apart from that, you told us at the WP3 meeting that you had to try something before we continue with the integration. Is it ready?

wekitecs commented 3 years ago

In GitLab by @william.guest on Feb 24, 2021, 10:20

Hi Ana, thanks for the update. Does this mean that Inigo's efforts to resolve the issue (conflicting/missing packages) have worked?

If so, the next step is to create a merge request from this branch. After that I can review it and, once confirming that it runs without errors, merge it in to the 'develop' branch.

If there are still bugs appearing, another option is remove any code that creates errors and then create a merge request. This will allow future tickets to build on any existing integration.

wekitecs commented 3 years ago

In GitLab by @wild on Feb 24, 2021, 10:43

Looks like we are progressing :). Every merge request will trigger the CI pipelines - it will run the test batteries (edit mode and play mode) and it will attempt to build for Hololens 1 and Hololens 2 (the Android and iOS build pipelines are not yet up and running online). If there is anything different from the local configuration you use to the configuration of the linux and windows runner for building and testing, then it will flag up then...

wekitecs commented 3 years ago

In GitLab by @smasneri on Feb 25, 2021, 11:34

Hi Will, I am not sure. On Iñigo's branch we were able to compile everything without issues, without modifying any file, just installing the required dependencies.

I will create a merge request now, let's hope it works :-)

wekitecs commented 3 years ago

In GitLab by @wild on Feb 25, 2021, 12:38

If I read it correctly, the tests passed without problem: https://platform.xr4all.eu/wekit-ecs/mirage-xr/pipelines/1552 - now let's see if the build pipeline succeeds (that's where it got stuck last time, I believe).

wekitecs commented 3 years ago

In GitLab by @a85jafari on Mar 26, 2021, 16:40

@adominguez Do you think you can create a merge request this week? Otherwise we need to move this ticket to the sprint 7.

wekitecs commented 3 years ago

In GitLab by @a85jafari on Mar 31, 2021, 13:23

I merged the Orkestra branch, and will close this issue. For updating it or adding more feature to it please open new tickets.

wekitecs commented 3 years ago

In GitLab by @a85jafari on Mar 31, 2021, 13:23

closed