After Disconnect/Connect, previous topics can't be correctly re-subscribed to "Listener already available in ros.listeners[0]"

[jaguardo]

using the simple example:
https://flynneva.github.io/react-ros/examples/simple_echo/

with a ROS turtle simulator.
When connected, can add "Topic to echo". However if "toggle connect" to disconnect and then again to reconnect, this message appears in the console:

Listener already available in ros.listeners[0]

and the topic is no longer pushed to the console...

However, if another topic is entered into the "topic to echo:" input it will subscribe, find without that error? and its message pushed to the console... can then repeat the disconnect/connect process and now that topic will no longer be subscribed and a similar message:

Listener already available in ros.listeners[1]

so need to be able to check if the listener is already created and use the available one, versus try to recreate a new one?

[jaguardo]

looks like that check is already being done as that is where the error message is coming from

index.js...

  function createListener(topic, msg_type, to_queue, compression_type) {
    var newListener = new ROSLIB.Topic({
      ros: ros.ROS,
      name: topic,
      messageType: msg_type,
      queue_length: to_queue,
      compression: compression_type
    });

    for (var listener in ros.listeners) {
      if (newListener.name === ros.listeners[listener].name) {
        console.log('Listener already available in ros.listeners[' + listener + ']');
        return ros.listeners[listener];
      }
    }

    ros.listeners.push(newListener);
    console.log('Listener created');
    return newListener;
  }

not sure why it wouldn't allow msgs to be pushed to the console then...
related? RobotWebTools/roslibjs#267

[jaguardo]

Perhaps with the reconnect the value of ros.ROS changes?

Might be able to add an && to that if statement?

if (newListener.name === ros.listeners[listener].name && newListener.ros ===ros.listeners[listener].ros)
you will then have 2 listeners within the same ros, but referencing two different ROS?

So maybe instead... reset the old listeners to the new ros.ROS if it changes??

  function setListenersToNewROS(){
    for (var listener in ros.listeners) {
      ros.listeners[listener].ros  = ros.ROS
    }
  }

in handleConnect (index.js)

var handleConnect = function handleConnect() {
    try {
      ros.ROS = new ROSLIB.Ros({
        url: ros.url
      });
      if (ros.ROS) ros.ROS.on('connection', function () {
        setListenersToNewROS()
        setROS(function (ros) {
          return _extends({}, ros, {
            isConnected: true
          });
        });
        getTopics();
      });
      if (ros.ROS) ros.ROS.on('error', function (error) {
        console.log(error);
      });
    } catch (e) {
      console.log(e);
    }
  };

?

[jaguardo]

This may work too:

    for (var listener in ros.listeners) {
      if (newListener.name === ros.listeners[listener].name) {
        console.log('Listener already available in ros.listeners[' + listener + ']');
        ros.listeners[listener].ros = ros.ROS;
        return ros.listeners[listener];
      }
    }

however using it with the example code:

    for (var i in topics) {
      if (topics[i].path == topicInput) {
        listener = createListener( topics[i].path,
                                   topics[i].msgType,
                                   Number(queue),
                                   compression);
        break;
      }
    }

actually adds listeners, so you get additional messages pushed to the console.

[jaguardo]

well, ok, it doesn't add listeners... it subscribes multiple times to the same listener... so the example code does this

listener.subscribe(handleMsg);

regardless if the listener was already contained in the ros.listeners, so if you delete the last alphanumeric of the topic path and then re-add it - react-ros returns the previous listener, but the example code then adds another subscribe and adds extra messages to the console.

[jaguardo]

possibly related: RobotWebTools/roslibjs#288

[jaguardo]

So still having issues with this (finally have some time to re-attack it!) in the example code-if put a topic in the "Topic to echo"

I get this on the console:

If I change the topic in the scratch pad and put it back (backspace on the "s" and then return it):

I get double messages (it subscribes to the listener twice?)... and if I change the IP to the ros connected to (backspace the last bit and re-add), I get this:

the message doesn't come back. Even if I try to remove and re-add part of the topic again, I can't get the messages to come back. I'd rather not have to hit the refresh button every time there is a disconnect or blip in the connecting... which is the only way I've found to get out of the errors.

could be something in my code, but I'm having a hard time determining how it is different from the sample.

[jaguardo]

I think if you modify this in the example, you might be able to eliminate the double messaging (see my comment on RobotWebTools/roslibjs#288)... I don't have the example currently working to check, but it seems to have eliminated my issue... even though it is a bit of a clunky workaround.

const unsubscribe = () => {
  if (listener) {
      console.log("Unsubscribing");
    if (!(listener._events === undefined || listener._events.message === undefined)) {
      listener.unsubscribe();
      listener._events.message = null;
    }
  }
}

[jaguardo]

As I wrote in the other thread...
listener.removeAllListeners()
seems to be a better way than listener.unsubscribe() and/or setting the message to null.

[flynneva]

@jaguardo thanks for digging into this. im not sure if ill have time to get to implementing the change this week - maybe by this weekend.

if you'd like to make a PR with any changes you think would be useful/could fix this issue it would help me out a lot!

[jaguardo]

I'm not sure how it should change (or if?)... making a change to the example makes sense.

could add it to the handleConnect/handleDisconnect functionality - which I think would be useful, but that might not fit with the "ROS" philosophy/standard? (ROS is still a little new for me)

From what I can tell they expect the user to handle this, but the methods to do that are obviously not intuitive. I would think anytime there is a disconnect and a reconnect that the topics/listeners should be erased (would probably break a lot of folks code though!) since you could be connecting to another ROS-bridge and might have different topics and the need for different listeners. And/or if you ever wanted to connect to more than 1 ROS connection, wouldn't want the listeners message handlers accumulating.

I also noticed if I subscribe to publish to a topic, (I'm not sure how this is mechanized either yet) it also requires removing the ros component from the listener as I think there is a similar problem there where it keeps adding to it rather then removing and creating a new one... so when I disconnect/connect I can no longer publish without refreshing the page, unless I do this before creating the listener:

listener.ros.removeAllListeners();

[jaguardo]

I cant seem to push a new branch on this repo... might be some permission issue? here is what I would have pushed, made a pull request:

...
      setROS(ros => ({ ...ros, isConnected: false }));
      setROS(ros => ({ ...ros, topics: [] }));
      setROS(ros => ({ ...ros, listeners: [] }));
...

add an additional line to reset the listeners back to blank, seems to help/work. not sure what other effects it will have yet. would be nice to eventually get to a point where even if there is an interruption (self initiated or otherwise) that the listeners and/or topics could be saved... or when creating the ros.ros connection quickly re-load what was used previously.

[flynneva]

@jaguardo if you fork it and then push to your fork, you should be able to make a PR that way.