Re: [ros-users] Announcing rosserial

Am 18.07.2011 22:59, schrieb Adam Stambler:
> It does create a new transport called "rosserial" for point to point
> serial connections. The goal was to provide a light weight packet
> where that could handle syncing the two end points and multiplexing the
> topic communication. It also has a checksum to for error detection.
> However, the error detection is not used to guarantee is packet
> received, but rather to guarantee that an endpoint does not act on a
> corrupted packet. It is an unreliable transport.
>
> All of this is detailed on the rosserial/Overview
> <http://www.ros.org/wiki/rosserial/Overview> page. We are planning to
> do an API review of both rosserial the transport and rosserial_arduino
> and would love some outside input. If there is anything you don't like
> or think could be improved, let us know.

Hi,

I've converted my avr_bridge project to rosserial, and I'm quite happy
with it. The transition was smooth and it works really well (ignoring a
couple of issues for which I'll be creating tickets in your trac).

Attached are a couple of patches I'd ask for inclusion into rosserial:

- allow custom baudrates
- simplify Subscriber/Publisher constructors
- make strings signed char*, instead of unsigned char*

cheers
Dariush
--
Institut für Technische Informatik / Universität zu Lübeck
http://www.iti.uni-luebeck.de/index.php?id=forouher0
# HG changeset patch
# User Dariush Forouher <forouher@iti.uni-luebeck.de>
# Date 1311859396 -7200
# Node ID c039523d65d88159cea23c023f88a575bf840915
# Parent 9ebe4e182a2d66da8a117872f0ea6bb698bcb9b2
rosserial_arduino: allow custom baudrates

diff -r 9ebe4e182a2d -r c039523d65d8 rosserial_arduino/src/ros_lib/ros.h
--- a/rosserial_arduino/src/ros_lib/ros.h    Wed Jul 13 17:29:39 2011 -0700
+++ b/rosserial_arduino/src/ros_lib/ros.h    Thu Jul 28 15:23:16 2011 +0200
@@ -109,6 +109,7 @@
       void syncTime(unsigned char * data);

       /* Start serial, initialize buffers */
+      void initNode(long baud);
       void initNode();

       /* This function goes in your loop() function, it handles 
diff -r 9ebe4e182a2d -r c039523d65d8 rosserial_arduino/src/ros_lib/ros_lib.cpp
--- a/rosserial_arduino/src/ros_lib/ros_lib.cpp    Wed Jul 13 17:29:39 2011 -0700
+++ b/rosserial_arduino/src/ros_lib/ros_lib.cpp    Thu Jul 28 15:23:16 2011 +0200
@@ -195,10 +195,10 @@
 }

-void ros::NodeHandle::initNode()
+void ros::NodeHandle::initNode(long baud)
{
// initialize serial
- fx_open();
+ fx_open(baud);
configured_ = false;
mode_ = 0;
bytes_ = 0;
@@ -206,6 +206,11 @@
topic_ = 0;
}

+void ros::NodeHandle::initNode()
+{
+  initNode(57600);
+}
+
 void ros::NodeHandle::spinOnce()
 {
   /* restart if timed-out */
diff -r 9ebe4e182a2d -r c039523d65d8 rosserial_arduino/src/ros_lib/serial_fx.cpp
--- a/rosserial_arduino/src/ros_lib/serial_fx.cpp    Wed Jul 13 17:29:39 2011 -0700
+++ b/rosserial_arduino/src/ros_lib/serial_fx.cpp    Thu Jul 28 15:23:16 2011 +0200
@@ -4,8 +4,8 @@

#include "serial_fx.h"

-void fx_open(){
-    Serial.begin(57600);
+void fx_open(long baud){
+    Serial.begin(baud);
 }

diff -r 9ebe4e182a2d -r c039523d65d8 rosserial_arduino/src/ros_lib/serial_fx.h
--- a/rosserial_arduino/src/ros_lib/serial_fx.h    Wed Jul 13 17:29:39 2011 -0700
+++ b/rosserial_arduino/src/ros_lib/serial_fx.h    Thu Jul 28 15:23:16 2011 +0200
@@ -7,7 +7,7 @@

#include "WProgram.h"

-void fx_open();
+void fx_open(long baud);

int fx_putc(char c);

# HG changeset patch
# User Dariush Forouher <forouher@iti.uni-luebeck.de>
# Date 1311862580 -7200
# Node ID a5fa361540218ee9e867dee5a1f23d3ce8e50b7a
# Parent c039523d65d88159cea23c023f88a575bf840915
Constructors of Publisher and Subscriber don't need full msg, just type

Just store reference to the msg type, not a full msg. Passing a full message
to the constructor of Pub. and Subscr. ist confusing to the unaided eye, as
it leaves one to wonder for what it is used, exactly.

Also make Msg::getType() static, so one can create a Publisher without having to
constuct a message, e.g.:

pub_diagnostics = ros::Publisher("/diagnostics", diagnostic_msgs::DiagnosticStatus::getType());

diff -r c039523d65d8 -r a5fa36154021 rosserial_arduino/scripts/make_library.py
--- a/rosserial_arduino/scripts/make_library.py    Thu Jul 28 15:23:16 2011 +0200
+++ b/rosserial_arduino/scripts/make_library.py    Thu Jul 28 16:16:20 2011 +0200
@@ -431,7 +431,7 @@
         f.write('     return offset;\n');
         f.write('    }\n')         
         f.write('\n')
-        f.write('    const char * getType(){ return "%s/%s"; };\n'%(self.package, self.name))
+        f.write('    static const char * getType(){ return "%s/%s"; };\n'%(self.package, self.name))
         f.write('\n')
         f.write('  };\n')
         f.write('\n')
diff -r c039523d65d8 -r a5fa36154021 rosserial_arduino/src/ros_lib/ros.h
--- a/rosserial_arduino/src/ros_lib/ros.h    Thu Jul 28 15:23:16 2011 +0200
+++ b/rosserial_arduino/src/ros_lib/ros.h    Thu Jul 28 16:16:20 2011 +0200
@@ -63,7 +63,7 @@
     public: 
       virtual int serialize(unsigned char *outbuffer) = 0;
       virtual int deserialize(unsigned char *data) = 0;
-      virtual const char * getType() = 0;
+      static const char * getType();

};

@@ -73,25 +73,25 @@
   class Publisher
   {
     public:
-      Publisher( const char * topic_name, Msg * msg );
+      Publisher( const char * topic_name, const char * type );
       int publish( Msg * msg );

       int id_;     
       NodeHandle * nh_;
       const char * topic_;
-      Msg * msg_;
+      const char * type_;
   };

   /* Generic Subscriber */
   class Subscriber
   {
     public:
-      Subscriber( const char * topic_name, Msg * msg, msgCb * callback);
+      Subscriber( const char * topic_name, const char * type, msgCb * callback);

       int id_;     
       NodeHandle * nh_;
       const char * topic_;
-      Msg * msg_;
+      const char * type_;
       msgCb * cb_;
   };

diff -r c039523d65d8 -r a5fa36154021 rosserial_arduino/src/ros_lib/ros_lib.cpp
--- a/rosserial_arduino/src/ros_lib/ros_lib.cpp    Thu Jul 28 15:23:16 2011 +0200
+++ b/rosserial_arduino/src/ros_lib/ros_lib.cpp    Thu Jul 28 16:16:20 2011 +0200
@@ -56,11 +56,11 @@
 /* 
  * Publishers 
  */
-ros::Publisher::Publisher(const char * topic_name, Msg * msg )
+ros::Publisher::Publisher(const char * topic_name, const char * type )
 {
   id_ = 0;
   topic_ = topic_name;
-  msg_ = msg;
+  type_ = type;
 }

int ros::Publisher::publish(Msg * msg)
@@ -74,11 +74,11 @@
/*
* Subscribers
*/
-ros::Subscriber::Subscriber(const char * topic_name, Msg * msg, msgCb *callback)
+ros::Subscriber::Subscriber(const char * topic_name, const char * type, msgCb *callback)
{
id_ = 0;
topic_ = topic_name;
- msg_ = msg;
+ type_ = type;
cb_ = callback;
}

@@ -128,7 +128,7 @@
     {
       ti.topic_id = publishers[i]->id_;
       ti.topic_name = (unsigned char *) publishers[i]->topic_;
-      ti.message_type = (unsigned char *) publishers[i]->msg_->getType();
+      ti.message_type = (unsigned char *) publishers[i]->type_;
       publish( TOPIC_PUBLISHERS, &ti );
     }
   }
