[libcamera-devel] [PATCH v4 04/37] utils: ipc: add templates for code generation for IPC mechanism

[paul.elder at ideasonboard.com]

Hi Jacopo,

On Tue, Nov 17, 2020 at 03:55:40PM +0100, Jacopo Mondi wrote:
> Hi Paul,
> 
> On Fri, Nov 06, 2020 at 07:36:34PM +0900, Paul Elder wrote:
> > Add templates to mojo to generate code for the IPC mechanism. These
> > templates generate:
> > - module header
> > - module serializer
> > - IPA proxy cpp, header, and worker
> >
> > Given an input data definition mojom file for a pipeline.
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> >
> > ---
> > Changes in v4:
> > For the non-template files:
> > - rename IPA{pipeline_name}CallbackInterface to
> >   IPA{pipeline_name}EventInterface
> >   - to avoid the notion of "callback" and emphasize that it's an event
> > - add support for strings in custom structs
> > - add validation, that async methods must not have return values
> >   - it throws exception and isn't very clear though...?
> > - rename controls to libcamera::{pipeline_name}::controls (controls is
> >   now lowercase)
> > - rename {pipeline_name}_generated.h to {pipeline_name}_ipa_interface.h,
> >   and {pipeline_name}_serializer.h to {pipeline_name}_ipa_serializer.h
> >   - same for their corresponding template files
> > For the template files:
> > - fix spacing (now it's all {{var}} instead of some {{ var }})
> >   - except if it's code, so code is still {{ code }}
> > - move inclusion of corresponding header to first in the inclusion list
> > - fix copy&paste errors
> > - change snake_case to camelCase in the generated code
> >   - template code still uses snake_case
> > - change the generated command enums to an enum class, and make it
> >   capitalized (instead of allcaps)
> > - add length checks to recvIPC (in proxy)
> > - fix some template spacing
> > - don't use const for PODs in function/signal parameters
> > - add the proper length checks to readPOD/appendPOD
> >   - the helper functions for reading and writing PODs to and from
> >     serialized data
> > - rename readUInt/appendUInt to readPOD/appendPOD
> > - add support for strings in custom structs
> >
> > Changes in v3:
> > - add support for namespaces
> > - fix enum assignment (used to have +1 for CMD applied to all enums)
> > - use readHeader, writeHeader, and eraseHeader as static class functions
> >   of IPAIPCUnixSocket (in the proxy worker)
> > - add requirement that base controls *must* be defined in
> >   libcamera::{pipeline_name}::Controls
> >
> > Changes in v2:
> > - mandate the main and callback interfaces, and init(), start(), stop()
> >   and their parameters
> > - fix returning single pod value from IPC-called function
> > - add licenses
> > - improve auto-generated message
> > - other fixes related to serdes
> > ---
> >  .../module_ipa_interface.h.tmpl               | 113 ++++
> >  .../module_ipa_proxy.cpp.tmpl                 | 238 +++++++++
> >  .../module_ipa_proxy.h.tmpl                   | 118 +++++
> >  .../module_ipa_proxy_worker.cpp.tmpl          | 187 +++++++
> >  .../module_ipa_serializer.h.tmpl              |  44 ++
> >  .../libcamera_templates/proxy_functions.tmpl  | 205 ++++++++
> >  .../libcamera_templates/serializer.tmpl       | 280 ++++++++++
> >  .../generators/mojom_libcamera_generator.py   | 488 ++++++++++++++++++
> >  8 files changed, 1673 insertions(+)
> >  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy.h.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/proxy_functions.tmpl
> >  create mode 100644 utils/ipc/generators/libcamera_templates/serializer.tmpl
> >  create mode 100644 utils/ipc/generators/mojom_libcamera_generator.py
> >
> > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> > new file mode 100644
> > index 00000000..a470b99e
> > --- /dev/null
> > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_interface.h.tmpl
> > @@ -0,0 +1,113 @@
> > +{#- SPDX-License-Identifier: LGPL-2.1-or-later -#}
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) {{year}}, Google Inc.
> 
> Wondering if the copyright shouldn't be attribute to who writes the
> template.

tbh, I'm not sure :/

> Apart from this, thanks for the monstrous patch, that's a template,
> I'm not reviewing it in detail

Hehe, thanks :)


Paul

<snip>