Question of phosphor-sel-logger

[Matt Spinler]

On 3/25/2021 3:22 AM, Duke Du (杜祥嘉) wrote:
>> -----Original Message-----
>> From: Matt Spinler <mspinler at linux.ibm.com>
>> Sent: Wednesday, March 24, 2021 9:20 PM
>> To: Duke Du (杜祥嘉) <Duke.Du at quantatw.com>; openbmc at lists.ozlabs.org
>> Cc: vernon.mauery at linux.intel.com; jason.m.bills at linux.intel.com; Fran Hsu
>> (徐誌謙) <Fran.Hsu at quantatw.com>; George Hung (洪忠敬)
>> <George.Hung at quantatw.com>
>> Subject: Re: Question of phosphor-sel-logger
>>
>>
>>
>> On 3/24/2021 6:28 AM, Duke Du (杜祥嘉) wrote:
>>> Hi all,
>>>
>>>        I used package phosphor-hwmon and phospor-sel-logger to
>> monitor sensor and create log when sensor reading
>>>      cross the threshold. I found after the commit
>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa of phosphor-sel-logger
>>>      would catch the signal "ThresholdAsserted" to create sensor threhold
>> log in the journal, but the phosphor-hwmon
>>>      would not send the signal "ThresholdAsserted" when sensor reading
>> is abnormal so that phosphor-sel-logger
>>>      would not create the sensor threhold log, am I right ?
>>>
>>>      If I'm right, can you give me some suggestion to fix this side effect, or
>> what setting I have lost in the
>>>      phosphor-hwmon or phosphor-sel-logger ?
>> Hi,
>> That signal isn't defined in phosphor-dbus-interfaces, so phosphor-hwmon
>> cannot use it.  When I tried to add it, it was rejected as-is with a
>> recommendation to break it up into separate signals for each alarm property
>> on each interface.  At that point I gave up and had the code I was working
>> on at the time just look at propertiesChanged signals instead.
>>
>> If you would like to take that up it would entail:
>> * Update
>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgerrit.
>> openbmc-project.xyz%2Fc%2Fopenbmc%2Fphosphor-dbus-interfaces%2F%2B
>> %2F39899&data=04%7C01%7CDuke.Du%40quantatw.com%7Cc5bf4d3d1
>> 6f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C1%7
>> C0%7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w
>> LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp
>> ;sdata=XoY4nKa3Go%2F9jt2coyzOcnXNrcMaw6XUtqnmK57k0ds%3D&res
>> erved=0
>> as requested
>> * Update phosphor-hwmon to emit the new signals
>> * Update phosphor-sel-logger to also listen for these new signals in addition
>> to the  current one, or change the dbus-sensors code to only emit the new
>> signals.
>>
>>
> Hi Matt,
>
>     Thanks for your reply, I want to add a event monitor to listen "signal PropertyChanged" for
>     "phosphor-phosphor-hwmon" only, like watchdog event monitor
>     (https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-sel-logger/+/37774),
>     I think this is a simple way to fix this side effect, what do you think about my thought ?
>     
>     Please feel free to give me some suggestion, thanks very much !

I don't really know the best way to go here other than what I already 
suggested.  sel-logger used to look at PropertiesChanged, but was 
changed to use ThresholdAsserted I think so that it could capture the 
sensor value within the signal.  But applications that need 
phosphor-dbus-interfaces bindings for their operations, like 
phosphor-hwmon, can't use ThresholdAsserted because it failed the PDI 
review.

Jason or Vernon, what do you suggest?


>
> Thanks
> Duke
>
>>>      phosphor-sel-logger commit
>> 25b26e162bd109b51aa09b16f26f9aa3d9d940fa link :
>>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>>
>> ub.com%2Fopenbmc%2Fphosphor-sel-logger%2Fcommit%2F25b26e162bd10
>> 9b51aa0
>> 9b16f26f9aa3d9d940fa&data=04%7C01%7CDuke.Du%40quantatw.com%
>> 7Cc5bf4
>> d3d16f046cc6efa08d8eec78fd7%7C179b032707fc4973ac738de7313561b2%7C
>> 1%7C0
>> %7C637521888783853893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
>> wMDAiLCJQ
>> IjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&sdata=rHxKqIk
>> Vg7
>>> yQXmqvjXal7I6eVBzw3ifl26gsZF8o4xo%3D&reserved=0
>>>
>>>      Thanks very much !
>>>      Duke