[sdbusplus] To generate a common header for public information of interfaces

[Lei YU]

On Wed, Feb 19, 2020 at 4:39 AM Patrick Williams <patrick at stwcx.xyz> wrote:
>
> On Thu, Feb 13, 2020 at 10:38:37PM +0800, Lei YU wrote:
> > Currently, sdbus++ generates the server code separately, and in
> > phosphor-dbus-interfaces, the separated headers are installed, and the
> > separated cpp files are combined together, and compiled as a library.
> > The public information of the interfaces (e.g, the interface strings,
> > the enum strings) are either in the separated header or in the cpp
> > files.
> >
> > The result is, when a phosphor service needs to use an interface, or
> > an enum, it has to define the interface string or the enum string
> > manually, and it happens everywhere.
>
> I would say any case where this was done should have been fixed.  There
> were already functions in sdbusplus to convert<Enum>ToString and
> convertForMessage(<Enum>).  There are lots of cases where these are
> being used today[1].  You recently made the interface public as well, so
> we should begin converting these over.
>
> I've also got commits pending, for merge soon, that make the enums be
> supported as native types, so code should rarely even need to call the
> "convert" functions anymore.  Another refactoring once that is merged.

The idea of my proposal here is not to use "convertXXX" functions,
instead, it is to provide the constexpr strings that could be used by
client code.
E.g. the client code does not need to call any function, but directly use:

* xyz::...::server::MyServer::interface, instead of a user-defined string
  for the interface.
* xyz::...::MyServer::MyEnum::Enum1, instead of a user-defined string for
  the enum.

>
> >
> > How about
> > 1. Making sdbusplus to generate a "common" header for each interface
> > including the public information;
> > 2. Then phosphor-dbus-interfaces could generate a single header file
> > that includes all the public information of the interfaces;
> > 3. Then the phosphor service could include a single header file, and
> > use the interface/enum strings it needs, without manually defining
> > them?
>
> Currently only the 'server' interfaces are generated and some clients
> are including the server header files even though they are a client.
> The intention all along was to make proper 'client' bindings but there
> hasn't been sufficient effort put into that yet.
>
> I'm not sure if it is better or worse to have an explicit 'common'
> header rather than the two separate 'server' and 'client' headers.  Is
> there any reason to not simply get started on the client headers?
>
> Some pro/cons of 'common':
>     - Have an extra pass we have to run through 'sdbus++'.  This will
>       probably make the templates more complex (3x code paths rather
>       than 2x).
>
>     + Explicitly consistent types between client and server bindings.

Yup, the implementation is actually part of the "client" header, we
could rename the "common" to "client".
But preferably, we could combine all the client header into a single
header, which makes it easier for the client code to use.
If a client needs to set an enum property to a service, it then does
not have to include the server header nor the specific client header,
but include a single header.
Anyway, this part is done in phosphor-dbus-interface.

So we could say that sdbusplus generates part of the "client" header
instead of "common" header.

>
> >
> > There is an initial concept implementation:
> > * https://github.com/mine260309/sdbusplus/commit/78cb63fb7e1ceb62165c71e15779f23f7e9f166c
> > * https://github.com/mine260309/phosphor-dbus-interfaces/commit/6079d25547f0143fc7562a1c7ee6beb888a66432
> >
> > With the above changes, a service could directly use the generated
> > interface string, e.g.
> > `sdbusplus::xyz::openbmc_project::Software::ApplyTime::interface`.
> > In the future, the enum strings could be put there and thus we do not
> > have to manually write the long full qualified string.
> >
> > Ideas and suggestions are welcome.
>
> [1] https://github.com/search?q=org%3Aopenbmc+convertForMessage&type=Code
>
> --
> Patrick Williams