py: gen-py-controls: Use generic workaround for enum prefixes

24251 diff mbox series

Detecting the prefix from the enum value strings doesn't work properly if
the individual values start with the same characters. This is the case, for
example, with On/Off and Start/Stop. Therefore, a hard-coded workaround was
required for the LensShadingMapMode (Off/On) control.

Since enum values typically use the control name as the prefix, a better
and more generic solution would be to use the name directly as the prefix.

However, since some existing control enums use a different prefix than
their name, the previous version is still required for these.

To eliminate the workaround and still remain compatible, the control name
is used as the prefix if the prefix determined with the original method
begins with the control name.

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
---
 src/py/libcamera/gen-py-controls.py | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)


---
base-commit: d54e5537ca0909339bb6950f3a565c9077406a3c
change-id: 20250828-py-control-prefix-72ca9aac9f16

Best regards,

Hi

2025. 08. 28. 11:25 keltezéssel, Matthias Fend írta:
> Detecting the prefix from the enum value strings doesn't work properly if
> the individual values start with the same characters. This is the case, for
> example, with On/Off and Start/Stop. Therefore, a hard-coded workaround was
> required for the LensShadingMapMode (Off/On) control.

Was something other than LensShadingMapMode missed? Or the motivation is just
to be more future proof?

FYI, the same issue is present in gen-gst-controls.py, although that only considers
a fixed set of controls.

In any case, I'm hoping that https://patchwork.libcamera.org/patch/23381/ could be
merged at some point, eliminating this issue once and for all.


Regards,
Barnabás Pőcze

> 
> Since enum values typically use the control name as the prefix, a better
> and more generic solution would be to use the name directly as the prefix.
> 
> However, since some existing control enums use a different prefix than
> their name, the previous version is still required for these.
> 
> To eliminate the workaround and still remain compatible, the control name
> is used as the prefix if the prefix determined with the original method
> begins with the control name.
> 
> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> ---
>   src/py/libcamera/gen-py-controls.py | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> index d43a7c1c7eab5744988febebc493c8a858f1d82e..8e9fb560145dd655f3d605be2863407feaef1fc1 100755
> --- a/src/py/libcamera/gen-py-controls.py
> +++ b/src/py/libcamera/gen-py-controls.py
> @@ -34,15 +34,9 @@ def extend_control(ctrl, mode):
>       if not ctrl.is_enum:
>           return ctrl
>   
> -    if mode == 'controls':
> -        # Adjustments for controls
> -        if ctrl.name == 'LensShadingMapMode':
> -            prefix = 'LensShadingMapMode'
> -        else:
> -            prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> -    else:
> -        # Adjustments for properties
> -        prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> +    prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> +    if prefix.startswith(ctrl.name):
> +        prefix = ctrl.name
>   
>       for enum in ctrl.enum_values:
>           enum.py_name = enum.name[len(prefix):]
> 
> ---
> base-commit: d54e5537ca0909339bb6950f3a565c9077406a3c
> change-id: 20250828-py-control-prefix-72ca9aac9f16
> 
> Best regards,

Hi Barnabás,

Am 28.08.2025 um 11:53 schrieb Barnabás Pőcze:
> Hi
> 
> 2025. 08. 28. 11:25 keltezéssel, Matthias Fend írta:
>> Detecting the prefix from the enum value strings doesn't work properly if
>> the individual values start with the same characters. This is the 
>> case, for
>> example, with On/Off and Start/Stop. Therefore, a hard-coded 
>> workaround was
>> required for the LensShadingMapMode (Off/On) control.
> 
> Was something other than LensShadingMapMode missed? Or the motivation is 
> just
> to be more future proof?

I created a new control that uses Start and Stop as values ​​and 
wondered why camshark offered "art" and "op" as options ;)

Instead of another hard-coded workaround, I tried this slightly more 
generic solution.

best regards
  ~Matthias

