ros-drivers / flir_camera_driver

153 stars 145 forks source link

GigE cameras with multiple network interfaces #190

Open iag0g0mes opened 3 days ago

iag0g0mes commented 3 days ago

Problem Description:

We are facing an issue with the spinnaker_camera_driver in a setup where we have multiple network interfaces on our computer. Specifically:

Hardware Setup:

Environment:

Issue:

When using SpinView to check the cameras, the order of network interfaces is inconsistent. Sometimes the cameras are accessed via the second interface, and other times via the fourth interface. SpinView works correctly as long as the right interface is selected. However, the spinnaker_camera_driver always attempts to connect to the cameras through the first interface. If this interface is on the wrong subnet, the driver crashes with an error indicating that the cameras are in a different subnet.

Solution:

To address this issue, we modified the driver to iterate over all network interfaces. The driver now adds cameras to the camera list if it can open the camera without errors. Here is the modified code:

void SpinnakerWrapperImpl::refreshCameraList() {
  cameraList_.Clear();
  Spinnaker::InterfaceList interfaceList = system_->GetInterfaces();

  for (size_t i=0; i < interfaceList.GetSize(); i++) {
    Spinnaker::InterfacePtr iface = interfaceList.GetByIndex(i);
    Spinnaker::GenApi::INodeMap& nodeMapInterface = iface->GetTLNodeMap();
    Spinnaker::GenApi::CEnumerationPtr ptrInterfaceType = nodeMapInterface.GetNode("InterfaceType");

    if (IsAvailable(ptrInterfaceType) && IsReadable(ptrInterfaceType))
    {
        Spinnaker::GenApi::CStringPtr ptrInterfaceDisplayName = nodeMapInterface.GetNode("InterfaceDisplayName");

        if (IsAvailable(ptrInterfaceDisplayName) && IsReadable(ptrInterfaceDisplayName))
        {
          Spinnaker::GenICam::gcstring interfaceDisplayName = ptrInterfaceDisplayName->GetValue();
          Spinnaker::CameraList camList = iface->GetCameras();

          for (size_t cam_idx = 0; cam_idx < camList.GetSize(); cam_idx++){

            Spinnaker::CameraPtr ptrCam = camList.GetByIndex(cam_idx);
            try{
              ptrCam->Init();
              ptrCam->DeInit();
            }catch(Spinnaker::Exception& e){
               //erro while open the cameras in this interface
               continue;
            }//end try-catch
            //successfully open the camera in the interface
            cameraList_.Add(ptrCam);

            LOG_INFO_FMT("found cam [serial: %s] from: [%s]",
                          get_serial(ptrCam).c_str(),
                          interfaceDisplayName.c_str());
          }//end for
        } else
        {
            std::cerr << "*** Unknown Interface (Display name not readable) ***" << std::endl;
        }
    }//end for
  }//end for interfaceList
}

Question:

Is this modification acceptable? It worked for our setup, but we are unsure if there are any limitations or potential issues we might have missed.

Suggestion:

If this modification is valid, would it be possible to integrate it into the official driver? This would allow us to update our project without conflicts whenever the repository receives updates.

berndpfrommer commented 3 days ago

Nice catch! We definitely want that in the main line driver. The usual process is that you would submit a Pull Request and I go over it, suggest modifications, eventually it gets approved and will be merged in. If that is too much work for you, I'll copy-and-paste your code and fold it in manually (at which point I will format the code correctly and add a few more modifications). The downside is that you will not show up in the commit history, depriving you of the recognition you deserve. Let me know which way to go. A couple of changes will be necessary: 1) Code formatting is off (use clang-format, then run colcon test spinnaker_camera_driver to check for correct formatting) 2) The "Enumeration.cpp" example in the Spinnaker SDK has an explicit "interfaceList.clear()". Not sure if it's necessary, but I'd put it in. 3) replace "std::cerr <<" with LOG_ERROR() 4) the catch clause of the exception: what is the exact exception type it's throwing? Is it a derived class of spinnaker::exception?

That's all I see in the first pass. I'd also like to test the driver with my USB3 cameras to make sure they still work as before.

iag0g0mes commented 3 days ago

Hello,

We can work on a pull request for the network interface changes.

However, we have made additional modifications to the launch file to search for camera_info files within the package and assign a name and namespace to each camera. To keep everything related to the cameras within the package, we created two new folders:

Should we copy the network interface changes to a different clone and proceed with the pull request?

Thank you for your assistance!

berndpfrommer commented 2 days ago

Yeah, make a new clone, branch off a new branch from humble-devel, and copy just the modified c++ file over. I usually keep launch and config files in a separate package that is related to the project, not the cameras.