Is ROSBridgeHandler meant to be Singleton?

[aaronsnoswell]

Hi again @andreihaidu :)

Thanks for this great repo. I have a question about the way this plugin should be used. Is the user supposed to only have one instance of ROSBridgeHandler in their project? I can't tell from the documentation. I currently have one TSharedPtr<FROSBridgeHandler> per Actor class in my project. This seems to work fine at the moment, but is this the intended way I should use this class?

If ROSBridgeHandler is supposed to be a singleton, maybe we could update the documentation Example.md file to have a section like the following;

#### A note on ROSBridgeHandler

Connections to ROSBridge work via a [`ROSBridgeHandler`](UROSBridge/Source/UROSBridge/Public/ROSBridgeHandler.h) object. There should only be one of these in your project.
If you have multiple actors that need to connect to ROS, a good pattern is to create a custom [GameInstance](https://docs.unrealengine.com/latest/INT/API/Runtime/Engine/Engine/UGameInstance/index.html), and store the `ROSBridgeHandler` object there as a public member.
Your actors can then access the `ROSBridgeHandler` by calling e.g. `this->GetGameInstance().Handler`

Or, better yet, make the class itself a singleton class?

[andreihaidu]

Thats's a good point, in theory it should be a singleton, I don't think it makes sense to have multiple of them (only if you want to connect in parallel to other ROS instances as well). I will add the description to the documentation. I will eave the issue opened until a decision will be made. Thanks for pointing this out!

[aaronsnoswell]

Hmm. I hadn't thought of the use-case where you might want to connect in parallel to two ROSBridge servers.

As I said, currently I have one ROSBridgeHandler for each actor in my scene. This seems to work ~50% of the time, but the other ~50% I get some kind of race condition exception at game end. The actual exception comes from ROSBridgeHandler.cpp:109 and says this->Handler->Client.Object was 0xFFFFFFFFFFFFFEAA. It seems the FWebSocket class doesn't shut down correctly sometimes when there are multiple instances of it.

Given this, I suggest we update the project to provide a base GameInstance class that people can use to store one global ROSBridgeHandler (I have an implementation already that I could package in a PR if you like). This will also require updating the Examples.md wiki page to reflect the new calling pattern for ROSBridgeHandler.

[aaronsnoswell]

I can think of one problem with this approach: Currently ROSBridgeHandler requires all subscribed topics, advertised topics, and advertised services to be registered before calling ROSBridgeHandler::Connect(). If we have one ROSBridgeHandler stored in e.g. the GameInstance, how will it know when to call Connect()? A timeout would be hackish. Perhaps we should update ROSBridgeHandler to support subscribing to topics and advertising topics and services in a lazy fashion, at any point in time after Connect() is called?

[aaronsnoswell]

Created #13

[andreihaidu]

Hi, I also got the this->Handler->Client.Object was 0xFFFFFFFFFFFFFEAA, it happened when ending the gameplay and and disconnecting the handler.

About the singleton, I noticed that having it as multiple handlers it has some advantages, for example I have a TFPublisher plugin, which already has an that takes care of the publishing, with the handler included. So a user can easily add it to his world without needing to change any code. So if he wants to publish something else, it will be another handler for it. Do you think this might be problematic, or have race conditions by having so many handler threads?

[aaronsnoswell]

I agree that it could be useful to have multiple handlers - I'm not sure what is causing the above error that we are both seeing though. Maybe keep this issue open and I'll keep looking in to this error. I'll let you know if I come up with a fix.