@@ -138,7 +138,7 @@
     {
       ti.topic_id = subscribers[i]->id_;
       ti.topic_name = (unsigned char *) subscribers[i]->topic_;
-      ti.message_type = (unsigned char *) subscribers[i]->msg_->getType();
+      ti.message_type = (unsigned char *) subscribers[i]->type_;
       publish( TOPIC_SUBSCRIBERS, &ti );
     }
   }

# HG changeset patch
# User Dariush Forouher <forouher@iti.uni-luebeck.de>
# Date 1311865413 -7200
# Node ID 07ce392971af47126c8d7e9e5d2fca0b74212054
# Parent a5fa361540218ee9e867dee5a1f23d3ce8e50b7a
strings in C are typically signed char. It makes things
more convenient if rosserial also treats them as signed.

diff -r a5fa36154021 -r 07ce392971af rosserial_arduino/scripts/make_library.py
--- a/rosserial_arduino/scripts/make_library.py    Thu Jul 28 16:16:20 2011 +0200
+++ b/rosserial_arduino/scripts/make_library.py    Thu Jul 28 17:03:33 2011 +0200
@@ -187,10 +187,10 @@

 class StringDataType(PrimitiveDataType):
-    """ Need to convert to unsigned char *. """
+    """ Need to convert to signed char *. """

     def make_declaration(self, f):
