-
Notifications
You must be signed in to change notification settings - Fork 8
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
Push to multiple subscriber #30
Conversation
Thanks @maxired. I'll try taking a more detailed look tonight or Wednesday night and merge the changes in. It's failing an automatic merge right now, but I assume it's because I moved the function order around in topic.js in master. |
I merged the changes plus a few more into the multiple clients branch. I need to make some changes before merging into master, including:
I'm hoping to have time to work on this Saturday or Sunday. |
Hi, What's your plan about splitting protocols ?
About the third point, I'm not sure to understand. |
I don't have a final plan on splitting up protocols yet. I was thinking about splitting up Topic's this.protocols['TCPROS'] to have a publishers array and a subscribers array. As for the third point, I think I can make it work so no messages would be lost on publish. My idea shouldn't remove your option, though, of being able to register as a publisher or subscriber before calling publish() or subscribe(). Both of my points are a little vague because I still need to play around with the code and investigate more. |
Thanks for this information. After I re-read the code, I've seen that in TCPROS, about the we still use this.socket in the subscribing parts. That's mean that also inside TCPROS we have to split the sockets between publishers and subscribers. Moreover, I'm not sure if we should firstly make some choice for the design, or begin to implements Services |
I realized where a source of some of our confusion with the multi client stuff was and tried to fix it with the latest commits to the multiple_clients branch. Basically, instead of having some multi logic in Topic and some multi logic in TCPROS, I put all the multi client handling logic in Topic. Each instance of TCPROS is meant to be a publisher to a single socket or a subscriber to a single socket. Topic will create instances of TCPROS as needed for each additional subscriber or publisher. The code still needs work, but it should give an idea of what's happening. |
As for services, I definitely agree this needs to get done. Issue #7 is dedicated to services implementation. There needs to be more work in defining the services API, but things like parsing the srv file could be done sooner. I left comments in the issue. |
Hi agree with you that with your last work , everything seems simpler. What do you think about that ? |
Hi,
This is not perfect, but it is a good start to fix #28
I identified an issue when publishing to multiples subscribers : we start a new connection to all nodes in the publisherUpdate message, even those to which we are already connected.
Otherwise, everything seems to work.
I will propably improve it, but I was thinking that you might want this version.