PointCloud2 won't unsubscribe from Topic

[roddc]

Description The PointCloud2 class cannot unsubscribe from the topic, because the reference of the callback is not the same as the one subscribed, and Roslib.Topic will not be unsubscribed if there's any other listeners.

• Library Version: latest
• ROS Version: Melodic
• Platform / OS: Ubuntu 20.04

Steps To Reproduce

const ros = new Ros({
  url: "ws://localhost:9090",
});

const tfClient = new TFClient({
  ros,
  angularThres: 0.01,
  transThres: 0.01,
  rate: 10.0,
  fixedFrame: "/map",
});

const viewer = new Viewer({
  divID: "map",
  width: window.innerWidth,
  height: window.innerHeight,
  background: "#708986",
  antialias: true,
  near: 0.1,
  far: 1000,
  cameraPose: {
    x: 0,
    y: 0,
    z: 100,
  },
});

const pc = new PointCloud2({
  ros,
  topic: "livox/lidar",
  tfClient: tfClient,
  rootObject: viewer.scene,
  material: { size: 0.5, color: "#EE0000" },
  throttle_rate: 1000,
});
// This doesn't work
pc.unsubscribe();

 this.rosTopic.subscribe(this.processMessage.bind(this));

 this.rosTopic.unsubscribe(this.processMessage);

Roslib.Topic
  // If there is any other callbacks still subscribed don't unsubscribe
    if (this.listeners('message').length) { return; }

I think this bug may appear in other classes like LaserScan.

Expected Behavior The topic should be unsubscribed.

Actual Behavior The unsubscribe function of PointCloud2 is not removing the correct callback.

[MatthijsBurgh]

I don't see what is wrong. So I wouldn't know how to fix it. Maybe @rayman has a clue, to fix this.

[Rayman]

this.rosTopic.subscribe(this.processMessage.bind(this));
this.rosTopic.unsubscribe(this.processMessage);

You need to unsubscribe with the exact same function. bind creates a new function so this doesn't work

[MatthijsBurgh]

@rayman Thanks! This is done in many places in the library. So this is broken in all those places.

Would it be fixed by changing the unsubscribe to this.rosTopic.unsubscribe(this.processMessage.bind(this));? Or does every call to bind create a new function that doesn't the old reference?

[Rayman]

bind creates a new function, so that doesn't work. I'm thinking how to solve this, but it has been a few years since I've programmed in javascript.

[roddc]

bind creates a new function, so that doesn't work. I'm thinking how to solve this, but it has been a few years since I've programmed in javascript.

There're few solutions:

1. Creating a class member e.g. this.boundProcessMessage = this.processMessage.bind(this); in the class constrcutor, and use it in the subscribe and unsubscribe functions.
2. Using arrow function, e.g. this.callback = (msg) => this.processMessage(msg).

Hope it can help.

[MatthijsBurgh]

This has already been done in other parts of the lib, this should be extend to the entire lib. https://github.com/RobotWebTools/ros3djs/blob/c474671126631134c296d2cd486655b360326c37/src/interactivemarkers/InteractiveMarkerHandle.js#L44

[Tiryoh]

This problem seems like fixed in https://github.com/RobotWebTools/ros3djs/pull/635 Thanks @MatthijsBurgh!