Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MarkerClient: Subscribe to MarkerArray topic #74

Merged
merged 4 commits into from
Aug 4, 2014

Conversation

T045T
Copy link
Contributor

@T045T T045T commented Aug 4, 2014

Fixes #73

T045T added 2 commits August 4, 2014 16:17
MarkerClient will now automatically subscribe to the topic provided in the constructor, *and* <topicname>_array
@rctoris
Copy link
Contributor

rctoris commented Aug 4, 2014

Would it make more sense for this to live in its own object type? e.g., MarkerArrayClient. Currently, this will subscribe to both a marker and marker array topic on the same client, when one or the other might not even exist.

@T045T
Copy link
Contributor Author

T045T commented Aug 4, 2014

It would be simple enough to make a MarkerArrayClient, but the RViz Marker Plugin also subscribes to both the regular and the _array topic, so I think it's what users would expect.
Is there any major downside to subscribing to an unpublished topic?

@rctoris
Copy link
Contributor

rctoris commented Aug 4, 2014

untitled
Hmmmm, looks like it does both. rviz also has a MarkerArray display type. I think the usecase is if the name does not conform to *_array which is not necessary.

@T045T
Copy link
Contributor Author

T045T commented Aug 4, 2014

Good point :)

I've moved the functionality into MarkerArrayClient, leaving your name in the author tag because it's mostly copy&paste from MarkerClient, hope that's okay with you.

@rctoris
Copy link
Contributor

rctoris commented Aug 4, 2014

Looks good! 🍰

rctoris added a commit that referenced this pull request Aug 4, 2014
MarkerClient: Subscribe to MarkerArray topic
@rctoris rctoris merged commit 3958549 into RobotWebTools:develop Aug 4, 2014
@DLu
Copy link
Contributor

DLu commented Aug 4, 2014

Thanks @T045T !

that.emit('change');
});
};
ROS3D.MarkerClient.prototype.__proto__ = EventEmitter2.prototype;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this line be
ROS3D.MarkerArrayClient.prototype.__proto__ = EventEmitter2.prototype;
?

I'm currently getting
Uncaught TypeError: Cannot read property 'prototype' of undefined
on this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, of course. It's fixed now, sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MarkerClient only subscribes to Marker, not MarkerArrays
3 participants