Re: [PATCH 1/3] tests: Support enum property type

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

One more comment.

On Sun, Jul 31, 2022 at 07:01:00PM +0300, Laurent Pinchart wrote:
> Hi Hayama-san,
> 
> Thank you for the patch.
> 
> On Mon, Jul 04, 2022 at 11:56:30AM +0900, Takanari Hayama wrote:
> > Add a support for enum property type to AtomicRequest.
> > 
> > Signed-off-by: Takanari Hayama <taki@igel.co.jp>
> > ---
> >  tests/kmstest.py | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/kmstest.py b/tests/kmstest.py
> > index 11cc328b5b32..224c160e32fa 100755
> > --- a/tests/kmstest.py
> > +++ b/tests/kmstest.py
> > @@ -269,8 +269,18 @@ class AtomicRequest(pykms.AtomicReq):
> >  
> >                      min, max = prop.values
> >                      v = min + int((max - min) * int(v[:-1]) / 100)
> > -                else:
> > +                elif v.isnumeric():
> >                      v = int(v)
> > +                else:
> > +                    prop = obj.get_prop(k)

I've run this test on a kernel that doesn't support the blend mode
property, and the prop.type access below raised an exception that isn't
very nice to read. If that's fine with you, I'll add

                    if not prop:
                        raise RuntimeError(f'Property {k} not supported by object {obj}')

here to make error messages more readable.

> > +                    if prop.type != pykms.PropertyType.Enum:
> > +                        raise RuntimeError(f'Unsupported property type {prop.type} for value {v}')
> > +                    for value, mode in prop.enums.items():
> 
> I'd replace "mode" with "name" here. Apart from that,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> I'll change this when applying the patch.
> 
> > +                        if mode == v:
> > +                            v = value
> > +                            break
> > +                    else:
> > +                        raise RuntimeError(f'Enum value with name "{v}" not found in property {k}')
> >  
> >              if not isinstance(v, int):
> >                  raise RuntimeError(f'Unsupported value type {type(v)} for property {k}')

-- 
Regards,

Laurent Pinchart