[sdbusplus] To generate client header (was: Re: To generate a common header for public information of interfaces)

[Lei YU]

> > > I don't see any reason any code should ever directly utilize the enum
> > > strings.  We generate code already to safely convert them to and from
> > > real C++ enum types.  Why would you want to use string comparison
> > > instead?
> > >
> > > Can you elaborate on why we should be enabling this usage pattern?
> >
> > There are cases for a service to use a "enum string" directly instead
> > of getting the string from enum by `convertForMessage()`
> > E.g.
> > * https://github.com/openbmc/phosphor-bmc-code-mgmt/blob/master/activation.hpp#L43
> > * https://github.com/openbmc/phosphor-host-ipmid/blob/master/chassishandler.cpp#L218
>
> Examples of poor code are not a use case.  Both of these examples are
> trivially converted to convert<enum>ToString APIs.  We should do that
> rather than facilitate unmaintainable code.

My concern to use "convertXXX" APIs is that it requires the client
code to link the server code, where logically all it needs is the enum
string.

>
> >
> > Some of the cases could be changed to use `convertForMessage()`, but
> > if one does want to directly use that, or use it as constexpr, why not
> > provide it directly?
>
> Because it limits our future ability to change the string format.  When
> people decided to reverse engineer what sdbusplus was doing and hard
> code strings, they made it so we cannot change the format of what
> sdbusplus is sending.
>
> There was a commit recently, as an example, that requested we add an
> Intel-only "Management Engine" enumeration to an xyz.openbmc_project interface
> rather than a com.intel one.  If we supported enumeration inheritance,
> there would have been less need for this.  I don't know how we might
> implement enumeration inheritance, but we might need to change the
> "on-the-wire" string as a result.  Right now we're hamstrung by people
> having hard coded strings.

This is exactly why I propose to provide enum strings in by the client header.
Currently, client code (poorly) uses "hard-coded" strings directly. If
we provide the constexpr strings in the client header, the client code
could be "refacted" to use the definitions from the client header.
Then sdbusplus is freely to update the string format without breaking
client code.

>
> > I do not think it's good behavior for a client to include another
> > server's heaader files.
> > E.g. if I am writing client code and want to set a property on another
> > service (e.g. foo.bar), I do not really want to include a
> > "foo/bar/server.hpp", because it's not part of my client.
> > What I want is some constexpr strings, like interface name, property
> > name, enum string, etc.
>
> With the current state of things, programming choices are:
>     1. Include a header poorly named "server" into a "client" code.
>     2. Reimplement what is in that "server" header file.
>
> Under no circumstances is a good approach or one that should be
> encouraged.
>
> > Ideally, sdbusplus would generate the full client binding code. But we
> > do not have that now.
> > So I am proposing to move a bit forward to generate a little client
> > code that would benefit the client's usage. The client won't have to
> > write the full string manually anymore.
>
> So maybe we call it "client" rather than "common"?  Common implies that
> there are header files that are okay to import under any case and I
> don't see that to be the case with what you're proposing we define.
>

Yeah, agreed.

> Again, I don't have any issue with the interface names.  I do take issue
> with the enumeration strings because they shouldn't ever be used outside
> of sdbusplus (or a similar dbus binding).
>

For the interface names, the patch is updated at:
https://gerrit.openbmc-project.xyz/c/openbmc/sdbusplus/+/29404
Let's treat it as a start point of client code.

> --
> Patrick Williams