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

[Patrick Williams <patrick@stwcx.xyz>]

[-- Attachment #1: Type: text/plain, Size: 2668 bytes --]

On Wed, Feb 19, 2020 at 10:59:40AM +0800, Lei YU wrote:
> On Wed, Feb 19, 2020 at 4:39 AM Patrick Williams <patrick@stwcx.xyz> wrote:
> >
> > On Thu, Feb 13, 2020 at 10:38:37PM +0800, Lei YU wrote:
> > 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.

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?

> 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.

Agree, but we don't even need this code now if people were just
including the server header files (and there are many examples of
clients doing exactly this).  If we're not going to create client
headers now, this seems like wasted effort just for some slight syntax
improvement.

We could simply symlink <xyz/openbmc_project/foo/server.hpp> to
<xyz/openbmc_project/foo/client.hpp> and add a `namespace
xyz::openbmc_project::foo::client = xyz::openbmc_project:server;` and
get the same syntactic enhancement without nearly as much work.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]