Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'Sensor Reading' not implemented #15

Closed
bcspragu opened this issue Mar 27, 2023 · 2 comments
Closed

'Sensor Reading' not implemented #15

bcspragu opened this issue Mar 27, 2023 · 2 comments

Comments

@bcspragu
Copy link

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
Copy link
Owner

bougou commented Mar 28, 2023

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
Copy link
Author

Amazing, that worked perfectly! Thanks for your help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants