Revision history [back]

I think this is much more complicated than it needs to be, particularly since you're not using custom callback queue(s). As I see it, the key concept to latch onto here is that all received messages go through a callback queue, each processed when a spin occurs. The primary simplifying factor here is that you only have a single callback queue (the global callback queue), with the result that a single ros::spin(), MultiThreadedSpinner::spin(), or AsyncSpinner.start() will process all queued messages as quickly as possible (that is, when a thread is available; not spinOnce).

One minor digression: it's not clear to me what dropped frames means here (in the pthreads loop). I'm guessing it means the loop sometimes runs slower than 100Hz, but it could mean you're occasionally dropping or missing message data. I think either might be the case:

1. The former makes some sense to me, as a spinOnce will block while a callback is performed; if the callback takes longer than 10ms, it'll throw off your timing. Actually, as your code is written, I'm pretty sure it will throw off you timing, since you instantiate a new ros::Rate inside the loop. If I have this right, the time for a given loop iteration will be the sum of the time to process a callback, assuming there is one, plus a 10ms sleep.
2. The latter also seems possible to me, as a callback handled by one of the AsyncSpinner threads in its entirety while blocked on the spinOnce might appear to be lost (depending on your data handling).

Pulling back out of the digression, it seems to me you might be better served by using a MultiThreadedSpinner, removing the AsyncSpinner from the Controller class, like so:

int main() {
    Controller c1("left");
    Controller c2("right");
    // start controller threads
    ros::MultiThreadedSpinner spinner(8); // 8 = 4 per controller
    spinner.spin();
}

The Controller class can retain its thread (which would need to be started prior to calling spinner.spin()), but modified like so:

pthread_run() {
    ros::Rate r(100);
    while(ros::ok()) {
        ...
        r.sleep();
    }
}

You might also want to look at (recommended) using Timers instead of ros::Rate (which utilize the callback queue -- see (5) below). Now, to your questions:

1. Yes, correct initialization (as described, maybe not as desired)
2. No, they use different threads
3. The spinOnce can be removed
4. I think item (1) in my discussion of dropped frames is the issue; if so, then yes, it's your implementation
5. Since I don't know what is in your control loops, I don't know if it makes sense to put that functionality in callbacks. But it's plausible -- for instance, it could be done with a Timer's callback method.

Honestly, though, I think just moving the ros::Rate instantiation outside the loop will solve your most important issue.