[OpenBMC] [phosphor-sel-logger] Property type conversion and the value of getScaledIPMIValue
George Hung (洪忠敬)
George.Hung at quantatw.com
Tue Mar 26 17:43:11 AEDT 2019
Hi all,
Sorry for the previous mail, I should not use the attached file for the mailing list. I re-describe what we would like to do:
Currently phosphor-sel-logger support handles only double style sensors, and we would like to add both of double and in64 support for phosphor-sel-logger.
But I'm not sure my change is OK and post my modification as follows first: (making sel-logger work with int64 and double.)
diff --git a/include/threshold_event_monitor.hpp b/include/threshold_event_monitor.hpp
index 9352805..00dba32 100644
--- a/include/threshold_event_monitor.hpp
+++ b/include/threshold_event_monitor.hpp
@@ -115,7 +115,7 @@ inline static sdbusplus::bus::match::match startThresholdEventMonitor(
"org.freedesktop.DBus.Properties", "GetAll");
getSensorValue.append("xyz.openbmc_project.Sensor.Value");
boost::container::flat_map<std::string,
- sdbusplus::message::variant<double>>
+ sdbusplus::message::variant<double, int64_t>>
sensorValue;
try
{
@@ -135,6 +135,14 @@ inline static sdbusplus::bus::match::match startThresholdEventMonitor(
ipmi::VariantToDoubleVisitor(), sensorValue["MinValue"]);
double sensorVal = sdbusplus::message::variant_ns::visit(
ipmi::VariantToDoubleVisitor(), sensorValue["Value"]);
+ double scale = 0;
+ if(sdbusplus::message::variant_ns::get_if<int64_t>(
+ &sensorValue["Value"]))
+ {
+ scale = sdbusplus::message::variant_ns::visit(
+ ipmi::VariantToDoubleVisitor(), sensorValue["Scale"]);
+ sensorVal *= std::pow(10, scale);
+ }
try
{
eventData[1] = ipmi::getScaledIPMIValue(sensorVal, max, min);
@@ -158,7 +166,7 @@ inline static sdbusplus::bus::match::match startThresholdEventMonitor(
conn->new_method_call(msg.get_sender(), msg.get_path(),
"org.freedesktop.DBus.Properties", "Get");
getThreshold.append(thresholdInterface, event);
- sdbusplus::message::variant<double> thresholdValue;
+ sdbusplus::message::variant<double, int64_t> thresholdValue;
try
{
sdbusplus::message::message getThresholdResp =
@@ -173,6 +181,10 @@ inline static sdbusplus::bus::match::match startThresholdEventMonitor(
}
double thresholdVal = sdbusplus::message::variant_ns::visit(
ipmi::VariantToDoubleVisitor(), thresholdValue);
+ if(sdbusplus::message::variant_ns::get_if<int64_t>(&thresholdValue))
+ {
+ thresholdVal *= std::pow(10, scale);
+ }
try
{
eventData[2] = ipmi::getScaledIPMIValue(thresholdVal, max, min);
--
If it's OK, I will push the patch to OpenBMC Gerrit.
Thanks.
Best Regard
George Hung
Research Division - Software Engineer
Cloud Computing Business Unit
Quanta Computer Inc.
Ext: 16830
E-Mail : George.Hung at QuantaTW.com
-----Original Message-----
From: George Hung (洪忠敬)
Sent: Tuesday, March 26, 2019 2:26 PM
To: 'Bills, Jason M'; OpenBMC Maillist; emilyshaffer at google.com; benjaminfair at google.com
Cc: Richard Tung (董彥屏); Buddy Huang (黃天鴻); Will Liang (梁永鉉); Fran Hsu (徐誌謙)
Subject: RE: [OpenBMC] [phosphor-sel-logger] Property type conversion and the value of getScaledIPMIValue
Hi all,
Considering there are two style of sensors (scaled int64 and double values) for sensor value DBus properties, we would like to add both of them support for phosphor-sel-logger.
The attached file is the patch for making sel-logger work with int64 and double.
If it's OK, I will push the patch to OpenBMC Gerrit.
Hi Jason,
Thanks for your kind advices.
About the first question I mentioned before, "The sensor DBus properties , “/MaxValue/MinValue/Value/” are always 0", it's because type conversion cause always get 0, not their values are invalid. I'm sorry for my confused description.
Thanks.
Best Regard
George Hung
Research Division - Software Engineer
Cloud Computing Business Unit
Quanta Computer Inc.
Ext: 16830
E-Mail : George.Hung at QuantaTW.com
-----Original Message-----
From: Bills, Jason M [mailto:jason.m.bills at linux.intel.com]
Sent: Saturday, March 23, 2019 6:50 AM
To: George Hung (洪忠敬)
Cc: Richard Tung (董彥屏); Buddy Huang (黃天鴻); Will Liang (梁永鉉); Fran Hsu (徐誌謙)
Subject: Re: [OpenBMC] [phosphor-sel-logger] Property type conversion and the value of getScaledIPMIValue
Hi George,
It is better to post these types of questions to the full mailing list as I am not the right person to answer all of them.
On 3/22/2019 12:20 AM, George Hung (洪忠敬) wrote:
> Hi Jason,
>
> Since our OpenBMC project needs to support IPMI SEL, I try to use
> phosphor-sel-logger to add new IPMI SEL to journal, but I found some
> issues need to discuss:
>
> 1.The sensor DBus properties , “/MaxValue/MinValue/Value/” are always
> 0
This means that something is wrong in the sensor configuration. These values should all be valid.
>
> 2.The /value/ parameter of /getScaledIPMIValue/ function uses the
> scaled value, not actual value,
The threshold event monitor is currently designed to monitor only the double-style sensors that store the actual value.
>
> it may cause the result of /scaleIPMIValueFromDouble /function is
> “value out of range”
>
> After tracing the code, we may find the root cause, but need your help
> to check if it’s the correct fix. I listed as follows:
>
> 1.The original type declaration is
> “sdbusplus::message::variant<double>”, but should be
> “sdbusplus::message::variant<int64_t>”
There are two styles of sensors, one style stores scaled int64 values and another style stores actual double values. The threshold event monitor currently handles only double style sensors.
If you need to support both styles of sensors, you can look here for an example visitor that can handle both int64 and double:
https://github.com/openbmc/phosphor-pid-control/commit/c065cf1b195f9ccd8fea882ce76416473d4fee7c.
>
> 2.Modify to use actual sensor value for “getScaledIPMIValue” function
>
> The attached file is the fix patch, it should be more clear.
>
> If it’s the correct fix, will you patch to OpenBMC Gerrit, or we do it
> directly ?
Please feel free to push your patch to OpenBMC Gerrit for review and discussion.
>
> BTW, the phosphor-sel-logger supports add IPMI SEL to journal, but
> seems only threshold-based sensor event logs, no discrete sensors event logs.
That is correct.
>
> Is there any plan to add discrete sensors event logs to
> phosphor-sel-logger ?
Intel plans to only support threshold sensor events in the IPMI SEL.
The phosphor-sel-logger should be extensible to support additional types of events.
More information about the openbmc
mailing list