Repository navigation
Decode SDR, SEL, FRU and completion codes by the specification (11 decoder issues) - #143
Conversation
Eleven decoder bugs, all found by the review of 2026-10, fixed together: - #127: a Get SDR reply that carries only the next record ID is accepted, so the walk skips the record instead of failing. - #87: reserved rate unit, modifier unit usage and power restore policy values decode to None/Unknown instead of throwing. - #81: a completion code the library does not list no longer aborts the decoding (and hence times out): CompletionCode.Unknown with the raw code on IPMIException; only 00h and C0h-FFh are generic for IPMI commands, Read FRU Data and Close Session decode their own codes; Get Channel Cipher Suites and Close Session check the completion code. - #86: the FRU locator reads the LUN from bits [4:3] and keeps its SDR record ID; the generic locator reads the bus, span and ID string where Table 43-10 puts them. - #125: SEL OEM entries are decoded with their own layout (manufacturer ID, OEM data), C0h and E0h are in the OEM ranges, reserved types give a Reserved record instead of an exception. - #82: the threshold status bits are decoded in severity order with the right names, and the states of a threshold sensor map the comparison bits to the matching going-low/going-high events. - #110: the scanning-disabled bit is exposed; a reading the BMC flags as unavailable or not scanned is not reported, nor are its states. - #83: thresholds are gated on byte 12, linearized like the reading, e^x and cube root are handled, non-linear types return the linear value, the accuracy exponent and the tolerance are decoded right. - #85: the last multirecord is decoded, unknown multirecords are skipped, a word is 2 bytes, the manufacturing date is UTC and null when unspecified, the power supply capacity is LS byte first, the compatibility masks are read at the record offset, the English language codes are 0 and 25, non-English strings are UTF-16LE; the common header checksum is checked and truncated areas are skipped. - #129: an undefined threshold is NaN so that 0 is a threshold; a Full record without analog reading produces no reading line; an OEM sensor with one state byte gets its state. - #128: a FRU 0 or Get FRU Inventory Area Info failure costs that FRU only, FRU 0 is returned once and from whichever area describes it, and a FRU is truncated at its first unreadable chunk instead of shifting the following chunks into the gap. Verified on the Lenovo IMM and the GIGABYTE BMC: the text result is identical to main apart from reading drift, minus the three GIGABYTE sensors whose readings the BMC flags as unavailable, and the Lenovo FRU warnings dropped from one per chunk to one per FRU. Fixes #127, fixes #87, fixes #81, fixes #86, fixes #125, fixes #82, fixes #110, fixes #83, fixes #85, fixes #129, fixes #128. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b286c459dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Deactivate Payload reads its command-specific codes from the raw byte and reports it on the exception. - Completion-code labels follow Table 5-2: 01h-7Eh OEM, 80h-BEh command-specific, 7Fh/BFh/D7h-FEh reserved. - Non-linear sensors (70h-7Fh) and reserved linearizations give NaN, and hasAnalogReading() is false for them: their conversion needs Get Sensor Reading Factors, which the library does not implement. - Javadoc reattached to calcFormula(int); Javadoc on the new setters. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2543486a17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…log reading - ReadFruData takes the offset in bytes and sends it in the unit of the device (divided by two for a word-addressed FRU), the count stays in bytes, as ipmitool does; the runner advances its cursor in bytes. - calcFormula() returns NaN for the data format 11b, as hasAnalogReading() promises. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4acb22b26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- A multirecord whose record checksum is wrong is skipped; a wrong header checksum ends the multirecord area, as the length and the end-of-list flag cannot be trusted. - upgrading.md tells low-level callers that the ReadFruData offset and count are now in bytes, with the loop to write. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A Dell iDRAC 8 (PowerEdge R630) writes the record checksum of its power supply multirecords off by one (0xEF for data summing to 0x10), on genuine 750 W records. The record checksum mismatch is therefore logged at DEBUG and the length-delimited record decoded anyway, as the area checksums are; the header checksum stays fatal for the area, as a wrong length would derail the walk. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f087ad032d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
One PR for the eleven decoder issues of the October review, as asked. Each fix is small and spec-cited; the table maps issues to changes.
GetSdraccepts a reply that carries only the next record ID: the runner skips the record and the walk goes onNone/Unknown(new enum constant) instead of throwing; type14hwas already skipped by the runner since #112CompletionCode.parseIntreturns the newUnknowninstead of throwing, so an OEM or command-specific code no longer aborts the decode and surfaces as a timeout.IpmiLanResponsetreats only00handC0h-FFhas generic for IPMI commands (the RMCP+ status codes keep their meaning for the RAKP coders).IPMIException.getRawCode()carries the byte; the message saysOEM completion code 0x8A..IpmiCommandCoder.decodeCommandSpecificCompletionCode()lets a command map its own codes: Read FRU Data (81hbusy) and Close Session (87h/88h) do. Get Channel Cipher Suites and Close Session now check the completion codegetId()keeps the SDR record ID. Generic locator: bus/span masks0x7, ID string at byte 16SelRecordTyperanges use>=, reserved types giveReserved.SelRecorddecodes OEM entries with their layout:getManufacturerId(),getOemData(), system-event fields left nullSensorState.parseInttests UNR to LNC with the right names.getStatesAsserted()maps the comparison bits of a threshold sensor to the going-low/going-high event offsetsisScanningEnabled()exposed;IpmiResultConverterandGetSensorsRunner.buildStates()skip a reading the BMC flags as unavailable or not scannedlinearizationset before the conversions;e^xandcbrtadded, non-linear types (70h+) return the linear value like ipmitool;(b & 0x0c) >> 2; tolerance = half raw counts x abs(M) x 10^RBaseUnit.Words= 2 bytes; mfg date in UTC,nullwhen unspecified; power supply capacity LS byte first; compatibility masks atoffset + 6; English = language code 0 or 25; non-English strings UTF-16LE; common header checksum checked; truncated areas skipped (bad area checksums logged at DEBUG and decoded, as ipmitool does)Double.NaN(the getters are unchanged), so0is reported as a threshold;FullSensorRecord.hasAnalogReading()and the converter skip data-format11brecords; one-byte OEM states are formatted0xLLVisible changes
Listed in
upgrading.md: NaN thresholds,CompletionCode.UnknownandgetRawCode(),FruDeviceLocatorRecord.getId(), the SEL OEM accessors, the UTC manufacturing date, unavailable sensors returned without reading.sensors.md,chassis-status.md,supported-commands.md,troubleshooting.mdand the SEL example oflow-level-api.md(re-run on the Lenovo IMM) are updated.Tests
33 new tests (76 in all):
GetSdrTest, completion codes inIpmiCommandCoderTest(with a sharedIpmiResponseshelper),GetSensorReadingTest,SelRecordTest,ReadFruDataTest(a built FRU image: areas, last multirecord, unknown multirecord, truncation, header checksum, capacity, mfg date),LocatorRecordTest,FullSensorRecordTest,IpmiResultConverterReadingTest,GetFrusRunnerTest,GetChassisStatusResponseDataTest, plus the updatedGetSensorsRunnerTest.mvn verifyon JDK 17: 0 checkstyle / PMD / CPD / SpotBugs findings.Live
getFrusAndSensorsAsStringResult()compared betweenmainand this branch, same harness, same minute:GPU_PROC1,GPU_PROC2andOCP30_TEMP, which the BMC flags as unavailable and which were reported as0.0(the evidence of "Reading unavailable" / "scanning disabled" bits are ignored - unavailable sensors are reported as 0.0 #110).Not reproduced on hardware, by the spec only: the word-addressed FRU, the non-English strings, the multirecord fixes (neither test BMC has a multirecord area), the generic device locator.
Fixes #127, fixes #87, fixes #81, fixes #86, fixes #125, fixes #82, fixes #110, fixes #83, fixes #85, fixes #129, fixes #128.
🤖 Generated with Claude Code