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

Publish to multiple subscribers #28

Closed
baalexander opened this issue Apr 16, 2012 · 8 comments · Fixed by #30
Closed

Publish to multiple subscribers #28

baalexander opened this issue Apr 16, 2012 · 8 comments · Fixed by #30

Comments

@baalexander
Copy link
Owner

Need to investigate if a publisher can publish to more than one subscriber.

A subscriber sends a requestTopic call to the publisher, which it does handle correctly for one subscriber. But if multiple subscribers call, does it keep track of each? Topic may need to have an array of protocols instead of a single protocol.

@baalexander
Copy link
Owner Author

Thanks for taking a look into this @maxired.

If you see a better way for communicating between node.js, topic.js, and tcpros.js while working on the issue, please let me know. I think it's a tad confusing right now.

@baalexander
Copy link
Owner Author

I included your commits in my API update to master. I did this because otherwise you would have merge conflicts since I got rid of node.js and changed up topic.js.

That said, I had to comment out most your multiple socket code because it wasn't working for me. I kept most of it in there, just commented out. Sorry for the change midway during your change, but hopefully my commit made the code a little easier to manage and work with.

@maxired
Copy link
Contributor

maxired commented Apr 26, 2012

Hi,

It appears that I can't fix this without changing the way the publish method works. (publish only to currently connected subscribers)

https://github.com/baalexander/rosnodejs/commit/6839c1dfcd5b690a4d813ea96b120946da8250f3#L5L28#L5L32 seems to show that you don't really want my proposal, so I don't know exactly what to do.

@baalexander
Copy link
Owner Author

@maxired - You can change the implementation of publish().

However, I'd still prefer being able to call topic.publish() directly like so:

var publisher = new ros.topic({
  node        : 'talker'
, topic       : 'publish_example'
, messageType : String
});

var message = new String({ data: 'howdy' });
publisher.publish(message);

Will that work? Or does calling publish() directly like mentioned above create a problem?

@maxired
Copy link
Contributor

maxired commented Apr 26, 2012

@baalexander : It will be easy if you are Okay with the fact that only currently connected nodes will receive the message.

@baalexander
Copy link
Owner Author

That works for me for me for now. I'll have better thoughts on supporting unconnected nodes after your pull request.

@baalexander
Copy link
Owner Author

I merged the fixes for this into master. Please let me know if it didn't work in your testing.

I'll update the npm version to 0.1.2 tomorrow if everything looks good.

@maxired
Copy link
Contributor

maxired commented May 16, 2012

Hi, I'll try today and let you know

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 a pull request may close this issue.

2 participants