[libcamera-devel] [PATCH v3 1/3] libcamera: controls: Define a default lens position behaviour

[Jacopo Mondi]

Hi Naush

On Tue, Jun 06, 2023 at 02:40:38PM +0100, Naushir Patuck via libcamera-devel wrote:
> Hi Jacopo,
>
> Thank you for your feedback!
>
> On Tue, 6 Jun 2023 at 14:22, Jacopo Mondi <jacopo.mondi at ideasonboard.com> wrote:
> >
> > Hi Naush
> >
> > On Mon, Jun 05, 2023 at 10:14:04AM +0100, Naushir Patuck via libcamera-devel wrote:
> > > Update the AfMode control description to explicitly define a
> > > startup/default behaviour.
> > >
> > > On startup, the camera will move the lens to the position given by the
> > > default value of the LensPosition control if operating in manual focus
> > > mode. Typically this would be the hyperfocal position of the lens.
> > >
> > > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > > ---
> > >  src/libcamera/control_ids.yaml | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
> > > index adea5f90acc5..765168e539a8 100644
> > > --- a/src/libcamera/control_ids.yaml
> > > +++ b/src/libcamera/control_ids.yaml
> > > @@ -408,6 +408,13 @@ controls:
> > >              LensPosition control.
> > >
> > >              In this mode the AfState will always report AfStateIdle.
> > > +
> > > +            If the camera is started in AfModeManual, it will move the focus
> > > +            lens to the position specified by the LensPosition control.
> > > +
> > > +            This mode is the recommended default value for the AfMode control.
> > > +            External cameras (as reported by the Location property set to
> > > +            CameraLocationExternal) may use a different default value.
> >
> > Missing an additional blank line
>
> I wasn't sure about this one.  The documentation for enum values doesn't seem to
> have a blank line between them so I left it out here as well.  Is this
> not correct?

Nope, you're right! I saw a blank line in the proposal Laurent sent
and thought it got lost, but it is indeed intentional and correct your
way

Sorry for the noise!

>
> Regards,
> Naush
>
> >
> > Can be fixed when applying
> >
> > Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> >
> > >          - name: AfModeAuto
> > >            value: 1
> > >            description: |
> > > --
> > > 2.34.1
> > >