[libcamera-devel] [PATCH] libcamera: Test sensor's ability to discover ancillary devices

[Kieran Bingham]

Hi Yunke,

Quoting Yunke Cao (2022-04-21 06:54:05)
> Use vimc lens to test sensor's ability to discover ancillary lens.

Great idea!

> Tested with the recent kernel patch for vimc lens:
> https://lore.kernel.org/linux-media/20220415023855.2568366-1-yunkec@google.com/

Could you test this with a kernel that /doesn't/ have that patch applied
as well please?

> 
> Signed-off-by: Yunke Cao <yunkec at google.com>
> ---
>  src/libcamera/camera_sensor.cpp      |  5 +++++
>  src/libcamera/pipeline/vimc/vimc.cpp |  2 ++
>  test/camera-sensor.cpp               | 13 +++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index eaa2da6b..fe366267 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -152,6 +152,11 @@ int CameraSensor::init()
>          */
>         if (entity_->device()->driver() == "vimc") {
>                 initVimcDefaultProperties();
> +
> +               ret = discoverAncillaryDevices();
> +               if (ret)
> +                       return ret;
> +
>                 return initProperties();
>         }
>  
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index c3e33ecf..1d8b9f4d 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -449,6 +449,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>         dm.add("Sensor B");
>         dm.add("Debayer A");
>         dm.add("Debayer B");
> +       dm.add("Lens A");
> +       dm.add("Lens B");

This will cause kernels that don't (yet) have a lens on VIMC to fail to
match the VIMC pipeline handler. I think you can leave these out.

>         dm.add("RGB/YUV Input");
>         dm.add("Scaler");
>  
> diff --git a/test/camera-sensor.cpp b/test/camera-sensor.cpp
> index 372ee4af..55a5b68d 100644
> --- a/test/camera-sensor.cpp
> +++ b/test/camera-sensor.cpp
> @@ -12,6 +12,7 @@
>  
>  #include <libcamera/base/utils.h>
>  
> +#include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor.h"
>  #include "libcamera/internal/device_enumerator.h"
>  #include "libcamera/internal/media_device.h"
> @@ -57,6 +58,12 @@ protected:
>                         return TestFail;
>                 }
>  
> +               lens_ = sensor_->focusLens();
> +               if (!lens_) {
> +                       cerr << "Failed to get ancillary lens" << endl;
> +                       return TestFail;
> +               }

And at least until we 'require' a kernel version with this - I think
this will have to be optional.

> +
>                 return TestPass;
>         }
>  
> @@ -104,6 +111,11 @@ protected:
>                         return TestFail;
>                 }
>  
> +               if (lens_->setFocusPosition(10)) {

And then this would have to first test if the lens is set or such
		if (lens_ && lens_->setFocusPosition(10)) {

--
Kieran


> +                       cerr << "Failed to set lens focus position" << endl;
> +                       return TestFail;
> +               }
> +
>                 return TestPass;
>         }
>  
> @@ -116,6 +128,7 @@ private:
>         std::unique_ptr<DeviceEnumerator> enumerator_;
>         std::shared_ptr<MediaDevice> media_;
>         CameraSensor *sensor_;
> +       CameraLens *lens_;
>  };
>  
>  TEST_REGISTER(CameraSensorTest)
> -- 
> 2.36.0.rc0.470.gd361397f0d-goog
>