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

v0.1.x ideas #19

Closed
baalexander opened this issue Feb 27, 2012 · 18 comments
Closed

v0.1.x ideas #19

baalexander opened this issue Feb 27, 2012 · 18 comments

Comments

@baalexander
Copy link
Owner

After listening to a few persons feedback, seeing how people are using it, and dealing with a few of the bugs that have risen, I think there's some architectural shifts that will make rosnodejs more useful and stable going forward.

  1. Do not share code between the browser and rosnodejs. I think Backbone still makes sense for the browser, but in practice, a competing event system like Backbone's in rosnodejs just makes things worse. The may end up with ros.js and others being forked into a roswebjs package. I still plan for the APIs to be very similar.

  2. Make publisher and subscriber more event based. Think HTTP.server or net.Socket. On the browser, this would mean Backbone models wrapped around socket.io events. Not sure for the server yet.

  3. Decouple TCPROS. While the current implementation of TCPROS work, I would like to see improvements all around. I think modeling it after net.Socket could be a good start. More event based should help with the decoupling from publisher and subscriber.

  4. No more message pre-generation. As mentioned in https://github.com/baalexander/rosmsgjs/issues/6, the generation of message files may not be needed. The server could generate the models on load time and the browser could populate a generic Ros.Message with JSON.

I'm aiming for most of the ideas and any others to be included in the first release meant for wider consumption: v0.1.0. Please let me know if any ideas. The work is being done on the public branch 0.1.0-wip.

@trevorjay
Copy link

So here are my thoughts on the architecture. Definitely don't mistake their (possibly) opinionated nature for being prescriptive or even necessarily well though out. :)

Foremost, I think getting rid of backbone.js would be a good idea. backbone.js's default use case of transparently handling networking so that an object appears to be co-located between client and server is just a poor fit. ROS simply has no inherent concept of object updates, the showcase feature of backbone.js. ROS is predicated on all communication being in the form of new objects. I agree that you need to move more to an event (I would go further and say a stream) model. backbone.js is a poor fit for this.

I definitely agree with having different library and client versions of libraries. I just finished rosbridge's answer to server-side use and this comes in the form of JavaScript library that has an identical interface to our web library but with an alternate implementation. With our testing so far, this works really well. For users, identical code still runs on the client and server.

I hate message pre-generation. Having something resembling a build step reminds me too much of normal ROS development. Avoiding a build cycle is one of the reasons users would want a JavaScript approach. I'm a big fan of runtime ROS -> JSON -> ROS conversion, but I understand not everyone is willing to bite that bullet. The proposed roslua like approach isn't the compromise I'd draw, but it is definitely better than up-front generation. For one, it would mean that you would only have to handle types that actually get used instead of all types in a package.

I'm torn about TCPROS and not just because your implementation is such an impressive coding achievement. One one hand: having your own implementation of TCPROS means you're going to run into the same "tracking what Willow does" problem that the rosserial project has. On the other hand: if you do something like link rosnodejs to the ROS C++ runtime it will create a dependency hell. Right now, I can deal exclusively with npm (and git) when doing rosnodejs work. A dependency to TCPROS will mean that I have to deal with the ROS and node package systems simultaneously. Not only does that inherently suck, avoiding the ROS package system is---again---one of the motivations a user would have for wanting to use a framework like rosnodejs in the first place. I'd take a hedging attitude. Maybe make your other proposed changes and see how TCPROS works in the new context?

Anyway, those are just some of my off-the-cuff thoughts. I'd love to have a higher bandwidth discussion if you're up for a Skype or Google Hangout session.

@baalexander
Copy link
Owner Author

I'll have a much better response later to your other points. I definitely agree a Google Hangout discussion would be useful. Maybe tomorrow evening if you're available (I want to try some more ideas out tonight)? I just wanted to clarify one thing real quick:

I meant only refactoring the TCPROS code to clean it up, but definitely still keep it. I just think the relation between publisher and subscriber and TCPROS is too coupled.

Really, I have no intentions of introducing any more dependencies on the ROS framework. If anything, with message pre-generation going away, I'm hoping rosnodejs can get away from any direct ROS calls or dependencies.

@trevorjay
Copy link

I meant only refactoring the TCPROS code to clean it up, but definitely still keep it. I just think the relation between publisher and subscriber and TCPROS is too coupled.

