Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
Community Bot
Community Bot
  • 1
added 1950 characters in body
Source Link
Nikita B
Nikita B
  • 13k
  • 1
  • 25
  • 57

Regarding your last question... The only point gray camera I have is this one, so I do not have a precise answer. :) If I understand your problem correctly, and if we remove your wrappers from the equation, your problem boils down to:

//pseudo code incoming
ManagedCameraBase CreateCamera(string serial)
{
     var camera =  new ManagedGigECamera();

     camera.Initialize(serial); //do some stuff to get correct info

     if (camera.GetCameraInfo() ...) // check type
     {
         return new ManagedGigECamera()
     }
     else
     { 
         return new ...; // another type of camera
     }
}

  1. This does not look right to me. It is either one of the shittiest API's I have ever seen, or this API simply was not designed to be used as camera detection mechanism (the way you use it). I think the second option is more likely to be true.

  2. When possible, I would prefer relying on correct configuration. I mean if you know the serial number of the camera beforehand you must know it's type as well. It is much easier to either let user select the type, or just load it form preset configuration file, for example:

    <HardwareConfiguration>
  <Camera Id="...">
     <SerialNumber>...</SerialNumber>
     <Type>...</Type>
     <PixelFormat>...</Type>
     <!-- etc... -->
  </Camera>
</HardwareConfiguration>

This approach scales really well, if later you decide to support cameras from other manufacturers.

  1. If you have to auto-detect cameras and can't just get the type from settings, then I am not sure what is the best approach here. You should definitely try contacting Point Gray and ask them whether or not this is a correct way to manage their cameras. Most large manufacturers have fairly decent support staff.

Regarding your last question... The only point gray camera I have is this one, so I do not have a precise answer. :) If I understand your problem correctly, and if we remove your wrappers from the equation, your problem boils down to:

//pseudo code incoming
ManagedCameraBase CreateCamera(string serial)
{
     var camera =  new ManagedGigECamera();

     camera.Initialize(serial); //do some stuff to get correct info

     if (camera.GetCameraInfo() ...) // check type
     {
         return new ManagedGigECamera()
     }
     else
     { 
         return new ...; // another type of camera
     }
}

  1. This does not look right to me. It is either one of the shittiest API's I have ever seen, or this API simply was not designed to be used as camera detection mechanism (the way you use it). I think the second option is more likely to be true.

  2. When possible, I would prefer relying on correct configuration. I mean if you know the serial number of the camera beforehand you must know it's type as well. It is much easier to either let user select the type, or just load it form preset configuration file, for example:

    <HardwareConfiguration>
  <Camera Id="...">
     <SerialNumber>...</SerialNumber>
     <Type>...</Type>
     <PixelFormat>...</Type>
     <!-- etc... -->
  </Camera>
</HardwareConfiguration>

This approach scales really well, if later you decide to support cameras from other manufacturers.

  1. If you have to auto-detect cameras and can't just get the type from settings, then I am not sure what is the best approach here. You should definitely try contacting Point Gray and ask them whether or not this is a correct way to manage their cameras. Most large manufacturers have fairly decent support staff.
Source Link
Nikita B
Nikita B
  • 13k
  • 1
  • 25
  • 57

I'd say no, this is not a good use of dynamic. This looks like a problem that can be easily solved by generics.

public abstract class PointGreyCamera<TCamera> : ICamera where TCamera :  ManagedCameraBase
{
    protected TCamera Camera { get; set; }
    
    ...
    
    public void SetFPS() 
    { 
        //access some imaginary ManagedCameraBase.Fps property
        Camera.Fps = 60;
    }
}

public class PointGreyGige : PointGreyCamera<ManagedGigECamera>
{
    ... 

    private void SetImagingSettings()
    {
        ...
        //access ManagedGigECamera.SetGigEImageSettings method
        Camera.SetGigEImageSettings(camerasSettings);
    }
}

Also you should not call virtual or abstract methods in constructor.