Hard coded timeouts in camera driver decrease maximum FPS

[claudiofantacci]

The following trigger- and grab-related timeouts are hard coded

pylon-ros-camera/pylon_camera/include/pylon_camera/internal/impl/pylon_camera_base.hpp

Lines 354 to 355 in 9f38321

	//grab_timeout_ = exposureTime().GetMax() * 1.05;
	grab_timeout_ = 500; // grab timeout = 500 ms

pylon-ros-camera/pylon_camera/include/pylon_camera/internal/impl/pylon_camera_base.hpp

Line 463 in 9f38321

int timeout = 5000; // ms

that results in the following code

pylon-ros-camera/pylon_camera/include/pylon_camera/internal/impl/pylon_camera_base.hpp

Lines 461 to 480 in 9f38321

	try
	{
	int timeout = 5000; // ms
	// WaitForFrameTriggerReady to prevent trigger signal to get lost
	// this could happen, if 2xExecuteSoftwareTrigger() is only followed by 1xgrabResult()
	// -> 2nd trigger might get lost
	if ((cam_->TriggerMode.GetValue() == TriggerModeEnums::TriggerMode_On))
	{
	if ( cam_->WaitForFrameTriggerReady(timeout, Pylon::TimeoutHandling_ThrowException) )
	{
	cam_->ExecuteSoftwareTrigger();
	}
	else
	{
	ROS_ERROR("Error WaitForFrameTriggerReady() timed out, impossible to ExecuteSoftwareTrigger()");
	return false;
	}
	}
	cam_->RetrieveResult(grab_timeout_, grab_result, Pylon::TimeoutHandling_ThrowException);
	}

to hang too much and, as a consequence, decrease the FPS of the camera.
Notice that this behavior, in conjunction with #28, provides a slow and delayed/unsynched stream of images, which is very undesirable when high frame-rate/real-time is desirable.
After some tests, setting grab_timeout_ = <framerate> and timeout = 2 * grab_timeout_ resulted in very stable performance with few image drops.

[pablo-quilez]

Hello,

you are right, we plan to do it configurable. It is a trade off between CPU performance and publishing speed which is really depending on the computer you are using. Therefore it is tricky to change it in embedded / low performance systems.

I see two possibilities to address this issue:

• Create a service to set it (preferred)
• Set as ROS parameter (through our drag&bot module yaml https://github.com/basler/pylon-ros-camera/blob/master/pylon_camera/module_config/module.yaml)

Please if you want to collaborate you are very welcome! I think it is important to have this service for setting the timeouts, what do you think?

Thanks!

[claudiofantacci]

Hi @pablo-quilez thanks for reaching out :-)

I brainstormed with colleagues and we believe that a normal node-level ROS param, appearing also in this YAML configuration file, is the preferred solution.
Otherwise, we believe a service will be the second best option.

[pablo-quilez]

You are welcome @claudiofantacci ! It is a pleasure. I am planning to address all of these FPS related topics starting on the next week but maybe you can help me to define exactly which next steps should we do. I suggest:

1. Make timeouts configurable (TBD: param / services)
2. Allow both self triggering / software triggering in the camera
3. Increase the performance of building ROS blob from camera picture in memory. The way the driver currently extracts the picture from the memory and converts to ROS may not be so optimal.

Do you think this will be enough? Am I missing something?

[claudiofantacci]

Hey @pablo-quilez I think these 3 points are well taken.

1. This is important as we discussed previously
2. This is probably the most important. We cannot achieve good framerate and stability with self-triggering and we opted for software triggering. This point, together with 1. will make a huge improvement in the driver 🚀
3. Not sure about this atm. But if there can be improvements on the memory side I think it is worth spending some effort, especially for hi-res streams.

I'll brainstorm with other people next week and be back to you with some other comments if any 😄 Thanks again!

[pablo-quilez]

Hi!

We have added the grabbing and timeout parameters both as parameters and as services. After internal tests we have merged into the main devel branch. Hopefully we can see this issue as closed!

#55

@claudiofantacci please let me know if we are missing something. I can reopen the issue if necessary. Thanks!