[ros-users] [PATCH][camera1394] Adding diagnostics support to camera1394

[Jack O'Quin]

On Sat, Feb 4, 2012 at 5:04 PM, Thomas Moulard <thomas.moulard at gmail.com> wrote:
> On Sat, Feb 4, 2012 at 5:54 PM, Jack O'Quin <jack.oquin at gmail.com> wrote:

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

It's not a matter of slowing down the node. I tried to explain the
problem in this comment in the poll() method:

    // The mutex lock should be sufficient, but the Linux pthreads
    // implementation does not guarantee fairness, and the reconfig()
    // callback thread generally suffers from lock starvation for many
    // seconds before getting to run.  So, we avoid acquiring the lock
    // if there is a reconfig() pending.

I worry that introducing another thread will make things worse. I can
experiment with different approaches, but it's hard to believe that
our test coverage is adequate for those kinds of problems.

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

I see.

It looks like FrequencyStatusParam() is a wrapper for the general
DiagnosticStatus message.

I wonder what the right tolerance is for general 1394 camera frame
rates? The frame rate is unlikely to run fast, but I have noticed that
cameras tend to slow down in low light situations due to the need for
longer exposure times. For various applications a slower rate may or
may not be acceptable. We may want to introduce a user parameter to
set the tolerance interval.

Also, it will be tricky to determine the frame rate when running
Format 7 modes. Unfortunately, the configured frame_rate has no effect
in that situation. That is not the fault of the driver, but of the
IIDC standard.
-- 
 joq