> 
> FYI, the same issue is present in gen-gst-controls.py, although that 
> only considers
> a fixed set of controls.
> 
> In any case, I'm hoping that https://patchwork.libcamera.org/ 
> patch/23381/ could be
> merged at some point, eliminating this issue once and for all.
> 
> 
> Regards,
> Barnabás Pőcze
> 
>>
>> Since enum values typically use the control name as the prefix, a better
>> and more generic solution would be to use the name directly as the 
>> prefix.
>>
>> However, since some existing control enums use a different prefix than
>> their name, the previous version is still required for these.
>>
>> To eliminate the workaround and still remain compatible, the control name
>> is used as the prefix if the prefix determined with the original method
>> begins with the control name.
>>
>> Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
>> ---
>>   src/py/libcamera/gen-py-controls.py | 12 +++---------
>>   1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/ 
>> gen-py-controls.py
>> index 
>> d43a7c1c7eab5744988febebc493c8a858f1d82e..8e9fb560145dd655f3d605be2863407feaef1fc1 100755
>> --- a/src/py/libcamera/gen-py-controls.py
>> +++ b/src/py/libcamera/gen-py-controls.py
>> @@ -34,15 +34,9 @@ def extend_control(ctrl, mode):
>>       if not ctrl.is_enum:
>>           return ctrl
>> -    if mode == 'controls':
>> -        # Adjustments for controls
>> -        if ctrl.name == 'LensShadingMapMode':
>> -            prefix = 'LensShadingMapMode'
>> -        else:
>> -            prefix = find_common_prefix([e.name for e in 
>> ctrl.enum_values])
>> -    else:
>> -        # Adjustments for properties
>> -        prefix = find_common_prefix([e.name for e in ctrl.enum_values])
>> +    prefix = find_common_prefix([e.name for e in ctrl.enum_values])
>> +    if prefix.startswith(ctrl.name):
>> +        prefix = ctrl.name
>>       for enum in ctrl.enum_values:
>>           enum.py_name = enum.name[len(prefix):]
>>
>> ---
>> base-commit: d54e5537ca0909339bb6950f3a565c9077406a3c
>> change-id: 20250828-py-control-prefix-72ca9aac9f16
>>
>> Best regards,
>

Quoting Barnabás Pőcze (2025-08-28 10:53:01)
> Hi
> 
> 2025. 08. 28. 11:25 keltezéssel, Matthias Fend írta:
> > Detecting the prefix from the enum value strings doesn't work properly if
> > the individual values start with the same characters. This is the case, for
> > example, with On/Off and Start/Stop. Therefore, a hard-coded workaround was
> > required for the LensShadingMapMode (Off/On) control.
> 
> Was something other than LensShadingMapMode missed? Or the motivation is just
> to be more future proof?
> 
> FYI, the same issue is present in gen-gst-controls.py, although that only considers
> a fixed set of controls.
> 
> In any case, I'm hoping that https://patchwork.libcamera.org/patch/23381/ could be
> merged at some point, eliminating this issue once and for all.

I'm fine progressing that patch for this next release window!

Could you rebase the work and get an update on the list please?

--
Kieran

> Regards,
> Barnabás Pőcze
> 
> > 
> > Since enum values typically use the control name as the prefix, a better
> > and more generic solution would be to use the name directly as the prefix.
> > 
> > However, since some existing control enums use a different prefix than
> > their name, the previous version is still required for these.
> > 
> > To eliminate the workaround and still remain compatible, the control name
> > is used as the prefix if the prefix determined with the original method
> > begins with the control name.
> > 
> > Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
> > ---
> >   src/py/libcamera/gen-py-controls.py | 12 +++---------
> >   1 file changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py
> > index d43a7c1c7eab5744988febebc493c8a858f1d82e..8e9fb560145dd655f3d605be2863407feaef1fc1 100755
> > --- a/src/py/libcamera/gen-py-controls.py
> > +++ b/src/py/libcamera/gen-py-controls.py
> > @@ -34,15 +34,9 @@ def extend_control(ctrl, mode):
> >       if not ctrl.is_enum:
> >           return ctrl
> >   
> > -    if mode == 'controls':
> > -        # Adjustments for controls
> > -        if ctrl.name == 'LensShadingMapMode':
> > -            prefix = 'LensShadingMapMode'
> > -        else:
> > -            prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> > -    else:
> > -        # Adjustments for properties
> > -        prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> > +    prefix = find_common_prefix([e.name for e in ctrl.enum_values])
> > +    if prefix.startswith(ctrl.name):
> > +        prefix = ctrl.name
> >   
> >       for enum in ctrl.enum_values:
> >           enum.py_name = enum.name[len(prefix):]
> > 
> > ---
> > base-commit: d54e5537ca0909339bb6950f3a565c9077406a3c
> > change-id: 20250828-py-control-prefix-72ca9aac9f16
> > 
> > Best regards,
>