On Sat, Feb 4, 2012 at 5:54 PM, Jack O'Quin wrote: > Your patch is a big help. I have no previous experience with those interfaces. > > The patch creates a separate thread to run the diagnostics updater, > but only for the nodelet implementation. The camera1394_node does not > create this thread or call the updater. I presume that is just a minor > oversight due to the slightly strange camera1394 source file structure > that has evolved. Yeah, I was not too sure where I should put the initialization. > Multi-threading is tricky in this driver, see the comments in > Camera1394Driver::poll(). I am wondering if we could just call the > updater there in the poll() thread, instead of creating another > thread. Is there any large overhead in calling the updater? The updater is publishing in /diagnostics at a fixed rate so no it is probably not that time consumming. However, it is still a shame to slow down the whole node. > Also, I am still a little confused about how the diagnostics updater > learns the actual frame rate of the camera. Shouldn't there be a > common data structure update? Wouldn't that require a mutex lock? The patch is missing a line: topic_diagnostics_.tick() should be called each time a message is published. This function is thead safe so we can safely publish diagnostics in one thread and tick in the main thread. -- Thomas Moulard