bougou / go-ipmi

IPMI library in pure Go
Apache License 2.0
60 stars 22 forks source link

'Sensor Reading' not implemented #15

Closed bcspragu closed 1 year ago

bcspragu commented 1 year ago

Hi there, and thanks for making this package!

The TL;DR is that Sensor Reading doesn't appear to be implemented, or I'm using it wrong. The rest of this issue is just explanation, but I think the real problem is that I was confusing Sensor Reading with Nominal Reading. Currently, Sensor Reading is hardcoded to zero, and I think this is the value I really want here. Are there any plans to implement this? If not, I'm happy to give it a shot and contribute a PR, any pointers would be appreciated.


I've been using this to access IPMI on Dell M610 blade servers in this project, specifically here. One thing I've noticed is that result of sdr.Full.ConvertReading(sdr.Full.NominalReadingRaw) never changes, and the results of running the equivalent ipmitool command produce different output. For example:

ipmitool -I lanplus -H <ip> -U <user> -P <pass> sensor get 'Ambient Temp' produces:

Locating sensor record...
Sensor ID              : Ambient Temp (0x8)
 Entity ID             : 7.1
 Sensor Type (Threshold)  : Temperature
 Sensor Reading        : 21 (+/- 1) degrees C
 Status                : ok
 Lower Non-Recoverable : na
 Lower Critical        : 3.000
 Lower Non-Critical    : 8.000
 Upper Non-Critical    : 42.000
 Upper Critical        : 47.000
 Upper Non-Recoverable : na
 Positive Hysteresis   : 1.000
 Negative Hysteresis   : 1.000
 Assertions Enabled    : lnc- lcr- unc+ ucr+
 Deassertions Enabled  : lnc- lnc+ lcr+ unc+ ucr+

A (hopefully) equivalent invocation in Go:

sdr, err := ic.GetSDRBySensorName("Ambient Temp")
if err != nil {  ...  }
fmt.Println(sdr.Full.String())       

This produces:

Sensor ID              : Ambient Temp (0x08)
Generator             : 0x20
Entity ID             : 7.1 (system board)
Sensor Type (threshold)      : Temperature (0x01)
Sensor Reading        : 0 (+/- 2) degrees C
Sensor Initialization :
  Settable            : false
  Scanning            : true
  Events              : true
  Hysteresis          : true
  Sensor Type         : true
  Default State:
    Event Generation  : enabled
    Scanning          : enabled
Sensor Capabilities   :
  Auto Re-arm         : yes(auto)
        Hysteresis Support  : ReadableSettable
        Threshold Access    : ReadableSettable
        Ev Message Control  : Per-threshold
Mask                  :
  Readable Thresholds : unc ucr lnc lcr
  Settable Thresholds : unc ucr lnc lcr
  Threshold Read Mask : unc ucr lnc lcr
  Assertions Enabled  :
  Deassertions Enabled:
Nominal Reading       : 0x97/23.000
Normal Minimum        : 0x8b/11.000
Normal Maximum        : 0xc5/69.000
Lower Non-Recoverable : unspecified
Lower Critical        : 0x83/3.000
Lower Non-Critical    : 0x88/8.000
Upper Non-Critical    : 0xaa/42.000
Upper Critical        : 0xaf/47.000
Upper Non-Recoverable : unspecified
Positive Hysteresis   : 0x01/-127.000
Negative Hysteresis   : 0x01/-127.000
Minimum sensor range  : unspecified
Maximum sensor range  : unspecified
SensorDirection       : 0
LinearizationFunc     : linear(0)
Reading Factors       : M: (1), T: (2), B: (-128), A: (194), A_Exp: (0), R_Exp: (0), B_Exp: (0)
bougou commented 1 year ago

The SDR record itself does not store current reading value for sensor. It can be separated get by IPMI Command GetSensorReading.

But the value returned by GetSensorReading is a raw value which means it's not aligned with Sensor Unit, so it needs to be converted.

After some thought, I added two extra fields SensorValue and SensorStatus to SDRFull and SDRCompact. SensorValue has already been converted. It can be used directly with SensorUnit.

Try latest code (v0.4.0) to see if it helps.

bcspragu commented 1 year ago

Amazing, that worked perfectly! Thanks for your help