Sorry for the misunderstanding. And I'm definitely leaning towards this being the right call. The tracking Willow issue isn't as bad for you as it is for rosserial because they have to do so on restricted environments like the Arduino. Also, JavaScript implementations are, in general, more agile than a C implementation.

If anything, with message pre-generation going away, I'm hoping rosnodejs can get away from any direct ROS calls or dependencies.

This would be excellent.

And I'm definitely up for a Hangout tomorrow evening. Let me know about your schedule as it become clearer.

@temsa
Copy link
Contributor

temsa commented Feb 28, 2012

Generally agreed with this move, and really pleased to see where it goes :)

I'd like to help, but right now I'm stuck on not being able to move my turtle_sim with a very simple program in rosnode, maybe I don't understand everything, I've just done the ros tutorial before some weeks ago.

I'll try to make the message loadtime "generation" in the next 2 days (I have some spare time, yet I don't know if I'll be able to reach my goal)

@temsa
Copy link
Contributor

temsa commented Feb 28, 2012

made an api improvement proposal in #21 you may be interested in

@baalexander
Copy link
Owner Author

Thank you @trevorjay and @temsa for the useful comments. I've spent the past couple days thinking about what y'all said and looking at other streaming, event-based frameworks for ideas. What's your thoughts on an API like this (inspired by Issue #21, socket.io, and a hint of rospy):

var ros   = require('rosnodejs'),
    // If no callback is passed as a second argument, then run it in synchronous mode
    Twist = ros.message('geometry_msgs/Twist')

var topic = {
  topic: 'cmd_vel'
, message: Twist
  // Protocol is optional and defaults to TCPROS. Affects the socket returned.
, protocol: 'TCPROS'
}

var node = ros.node('talker')

var publisher = node.publisher(topic, function(socket) {
  socket.on('error', function(error) {
  })

  var message = new Twist({ linear: { x: 1.0 }})
  // Can publish a message using:
  socket.emit('message', message)
  // or
  socket.publish(message)
})

var subscriber = node.subscriber(topic, function(socket) {
  socket.on('error', function(error) {
  })

  // Can subscribe to a topic using:
  socket.on('message', function(message) {
  })
  // or
  socket.subscribe(function(message) {
  })
})

What I'm really liking about this approach is that the heart of the framework and code becomes the socket.

The socket emits events for things like error, message, and perhaps connections and others. Right now, I'm thinking the socket is more like an interface to TCPROS.js or UDPROS.js. So you interact with the socket using .on() and .emit() like normal, and TCPROS.js or UDPROS.js takes care of all the protocol implementation for you.

The reason for the publish() and subscribe() is to make the XML-RPC calls to master inorder to register the node as a publisher or subscriber. That way any socket calls to emit() or on() don't have to worry about XML-RPC registration and can simply focus on TCPROS or UDPROS.

The syntax should also be nearly identical for web or server. With web using socket.io and server using TCPROS.js or UDPROS.js. The one part I'm still unsure of is the message generation in the browser.

I'm looking forward to y'alls thoughts.

@temsa
Copy link
Contributor

temsa commented Feb 29, 2012

For some projects, I personally use hook.io, which is a very simple message-bus with multiple transports, and I'm pretty happy with it's api. Ros could have an api pretty close to the hook.io one.

You may want to have a look at it :)

Concerning your proposal, I still find it pretty verbose, but I like the really near to socket idea too. More details on my thoughs on #21

P.S. You may consider using EventEmitter2 which brings an improved api (like ".many"), more performances, namespaces and wildcards, which may suit a lot ROS; considering a '/' separator for namespaces, you could register to all messages from a topic using something like node.on("cmd_vel/*", function(message) { ... }) or for a specific message node.on("cmd_vel/twist", function(message) { ... }) , so it can bring type inference back in this pretty typed message bus.

P.P.S: you may want to have a look too to broadway to able plugin interaction, so you can create a high level api, and provide an easy way to have more control on it fot people interested in particular cases and hackability

@baalexander
Copy link
Owner Author

I'm also liking EventEmitter2 as the main Event library for Node.js and browser support.

As a compromise between emitting events on subscriber()/publisher() and emitting event on node(), I'm leaning towards something like this:

var params = { topic: 'cmd_vel' , message: Twist }

