Showing signed sensor value when the command "ipmitool sel elist" is executed

[Tony Lee (李文富)]

> On 10/29/19 9:18 AM, James Feist wrote:
> > On 10/28/19 11:40 PM, Tony Lee (李文富) wrote:
> >>> On 10/27/19 11:41 PM, Tony Lee (李文富) wrote:
> >>>> Hi Jason,
> >>>>
> >>>> We know that when we execute the command "ipmitool sel elist", it
> >>>> will
> >>> return something like the following,
> >>>> "18 | 10/16/19 | 18:28:41 UTC | Temperature nvme0 | Upper
> >>>> Non-critical
> >>> going high | Asserted | Reading 72 > Threshold 70 degrees C".
> >>>>
> >>>> I met a problem that when the sensor value in the d-bus is
> >>>> negative, the
> >>> current reading in "ipmitool sel elist" will be 0.
> >>>> It seems that because the type of scaledValue is defined
> >>>> "uint32_t", there is
> >>> always a none negative value even current value is a negative value
> >>> obtained from the d-bus. In
> >>>>
> >>> https://github.com/openbmc/phosphor-sel-logger/blob/master/include/s
> >>> ens
> >>> orutils.hpp#L159
> >>>>
> >>>> Is this is an issue or I need to set it up somewhere?
> >>>
> >>> If min is < 0, then this should work:
> >>>
> >>>
> https://github.com/openbmc/phosphor-sel-logger/blob/6afe9560852c6431
> >>> c4
> >>> 3c8e79a28e2b7cb498e355/include/sensorutils.hpp#L61
> >>>
> >>>
> https://github.com/openbmc/phosphor-dbus-interfaces/blob/12162be363f
> >>> 11
> >>>
> b9dafa92b5379db671712b3523c/xyz/openbmc_project/Sensor/Value.interfa
> >>> c
> >>> e.yaml#L28
> >>>
> >>> It uses MinValue < 0 to determine if the sensor is signed or not.
> >>
> >> Hi James,
> >>
> >> Yes, My MinValue and min are < 0.
> >> I understand that If min is < 0 then bSigned = true finally it will
> >> return static_cast<int8_t>(scaledValue) then this should work. But,
> >> after
> >>
> https://github.com/openbmc/phosphor-sel-logger/blob/6afe9560852c6431c
> >> 43c8e79a28e2b7cb498e355/include/sensorutils.hpp#L159
> >>
> >> I got scaledValue = 0 and return 0 in the end.
> >>
> >> There are another question that why is "scaledValue" defined as uint32_t?
> >> Shouldn't it return a byte(uint8_t) after calculation?
> >
> > The return value is an uint8_t (about 5 lines below where you linked).
> > The point of it being a uint32_t is so you can check for overflow.
> > That being said looking at the unit tests I don't believe we check for
> > negative, and that might need to be a int32_t.
OK, I get it.

> > The tests are here:
> > https://github.com/openbmc/intel-ipmi-oem/blob/master/tests/test_senso
> > rcommands.cpp
> >
> >
> > I'll try to take a look in the next day or two and write a new test to
> > see if that fixes it, but feel free to add a test yourself if you beat
> > me to it.
> 
> Give this a shot:
> https://gerrit.openbmc-project.xyz/c/openbmc/intel-ipmi-oem/+/26664
Looks good to me, thanks for the help.

> It'll need to be ported to phosphor-sel-logger when review passes.
> 
> >
> > -James
> >
> >>
> >> Here is my settings and debug logs:
> >> max = 127.000000
> >> min = -128.000000
> >> value = -1.000000
> >> mValue = 1
> >> rExp = 0
> >> bValue = 0
> >> bExp = 0
> >> bSigned = true
> >> scaledValue = 0
> >>
> >> Thanks,
> >> -Tony
> >>
> >>> Thanks,
> >>>
> >>> -James
> >>>
> >>>>
> >>>> Thanks
> >>>> Best Regards,
> >>>> Tony
> >>>>