| Message ID | 20220704025632.3911253-2-taki@igel.co.jp (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | Test pixel blend mode | expand |
Hello! On 7/4/22 5:56 AM, 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) > + if prop.type != pykms.PropertyType.Enum: > + raise RuntimeError(f'Unsupported property type {prop.type} for value {v}') > + for value, mode in prop.enums.items(): > + if mode == v: > + v = value > + break > + else: Hm, doesn't seem to be correctly aligned? > + 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}') MBR, Sergey
Hi Sergei, > 2022/07/04 17:43、Sergei Shtylyov <sergei.shtylyov@gmail.com>のメール: > > Hello! > > On 7/4/22 5:56 AM, 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) >> + if prop.type != pykms.PropertyType.Enum: >> + raise RuntimeError(f'Unsupported property type {prop.type} for value {v}') >> + for value, mode in prop.enums.items(): >> + if mode == v: >> + v = value >> + break >> + else: > > Hm, doesn't seem to be correctly aligned? That ‘else’ is for ‘for’-loop. Am I missing something here? >> + 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}') > > MBR, Sergey Many Thanks, Takanari Hayama, Ph.D. <taki@igel.co.jp> IGEL Co., Ltd. https://www.igel.co.jp/
On 7/4/22 11:53 AM, 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) >>> + if prop.type != pykms.PropertyType.Enum: >>> + raise RuntimeError(f'Unsupported property type {prop.type} for value {v}') >>> + for value, mode in prop.enums.items(): >>> + if mode == v: >>> + v = value >>> + break >>> + else: >> >> Hm, doesn't seem to be correctly aligned? > > That ‘else’ is for ‘for’-loop. Am I missing something here? Hm, I don't really know Python, can it actually have else for a loop? >>> + 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}') MBR, Sergey [...]
Hi, > 2022/07/04 18:00、Sergei Shtylyov <sergei.shtylyov@gmail.com>のメール: > > On 7/4/22 11:53 AM, 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) >>>> + if prop.type != pykms.PropertyType.Enum: >>>> + raise RuntimeError(f'Unsupported property type {prop.type} for value {v}') >>>> + for value, mode in prop.enums.items(): >>>> + if mode == v: >>>> + v = value >>>> + break >>>> + else: >>> >>> Hm, doesn't seem to be correctly aligned? >> >> That ‘else’ is for ‘for’-loop. Am I missing something here? > > Hm, I don't really know Python, can it actually have else for > a loop? Yes. :) > When the items are exhausted (which is immediately when the sequence is empty or an iterator raises a StopIteration exception), the suite in the else clause, if present, is executed, and the loop terminates. From https://docs.python.org/3/reference/compound_stmts.html#the-for-statement > >>>> + 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}') > > MBR, Sergey Many Thanks, Takanari Hayama, Ph.D. <taki@igel.co.jp> IGEL Co., Ltd. https://www.igel.co.jp/
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) > + 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}')
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}')
Hi Laurent, Thank you for reviewing the patches. > 2022/08/01 1:18、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. Right. That makes more sense. > >>> + 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. Thank you. >> >>> + 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 Cheers, Takanari Hayama, Ph.D. <taki@igel.co.jp> IGEL Co., Ltd. https://www.igel.co.jp/
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) + if prop.type != pykms.PropertyType.Enum: + raise RuntimeError(f'Unsupported property type {prop.type} for value {v}') + for value, mode in prop.enums.items(): + 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}')
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(-)