[libcamera-devel] [PATCH v7 05/13] py: generate control enums from yaml

[Laurent Pinchart]

On Thu, May 05, 2022 at 03:15:53PM +0100, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-05-05 11:40:56)
> > Generate enums for controls from control_ids.yaml. The generator script
> > has some heuristics to generate nicer enum names. E.g. instead of having
> > "LensShadingMapMode.LensShadingMapModeOff" we get
> > "LensShadingMapMode.Off". This heuristics may need to be updated when
> > the yaml file is changed or new controls are added.

Maybe we should do the same in C++, with enum class ?

> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> > ---
> >  src/py/libcamera/gen-py-control-enums.py  | 95 +++++++++++++++++++++++
> >  src/py/libcamera/meson.build              |  9 +++
> >  src/py/libcamera/pyenums_generated.cpp.in | 21 +++++
> >  src/py/libcamera/pymain.cpp               |  2 +
> >  4 files changed, 127 insertions(+)
> >  create mode 100755 src/py/libcamera/gen-py-control-enums.py
> >  create mode 100644 src/py/libcamera/pyenums_generated.cpp.in
> > 
> > diff --git a/src/py/libcamera/gen-py-control-enums.py b/src/py/libcamera/gen-py-control-enums.py
> > new file mode 100755
> > index 00000000..f1b18389
> > --- /dev/null
> > +++ b/src/py/libcamera/gen-py-control-enums.py
> > @@ -0,0 +1,95 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +#
> > +# Generate Python bindings enums for controls from YAML
> > +
> > +import argparse
> > +import string
> > +import sys
> > +import yaml
> > +
> > +
> > +def find_common_prefix(strings):
> > +    prefix = strings[0]
> > +
> > +    for string in strings[1:]:
> > +        while string[:len(prefix)] != prefix and prefix:
> > +            prefix = prefix[:len(prefix) - 1]
> > +        if not prefix:
> > +            break
> > +
> > +    return prefix
> > +
> > +
> > +def generate_py(controls):
> > +    out = ""

The other python scripts use single quotes for strings, could you
standardize on that ? You already use single quotes in several locations
in this script actually.

> > +
> > +    for ctrl in controls:
> > +        name, ctrl = ctrl.popitem()
> > +
> > +        enum = ctrl.get('enum')
> > +        if not enum:
> > +            continue
> > +
> > +        if ctrl.get('draft'):
> > +            ns = "libcamera::controls::draft::"
> > +        else:
> > +            ns = "libcamera::controls::"
> > +
> > +        cpp_enum = name + "Enum"
> > +
> > +        out += "\tpy::enum_<{}{}>(m, \"{}\")\n".format(ns, cpp_enum, name)
> > +
> > +        if name == "LensShadingMapMode":
> > +            prefix = "LensShadingMapMode"
> 
> This seems a bit arbitrary ...
> 
> Perhaps this needs to be in a list. But if there's only one for now - it
> can be reworked when it comes up I guess?
> 
> Anyway, that's what you said in the commit message too, and the rest
> seems ok I think so lets get it tested and in use to find out more ;-)
> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > +        else:
> > +            prefix = find_common_prefix([e["name"] for e in enum])
> > +
> > +        for entry in enum:
> > +            cpp_enum = entry["name"]
> > +            py_enum = entry["name"][len(prefix):]
> > +
> > +            out += "\t\t.value(\"{}\", {}{})\n".format(py_enum, ns, cpp_enum)
> > +
> > +        out += "\t;\n"
> > +
> > +    return {"enums": out}
> > +
> > +
> > +def fill_template(template, data):
> > +    template = open(template, 'rb').read()
> > +    template = template.decode('utf-8')
> > +    template = string.Template(template)
> > +    return template.substitute(data)
> > +
> > +
> > +def main(argv):
> > +    # Parse command line arguments
> > +    parser = argparse.ArgumentParser()
> > +    parser.add_argument('-o', dest='output', metavar='file', type=str,
> > +                        help='Output file name. Defaults to standard output if not specified.')
> > +    parser.add_argument('input', type=str,
> > +                        help='Input file name.')
> > +    parser.add_argument('template', type=str,
> > +                        help='Template file name.')
> > +    args = parser.parse_args(argv[1:])
> > +
> > +    data = open(args.input, 'rb').read()
> > +    controls = yaml.safe_load(data)['controls']
> > +
> > +    data = generate_py(controls)
> > +
> > +    data = fill_template(args.template, data)
> > +
> > +    if args.output:
> > +        output = open(args.output, 'wb')
> > +        output.write(data.encode('utf-8'))
> > +        output.close()
> > +    else:
> > +        sys.stdout.write(data)
> > +
> > +    return 0
> > +
> > +
> > +if __name__ == '__main__':
> > +    sys.exit(main(sys.argv))
> > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> > index e1f5cf21..42988e55 100644
> > --- a/src/py/libcamera/meson.build
> > +++ b/src/py/libcamera/meson.build
> > @@ -17,6 +17,15 @@ pycamera_sources = files([
> >      'pyenums.cpp',
> >  ])
> >  
> > +gen_input_files = [meson.project_source_root() / 'src/libcamera/control_ids.yaml', 'pyenums_generated.cpp.in']

gen_input_files = [
    meson.project_source_root() / 'src/libcamera/control_ids.yaml',
    'pyenums_generated.cpp.in',
]

Possibly with

    meson.project_source_root() / 'src' / 'libcamera' / 'control_ids.yaml',

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > +
> > +generated_sources = custom_target('py_gen_controls',
> > +                                  input : gen_input_files,
> > +                                  output : ['pyenums_generated.cpp'],
> > +                                  command : ['gen-py-control-enums.py', '-o', '@OUTPUT@', '@INPUT@'])
> > +
> > +pycamera_sources += generated_sources
> > +
> >  pycamera_deps = [
> >      libcamera_public,
> >      py3_dep,
> > diff --git a/src/py/libcamera/pyenums_generated.cpp.in b/src/py/libcamera/pyenums_generated.cpp.in
> > new file mode 100644
> > index 00000000..96daf257
> > --- /dev/null
> > +++ b/src/py/libcamera/pyenums_generated.cpp.in
> > @@ -0,0 +1,21 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> > + *
> > + * Python bindings
> > + *
> > + * This file is auto-generated. Do not edit.
> > + */
> > +
> > +#include <libcamera/libcamera.h>
> > +
> > +#include <pybind11/smart_holder.h>
> > +
> > +namespace py = pybind11;
> > +
> > +using namespace libcamera;
> > +
> > +void init_pyenums_generated(py::module& m)
> > +{
> > +${enums}
> > +}
> > diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> > index 54674caf..81d48a20 100644
> > --- a/src/py/libcamera/pymain.cpp
> > +++ b/src/py/libcamera/pymain.cpp
> > @@ -115,10 +115,12 @@ static void handleRequestCompleted(Request *req)
> >  }
> >  
> >  void init_pyenums(py::module &m);
> > +void init_pyenums_generated(py::module &m);
> >  
> >  PYBIND11_MODULE(_libcamera, m)
> >  {
> >         init_pyenums(m);
> > +       init_pyenums_generated(m);
> >  
> >         /* Forward declarations */
> >  

-- 
Regards,

Laurent Pinchart