var node = ros.node('talker')
var topic = node.topic(params, function(socket) {
  socket.on('error', function(error) {})

  var message = new Twist({ linear: { x: 1.0 }})
  socket.emit('message', message)

  socket.on('message', function(message) { })
})

The idea is a socket is now a topic socket. Similar to your API proposal @temsa in that you can publish or subscribe on the socket without caring if it's a publisher or subscriber, but the socket is still namespaced under the topic.

I'm going to start implementing the new API over the weekend. I think we're on a similar enough page with the socket and event approach that future updates won't be as drastic.

@temsa
Copy link
Contributor

temsa commented Mar 1, 2012

This proposal looks good to me, pretty low level, still practical, and it could be easily improved to create a fluid api :

ros
  .node('talker')
  .topic('cmd_vel',  Twist, function(socket) {
    socket.on('error', console.error)
    socket.on('message', function(message) { console.log (message) })
    socket.emit('message', Twist({ linear: { x: 1.0 }})
  })
  .topic({ topic: 'hello' , message: World,  , protocol: 'TCPROS' }, function (socket) {
     socket.on('error', console.error)
     socket.on('message', console.log)
  })

".topic()" returns the node rather than the topic, so that we can chain topics definition, or could return the actual actual topic, but also provide the node api as well, so we can still chain calls(but if so we should avoid namespace conflict between node and topic, or just take what is the most relevant considering the context).

Note: my generic policy about function arguments : "up to 3-4 parameters top, use inline arguments, more needed ? Then an object is better. If mandatory arguments are 2 or less, support both inline and object if possible." (here we can, by testing the type of first and second argument), we can support both, as written above :)

I guess "this" can be maintained to the node itself in every callback, so you can call 'this.stop()" to stop the node (may be interesting for #11

What I may do as a third party library on top of this (if you don't want to allow it as a first class citizen), is add such an api on top of it :

ros
  .node('talker')
  .topic('cmd_vel',  Twist)
  .topic({ topic: 'hello' , message: World, protocol: 'TCPROS' }})

  .on ('error/**', console.error) //handles any error that may happened in the node, the topics, etc.

  .on ('error/topic/*', console.warn) //handles any error that may happened in any topic

  .on('topic/cmd_vel/twist', function onTwist (twist) {
    this.emit('topic/hello/world', World({hello:"world"}))
  })

  .on('topic/hello/*', function(message) { console.log('hello,', message) /*message can be an instance of World, or any Type we have registered as a topic*/ })

  .on('topic/**', function(message) {console.debug('yes, I can easily debug by logging any message emitted and received in my node thanks to this event hierarchy:', message)}

It would not replace the original API, just extends it. The only thing I'd need in order not to monkey patch rosnodejs would to be able to provide a function that can wrap any callback provided in the topic registration (something like connect middlewares ?), so I can make this callback optional, and handle everything through EventEmitter2 on the node itself (so i can relay errors, etc.).
Maybe I could also improve it a bit more to add some magic for registering topic independently from message type too, could be great :)

@baalexander
Copy link
Owner Author

I am cool with topic() returning a reference to node for chaining. Though I'm interested from ROS users if using multiple topics on a node is common enough for chaining, or is the encouraged way to do multiple nodes.

I'll probably program the first draft to take an object param. But variable function arguments would be minor to add shortly after.

After I get out an initial version, I'd like to discuss with you more on providing a way to easily wrap the callbacks for your API.

@baalexander
Copy link
Owner Author

The API below is based on inputs from this thread and the folks from the @Rosjs organization.

