rosnodejs icon indicating copy to clipboard operation
rosnodejs copied to clipboard

Ideas for ~0.1.2

Open maxired opened this issue 12 years ago • 6 comments

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 ?

maxired avatar Apr 16 '12 16:04 maxired

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 #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 #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.

baalexander avatar Apr 16 '12 17:04 baalexander

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.

maxired avatar Apr 16 '12 21:04 maxired

@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.

baalexander avatar Apr 19 '12 21:04 baalexander

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

maxired avatar Apr 19 '12 22:04 maxired

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 avatar Apr 20 '12 04:04 baalexander

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.

baalexander avatar May 16 '12 03:05 baalexander