roslibjs
roslibjs copied to clipboard
RobotWebTools
Feature/sharedworker
Public API Changes There is a new transportLibrary : sharedworker There is a new transportOption : sharedWorkerURL
Description A shared worker allow to create only One Websocket connection to rosbridge server for several browser windows (or tab). This is like a worker (that run a script in a separate thread), but this worker is "shared" between all openned windows or tab.
You can try the exemple provided. If you open it in several windows of the same browser, there will be only one connection to the rosbridge server.
Of course, if you open one firefox and one chromium, you will have two connections.
I am not very good with Karma, so I did a little hack to make the test passing (I copy the worker script close to the test script). I tried to modify the karma.conf file to make it work without this copy, but I failed !
We've got a very big application that use roslibjs. During the next weeks, we will try this new feature in this application (I already did some tests on a smallest app). If we don't have any regressions, I will mark this PR as "mergeable". For now, I just publish it to have a first feed back.
Regards,
Before looking into the actual code, why are you coping the new file instead of including it in the build files?
@MatthijsBurgh : If you speek about my hack for test. I precisely tried to do what you suggest. I add the build/sharedWorkerImpl.js in the list of files that karma must serve for my tests. I tried to play with the "base" argument of the karma.conf.js file. But, unsuccessfully. I also look at this closed karma issue : https://github.com/karma-runner/karma/issues/1302...
As I told in my comment, If the rest of the PR is good for you, I will work on this karma configuration.
Ok, I don't like to add this file too... But I think we need to do it.
At the begining, I tryied to use webworkify (like you did for the worker). You can look at this commit :
But doing that, I learned that webworkify generate an unique URL each time we call it. So if in two browser tabs we call webworkify on the same resource, the URL will be differents. And so the browser can't understand that the resource we are fetching is the same file. So it instanciate a differents shared worker... So by using webworkify, everythings works, but our shared worker is not shared...
My conclusion is as follow :
To make shared worker works we need to load them via the strictly same URL in all the tabs. This is why I added the new transportOption.sharedWorkerURL. In the exemple I provide in this PR, i set it to ../build/sharedWorkerSocketImpl.js.
In my private application (based on angular cli), I add the sharedWorkerSocketImpl.js in the assets of my application.
For me the additional file that is required, is too big of a breaking point, to allow this feature.
I understand.
If someone have an idea on how to generate a constant URL when we create a new Blob in javascript I am fully open !
I will try to patch webworkify to make it work with shared workers. If I succeed, it will be trivial to remove this file in the build folder.
@MatthijsBurgh : I succeed to make the shared worker works with the workerscript contained in the bundle. The trick is here : https://github.com/RobotWebTools/roslibjs/pull/524/files/3d63be984c76c0291e09f6b4f58e77c971f75ea5..675a9066970fb45ee84086e8fad4337b2de3f8fe#diff-c8b8dc98173d711d9a5f56d84c366c58ca3bd253a3cc53ca5ef08ef8c2a5dda9R10
I use brfs. This is a small browserify extension that allow to inline content of a file in a browserified javascript file. Then I create a data URL with the base64 encoded worker script file. Hence, URL is the same across all my browser context, and the shared worker is shared.
@adrienbarral Good job.
Could you please rebase on the current develop branch. So that GH actions can run.
You need to extend test/build.bash to include test-sharedworkersocket
@adrienbarral @lroixblue I have converted this to draft. Please fix the issues and just let me know when this PR is ready for a final review.
Thank you @MatthijsBurgh, we are still working on it. We will notify you when we will be ready for a final review.
@lroixblue please let me know, whether you have any plans to work on this. As we are getting close to making a final v1 release. After which more significant changes will be made. Which will make it impossible to merge this.