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

Ideas for ~0.1.2 #27

Open
maxired opened this issue Apr 16, 2012 · 6 comments
Open

Ideas for ~0.1.2 #27

maxired opened this issue Apr 16, 2012 · 6 comments

Comments

@maxired
Copy link
Contributor

maxired commented Apr 16, 2012

Hi @baalexander ,

Currently with 0.1.1, and using the code from the example, I can run a publisher, a subscriber and send a message from the subscriber to the publisher.

I didn't success (and I didn't see anything in the code but I could have missed it) to have a publisher with several subscribers, or lauching a subscriber then stop it, then relaunched it...

Do you have any plan in this way and more globally to make rosnodejs more flexible.
Would you accept pull request going in in that direction ?

Otherwhise,

What is your next developpement step if you have one ?

@baalexander
Copy link
Owner

Good questions.

My goal for the rest of v0.1.x is to improve publishing and subscribing robustness. This includes:

  1. Unregistering publishers and subscribers. Right now, after closing the program, you need to restart roscore. When the app finishes, an unregister call needs to be made to master. I don't know where that should go though.
  2. Stopping a node and/or stopping a topic in code. As you said, there needs to be a way to stop a publisher or subscriber. As for relaunching, I think the topic should reregister with master next time it calls subscribe or publish (lazily, like it currently does).
  3. Publish to multiple subscribers. I opened up Issue Publish to multiple subscribers #28 to address this.
  4. Get TurtleSim working. I know there was some issues with v0.0.1, I haven't had a chance to retest with v0.1.1. See Issue Cannot move my turtle :'( #20.

Issues 1 and 2 should fall under Issue #11. I proposed an API idea there, please let me know on that issue any thoughts.

Would you accept pull request going in in that direction ?

Absolutely, I would like pull requests. All I ask is if it involves some API changes, we discuss it first in the appropriate issue. We should probably also mention which issue any of us are currently working on so we can avoid working on the same part.

@maxired
Copy link
Contributor Author

maxired commented Apr 16, 2012

Thanks for you answer.

I started a work on #28, you may have a pullrequest by the end of the week.
#11 might be a bit trickier, but if I got times, I'll let you know when I'll be working on it.

@baalexander
Copy link
Owner

@maxired - I've been considering what we discussed. I'm thinking about a slight tweak to the API. Basically, instead of ros.node() and ros.topics(), you can initialize a topic directly. The only function that needs to wait then is ros.types, since you can't really do anything without the message definitions.

Example:

ros.types([
  'std_msgs/String'
], function(String) {
  var hello = new ros.topic({
    node: 'talker'
  , topic: 'hello'
  , messageType: String 
  });

  hello.on('error', function(error) {
    console.error(error);
  });

  hello.subscribe(function(message) {
    console.log(message);
  });

  var howdy = new String({data: 'Hello World!' });
  hello.publish(howdy);
});

The realization that topics(), services(), and params() are not needed came about when working on the ros.js web front here: https://github.com/rosjs/rosjs/pull/3.

I'm hoping this change not only simplifies the API some, but also makes the implementation code cleaner since there won't be a node.js class and a topic.js class, just the topic.js class now.

@maxired
Copy link
Contributor Author

maxired commented Apr 19, 2012

I'm ok with it.
I also think of a third optionnal parameter , something like

ros.types([
  'std_msgs/String'
],
 { mode:"subsribe"},
 function(String){}); 

The goal of this is set the node on the given topics as "subscriber only", "publisher only" or both of them.
It allows to start the registering of the node as subscriber/publisher without waiting for the call of the subscribe/publish.

This way, we also give more time to other nodes to connect to our node before need to send messages.
It is an objects if we need later to have more options.

What do you think about it ?

I am not ready yet to ask for a pull request, but you can watch the begin of the work on https://github.com/maxired/rosnodejs/commits/master and using the working sendingTest.js

@baalexander
Copy link
Owner

I updated the API to get rid of node() and topics(). You can now initialize a topic directly with var topic = new ros.topic(); The example/how_to.js has been updated with the updated API.

This change gets rid of the node.js class and should simplify the implementation code. Though there's still room for more simplification.

@baalexander
Copy link
Owner

With the updates in Issue #28, I made registerSubscriber and registerPublisher public functions. You can now call these directly instead of passing in a mode. Or, just call publish() or subscribe() and it will register the topic for you. Either way should work.

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

No branches or pull requests

2 participants