ros.types([
  'geometry_msgs/Twist'
, 'people_tracker/ImagePosition'
], function(Twist, Pos) {

  var talker = ros.node('talker')

  // Topic() can take a single topic and return a single socket. Or take an
  // array of topics and return sockets for each. topics() is also chainable.
  talker.topics([
    {topic: '/cmd_vel', type: Twist}
  , {topic: '/person', type: Pos }
  ], function(cmd_vel, person) {
    person.on('error', function(error) {
      console.log('There was an error');
    });

    person.subscribe(function(msg) {
      cmd_vel.publish(twist);
    });
  });

  // Service() can take one or many services and return a socket for each. The
  // callback is only called when the services are available (think:
  // rospy.wait_for_service(), but asynchronous)
  ros.services([
    { service: 'math_srv/add_two_ints' }
  , { service: 'math_srv/multiply_by_3' }
  ], function(addTwoInts, multiplyByThree) {
    // AddTwoInts is a Service 'socket', similar to the Topic sockets above.
    // Events could include 'error', 'disconnect', etc.
    addTwoInts.on('error', function(error) {

    });
    // Calls the service. Need another name than call() since that's already a
    // function prototype named 'call' in JavaScript.
    addTwoInts.call(2, 3, function(error, results) {
      // Prints 5
      console.log(results.sum);
    });
  });

  // Similar to topic() and service(), creates a 'socket' for each param.
  // If the param exists, gets the current value for the param and subscribes to
  // updates. If the param does not exists, waits for a .set() then subscribes
  // to changes.
  ros.params([
    'escape_vel'
  , 'max_vel_x'
  ], function(escapeVel, maxVelX) {
    // Not sure if this should be synchronous (assuming stores latest value) or
    // not.
    var maxX = maxVelX.get();

    // Update event is emitted when the param has been updated by the server.
    escapeVel.on('update', function(newValue) {
      // Update some HTML
    });
    escapeVel.set(0.9);
  });

});

A few changes from earlier:

  • Adds support for services and params.
  • Most functions that return a socket object (like topic sockets, service sockets, or param sockets) can now take an array of arguments. This should minimize nested callbacks.
  • Multiple message types can be fetched from ros.types (or maybe rename to ros.messages()).

@baalexander
Copy link
Owner Author

One last update:

The rosjs organization was formed on GitHub recently. Rosjs is bringing together groups that have been working on web JavaScript and ROS, like Brown University and Bosch. The focus of the organization is on JavaScript in the browser and ROS.

However, we are trying to move to this standardized API for rosnodejs and core rosjs so JavaScript written for one platform should work well for the other. The web implementation (ros.js) will be available at the rosjs repo.

baalexander added a commit that referenced this issue Mar 28, 2012
baalexander added a commit that referenced this issue Mar 28, 2012
@baalexander
Copy link
Owner Author

Updates on v0.1.0 progress

Publishing works now. how_to.js has an example. I've tried with messages containing primitive types, arrays, and more messages. You can run the example by running:

  1. roscore
  2. rostopic echo /hello_world
  3. ./node_modules/example/how_to.js --timeout 5000 example/how_to.js.

There's no unregistering of publishers though, so you'll want to restart roscore and rostopic each run.

I'm currently working on subscribing. After subscribing works, I'll merge 0.1.0-wip into master. After 0.1.0-wip is merged into master, I'll also try to clean up the tickets since some may be outdated or be handled outside of core rosnodejs.

@temsa
Copy link
Contributor

temsa commented Apr 4, 2012

Great job :-)
Any help needed ?
Le 4 avr. 2012 05:23, "Brandon Alexander" <
reply@reply.github.com>
a crit :

Updates on v0.1.0 progress

Publishing works now. how_to.js
has an example. I've tried with messages containing primitive types,
arrays, and more messages. You can run the example by running:

  1. roscore
  2. rostopic echo /hello_world
  3. ./node_modules/example/how_to.js --timeout 5000 example/how_to.js.

There's no unregistering of publishers though, so you'll want to restart
roscore and rostopic each run.

I'm currently working on subscribing. After subscribing works, I'll merge
0.1.0-wip into master. After 0.1.0-wip is merged into master, I'll also try
to clean up the tickets since some may be outdated or be handled outside of
core rosnodejs.


Reply to this email directly or view it on GitHub:
#19 (comment)

@baalexander
Copy link
Owner Author

Thanks @temsa.

A bug that I haven't had a chance to look into yet, that you or @maxired may be more familiar with, is Issue #24. The MD5 sum isn't matching up with rosmsg md5 <Message Type>.

@maxired
Copy link
Contributor

maxired commented Apr 4, 2012

@baalexander , did you mean "mocha example/how_to.js --timeout 5000 example/how_to.js" ?

I couldn't run with "./node_modules/example/how_to.js --timeout 5000 example/how_to.js"

@baalexander
Copy link
Owner Author

Oh, yes, it was a typo: ./node_modules/.bin/mocha --timeout 5000 example/how_to.js. The --timeout 5000 is because mocha errors on tests that take longer than 2 seconds unless told otherwise.

@baalexander
Copy link
Owner Author

Master has been updated to version 0.1.x and implements the discussed API.

I will be closing all related API issues as most originated from the old 0.0.x versions.

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

4 participants