-        f.write('      unsigned char * %s;\n' % self.name)
+        f.write('      char * %s;\n' % self.name)

     def serialize(self, f):
         cn = self.name.replace("[","").replace("]","")
@@ -204,7 +204,7 @@
         cn = self.name.replace("[","").replace("]","")
         f.write('      long * length_%s = (long *)(inbuffer + offset);\n' % cn)
         f.write('      offset += 4;\n')
-        f.write('      this->%s = (inbuffer + offset);\n' % self.name)
+        f.write('      this->%s = (char *)(inbuffer + offset);\n' % self.name)
         f.write('      offset += *length_%s;\n' % cn)

diff -r a5fa36154021 -r 07ce392971af rosserial_arduino/src/ros_lib/ros_lib.cpp
--- a/rosserial_arduino/src/ros_lib/ros_lib.cpp    Thu Jul 28 16:16:20 2011 +0200
+++ b/rosserial_arduino/src/ros_lib/ros_lib.cpp    Thu Jul 28 17:03:33 2011 +0200
@@ -127,8 +127,8 @@
     if(publishers[i] != 0) // non-empty slot
     {
       ti.topic_id = publishers[i]->id_;
-      ti.topic_name = (unsigned char *) publishers[i]->topic_;
-      ti.message_type = (unsigned char *) publishers[i]->type_;
+      ti.topic_name = (char *) publishers[i]->topic_;
+      ti.message_type = (char *) publishers[i]->type_;
       publish( TOPIC_PUBLISHERS, &ti );
     }
   }
@@ -137,8 +137,8 @@
     if(subscribers[i] != 0) // non-empty slot
     {
       ti.topic_id = subscribers[i]->id_;
-      ti.topic_name = (unsigned char *) subscribers[i]->topic_;
-      ti.message_type = (unsigned char *) subscribers[i]->type_;
+      ti.topic_name = (char *) subscribers[i]->topic_;
+      ti.message_type = (char *) subscribers[i]->type_;
       publish( TOPIC_SUBSCRIBERS, &ti );
     }
   }