[phosphor-pid-control] scaling issue

[Hank Liou (劉晉翰)]

Hi Patrick,

I think it's OK to parse the config min&max for temp sensors.

Any suggestion?

Thanks,
Hank

>-----Original Message-----
>From: openbmc [mailto:openbmc-
>bounces+hank.liou=quantatw.com at lists.ozlabs.org] On Behalf Of Hank Liou
>(劉晉翰)
>Sent: Friday, August 23, 2019 4:31 PM
>To: Patrick Venture <venture at google.com>; James Feist
><james.feist at linux.intel.com>
>Cc: openbmc at lists.ozlabs.org
>Subject: RE: [phosphor-pid-control] scaling issue
>
>Hi Patrick,
>
>>-----Original Message-----
>>From: Patrick Venture [mailto:venture at google.com]
>>Sent: Wednesday, August 21, 2019 10:32 PM
>>To: Hank Liou (劉晉翰) <Hank.Liou at quantatw.com>; James Feist
>><james.feist at linux.intel.com>
>>Cc: openbmc at lists.ozlabs.org
>>Subject: Re: [phosphor-pid-control] scaling issue
>>
>>On Wed, Aug 21, 2019 at 1:11 AM Hank Liou (劉晉翰)
>><Hank.Liou at quantatw.com> wrote:
>>>
>>> Hi All,
>>>
>>>
>>> After commit [1], I found my temp sensor reading would be re-scaled
>>> by
>>multiplying 1 over 255, making temperature into unfamiliar unit. Also
>>the fan rpm reading would lie in [0,1] interval, letting the fan input
>>to be 0 (since the input value of fan is from an integer array [2]). Are these
>normal behaviors?
>>Or do I miss something?
>>
>>Are you using dbus configuration or json?  If json, can you attach your config.
>>Since you're saying it was working and now isn't, I'm assuming there's
>>something about the config being treated differently with the code
>>changes in an unexpected way.
>
>I found pid control will first read min and max from dbus and then (before
>commit [1]) revise them if
>
>info->min != conf::inheritValueFromDbus (in dbus/dbuspassive.cpp)
>
>After value initialization, the min and max would be the ones in json file (Json
>file [3]). However, after commit [1] the min and max values of config would
>not be fed into the fan control process. The scaling issue is originated from
>commit [4] with the aim to treat fan rpm as percentage. It seems that commit
>[1] unexpectedly affects temp sensors in the sense that the temp is the
>integer reading not percentage. Before commit [1], it would not re-scale the
>temp reading since my min and max are 0 [6].
>
>
>
>There is another issue with commit [1]. Now the fan rpm would be something
>like
>
>1500 / 20000 = 0.075
>
>where rpm max 20000 is from dbus. However the fan input function would
>transfer double into int, which is 0 for 0.075 (see [5]). Hence the fan input is 0
>not percentage. Is there something wrong?
>
>[1] https://github.com/openbmc/phosphor-pid-
>control/commit/fc2e803f5d9256944e18c7c878a441606b1f121c
>[2] https://github.com/openbmc/phosphor-pid-
>control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle
>r.cpp#L86
>[3]
>       {
>            "name": "fan1",
>            "type": "fan",
>            "readPath": "/sys/class/hwmon/hwmon1/fan1_input",
>            "writePath": "/sys/class/hwmon/hwmon1/pwm1",
>            "min": 0,
>            "max": 255
>        },
>        {
>            "name": "temp1",
>            "type": "temp",
>            "readPath": "/xyz/openbmc_project/sensors/temperature/temp1",
>            "writePath": "",
>            "min": 0,
>            "max": 0
>        }
>[4] https://github.com/openbmc/phosphor-pid-
>control/commit/75eb769d351434547899186f73ff70ae00d7934a
>[5] https://github.com/openbmc/phosphor-pid-
>control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/pid/fancontrolle
>r.cpp#L64
>[6] https://github.com/openbmc/phosphor-pid-
>control/blob/a7ec8350d17b70153cebe666d3fbe88bddd02a1a/dbus/dbuspassi
>ve.cpp#L158
>
>>
>>>
>>>
>>> [1]
>>> https://github.com/openbmc/phosphor-pid-
>>control/commit/fc2e803f5d92569
>>> 44e18c7c878a441606b1f121c
>>>
>>> [2]
>>> https://github.com/openbmc/phosphor-pid-
>>control/blob/a7ec8350d17b70153
>>> cebe666d3fbe88bddd02a1a/pid/fancontroller.cpp#L86
>>>
>>>
>>> Thanks,
>>>
>>>
>>> Hank Liou
>>>
>>> Quanta Computer Inc.
>>>
>>>
>
>Sincerely,
>Hank