dbus-sensor: setting the upper non-critical(unc) threshold value smaller than reading value would get 3 logs ( assert log, de-assert log and then assert log)

[James Feist]

On 2/18/20 5:32 PM, Max Lai/WYHQ/Wiwynn wrote:
> *Hi James,*
> 
> *Our validation team met a problem in dbus-sensor recently. The bug is 
> about that if we set the upper non-critical (unc) threshold value 
> smaller than reading value, we should get only one assert log but we got 
> 3 logs ( assert log, de-assert log and then assert log) .*
> 
> *We traced the code of Ipmbsensor. The picture below is the code flow of 
> Ipmbsensor.*
> 
> **
> 
> **
> 
> **
> 
> *We found that when we set the threshold , the function match 
> (configMatch) which is registered in Ipmbsensor would catch the signal 
> like the below*
> 
> **
> 
> **
> 
> **
> 
> *When it trigger the handler (eventhandler) and then enter the function 
> of createsensor() and later enter the function of 
> setInitialProperties(). In the setInitialProperties(), It would initial 
> the property (setting the threshold of alarm to default (false) is the 
> root cause). This would trigger Ipmbsensor to send the deassert log. We 
> was wondering that why setting the threshold would initial the property. 
> Is the code flow we draw is correct? Is there any misunderstanding in 
> our thought ? What is the purpose of registering match this signal ?*

Because configurations can change at runtime. For instance if you change 
a threshold, or you add/remove a card. This gets persisted back to 
entity-manager 
https://github.com/openbmc/dbus-sensors/blob/a97f1343379bbb52371195bc613a689cbfb374f3/src/Thresholds.cpp#L118 
and that will trigger an update.

I'm guessing this todo is what is needed to fix your issue:
https://github.com/openbmc/dbus-sensors/blob/a97f1343379bbb52371195bc613a689cbfb374f3/include/sensor.hpp#L117

I was wondering when I wrote this if the config can just update the 
threshold, and updating it locally was not needed. Deleting lines 117 
and 118 might fix your issue.


> 
> **
> 
> *Our source revision :**fb64f45d3399b182ceadffb8fa86ee68c0aa0a11*
> 
> **
> 
> *Note: We found that this issue didn’t happen in CPUSensor and 
> HwmonTempSensor.*
> 
> **
> 
> *Regards,*
> 
> *Max Lai*
> 
> *---------------------------------------------------------------------------------------------------------------------------------------------------------------*
> 
> *This email contains confidential or legally privileged information and 
> is for the sole use of its intended recipient. *
> 
> *Any unauthorized review, use, copying or distribution of this email or 
> the content of this email is strictly prohibited.*
> 
> *If you are not the intended recipient, you may reply to the sender and 
> should delete this e-mail immediately.*
> 
> *---------------------------------------------------------------------------------------------------------------------------------------------------------------*
>