dbus-sensors:hwmontemp: additional attribute proposal

[Ed Tanous]

On Tue, Aug 11, 2020 at 5:46 PM Alex Qiu <xqiu at google.com> wrote:
>
> The question was more of a general ask on whether dbus-sensors plans for this on record. If so, individual daemon should have the config option to ignore a device completely. Currently, I think PSUSensor has the ability, but HwmonTempSensor does not.
>

You're talking about ignoring a specific leg of a device?  Like
ignoring a particular sensor on an installed device?  Or are you
talking about ignoring a device class entirely?  Maybe calling out the
specific use case of what you're wanting to ignore might help here.

> The reason behind it may be complicated. One is if we can fix the PSUSensor performance issue on time so that we can use it directly for PID control based on VR temperatures. And then, if we can't fix it on time, what work around can we have?

I'm assuming "on time" here refers to some product schedule?  Without
knowing your particular timelines, I will say this: we've solved
several orders of magnitude worse performance problems in dbus sensors
in the past.  I'm assuming this one just requires the proper
application of debugging, thought, and engineering.  I'm not sure how
to answer your question about workarounds:  You would have whatever
workarounds you're willing to build, that's the beauty of open source.
If using HwMonTempSensor is a workaround, and you're willing to live
with the patch, by all means, use it.

> Is it upstream-able or local patches

That would depend on what your workarounds entail.  If your
workarounds follow the principles of the project, don't duplicate
functionality, don't break any of the existing use cases, and are
maintainable, then they're probably upstreamable.  If you're
duplicating features between daemons unnecessarily for lack of wanting
to hunt down and understand a performance bug, that's probably
something you need to keep in a local patch until you have time to do
the appropriate debugging.

> ? What's more, can we have different polling rates for temperature and voltage/current/power by using multiple daemon for the same device?

Why don't we just make the polling rate configurable in the EM config?
 Each sensor has its own timer for exactly this reason.  In the
original design of dbus-sensors, each sensor instance also ran in its
own process.  We moved it to each sensor type running in a shared
process because the context switches were getting really bad, and a
lot of the sensors of similar types had very similar event matches, so
it allowed us to reduce the dbus load for things like power on.  We
could certainly revisit this, but for what you're wanting, I suspect
we can just configure the polling rates per sensor.  We hardcoded them
under the assumption that we could build one reasonable default that
worked for everyone, but clearly you've broken that assumption, so
lets throw it in a config file, put some reasonable defaults on it,
and call it good.  (note, this is subject to James' opinion as
maintainer, I'm not sure if he'll agree here).

> Of course, ideally, we can go for a fast feature-enriched PSUSensor to take care of everything and deprecate HwmonTempSensor, but you know I have been talking about schedule for multiple times with you before...
>

I understand short schedules.  It always feels like there's not enough
time.  The best advice I have here is to try to break your problem
space down into small problems such that you can get a patch written,
tested, and pushed to gerrit out for that day.  Then use those patches
to slowly move toward what you want, even if you keep stacking them up
in upstream.  Eventually the maintainer will review them, or you'll
have solved someone else's problem before they hit it.  If enough
people do this, we'll be way ahead on these types of bugs.

At some point James might need to school us on what the theoretical
difference is between PSUsensor and HwmonSensor.  I know one was
originally built for PSU modules, and the other was built for on board
hardware, but they're getting pretty similar, maybe it does make sense
to have certain devices in both?