dbus-sensors:hwmontemp: additional attribute proposal

[Ed Tanous]

On Fri, Aug 7, 2020 at 9:54 AM Jason Ling <jasonling at google.com> wrote:
>
> > What about making the device/type lists in PSUSensors extendable using JSON?
>>
>> Say more about what you're wanting to do here.....
>
>  Take https://github.com/openbmc/dbus-sensors/blob/master/src/PSUSensorMain.cpp#L43 and https://github.com/openbmc/dbus-sensors/blob/master/src/PSUSensorMain.cpp#L59 and make it so those can be extended using (for example) without an upstream code change. I picked JSON as the easiest example.
> IIRC PSUSensors does validity checks to make sure that the device emits a name in its 'permit list' (hwmontempsensor is less picky) so tricking PSUSensors into gathering telemetry for a non-public device is tricky.
>

I'm conflicted on that.  Part of the reason that list is there, and
not in the config files directly, is to convey that those are the
types that have been tested to work correctly, and the types that the
config files "promise" to work sanely.  The other thing is, if you've
done the testing, adding to that list is (should be) relatively
trivial and straightforward.  Opening up that list to runtime parsing
opens a lot of security questions I'm not prepared to answer.

>> Can you give an
>> example of what you would use it for?
>
> Sure, the primary use case would be
> It's a non public device. Would rather not broadcast information about it or have to obfuscate the device name. Really would rather not maintain patches until the device is made public.

First off, you will always have merge conflicts when you do this.
Moving the config to json doesn't eliminate the possibility of merge
conficts.  I'd highly recommend doing the research in your org to
understand why these things can't be public.  In a lot of cases it's
legitimate, in others it's just dogma.  The intent behind EM was that
even if the configuration files have to live downstream, a majority of
the daemon code can be upstream.  Unless you're working on custom
silicon (and please don't tell me if you are) this should work just
fine with most commercially available devices.
In theory, the intended "recommended by the authors" way to handle a
"custom, can't be published anywhere" device would be to create a
totally separate daemon that implements that new device functionality,
and adds an exposes record specifically for that to your config in a
patch.  In theory, the config file patch is small, and pretty easy to
merge, and should be the only conflict you're likely to get if your
app is self-contained.  That's not to say this is supported by the
project, or that the EM maintainers make any promise to not break you,
but it tends to be a lot less likely, and has been a pattern that I've
seen work well in the past.