Refactor ipmi::sensor::GetSensorResponse away from std::array
This change refactors GetSensorResponse from std::array to struct.
This change depends on change #23544.
GetSensorResponse is an internal, intermediate structure, an unpacked
form of a Get Sensor Reading response, providing direct access to its
fields. Its life time is:
GetReadingResponse -> GetSensorResponse -> ipmi::RspType.
It is written to in 5 functions in the ipmi::sensor::get namespace, by
four setter functions (setOffset, setReading, setAssertionBytes,
enableScanning).
It is currently read from by 1 function (ipmiSensorGetSensorReading)
for transforming to an ipmi::RspType.
Originally, the setter functions assumed bitwise equivalence between
GetSensorResponse and GetReadingResponse, and the setter functions
used reinterpret_cast to assign to a GetSensorResponse as if it were
a GetReadingResponse.
With this change, the reinterpret_cast's are removed, and the set
functions now accept GetSensorResponse instead of GetReadingResponse,
so the code gets a bit easier to read.
Tested: Tested using a server with a BMC; sensor readings obtained
through `ipmitool` appear to be correct (the reading might change
within a small range):
# ipmitool raw 0x04 0x2d 0x16
9B 40 00 00
Signed-off-by: Sui Chen <suichen@google.com>
Change-Id: I5d454d6249f5431fb98169e6ef7c585c34024004
diff --git a/include/ipmid/types.hpp b/include/ipmid/types.hpp
index e2f80c0..3e64cb4 100644
--- a/include/ipmid/types.hpp
+++ b/include/ipmid/types.hpp
@@ -100,7 +100,15 @@
constexpr auto inventoryRoot = "/xyz/openbmc_project/inventory";
-using GetSensorResponse = std::array<uint8_t, sizeof(GetReadingResponse)>;
+struct GetSensorResponse
+{
+ uint8_t reading; // sensor reading
+ bool readingOrStateUnavailable; // 1 = reading/state unavailable
+ bool scanningEnabled; // 0 = sensor scanning disabled
+ bool allEventMessagesEnabled; // 0 = All Event Messages disabled
+ uint8_t thresholdLevelsStates; // threshold/discrete sensor states
+ uint8_t discreteReadingSensorStates; // discrete-only, optional states
+};
using OffsetValueMap = std::map<Offset, Values>;
diff --git a/sensordatahandler.cpp b/sensordatahandler.cpp
index 923b96a..0d1182d 100644
--- a/sensordatahandler.cpp
+++ b/sensordatahandler.cpp
@@ -132,7 +132,6 @@
{
sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
GetSensorResponse response{};
- auto responseData = reinterpret_cast<GetReadingResponse*>(response.data());
auto service = ipmi::getService(bus, interface, path);
@@ -149,7 +148,7 @@
{
if (propValue == value.second.assert)
{
- setOffset(value.first, responseData);
+ setOffset(value.first, &response);
break;
}
}
@@ -169,7 +168,6 @@
{
sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
GetSensorResponse response{};
- auto responseData = reinterpret_cast<GetReadingResponse*>(response.data());
auto service = ipmi::getService(bus, sensorInfo.sensorInterface,
sensorInfo.sensorPath);
@@ -188,7 +186,7 @@
{
if (propValue == value.second.assert)
{
- setReading(value.first, responseData);
+ setReading(value.first, &response);
break;
}
}
diff --git a/sensordatahandler.hpp b/sensordatahandler.hpp
index 129cbb4..99c3ae9 100644
--- a/sensordatahandler.hpp
+++ b/sensordatahandler.hpp
@@ -156,7 +156,6 @@
{
sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
GetSensorResponse response{};
- auto responseData = reinterpret_cast<GetReadingResponse*>(response.data());
auto service = ipmi::getService(bus, sensorInfo.sensorInterface,
sensorInfo.sensorPath);
@@ -166,8 +165,7 @@
sensorInfo.propertyInterfaces.begin()->first,
sensorInfo.propertyInterfaces.begin()->second.begin()->first);
- setAssertionBytes(static_cast<uint16_t>(std::get<T>(propValue)),
- responseData);
+ setAssertionBytes(static_cast<uint16_t>(std::get<T>(propValue)), &response);
return response;
}
@@ -184,10 +182,10 @@
GetSensorResponse readingData(const Info& sensorInfo)
{
sdbusplus::bus::bus bus{ipmid_get_sd_bus_connection()};
- GetSensorResponse response{};
- auto responseData = reinterpret_cast<GetReadingResponse*>(response.data());
- enableScanning(responseData);
+ GetSensorResponse response{};
+
+ enableScanning(&response);
auto service = ipmi::getService(bus, sensorInfo.sensorInterface,
sensorInfo.sensorPath);
@@ -228,8 +226,7 @@
auto rawData = static_cast<uint8_t>((value - sensorInfo.scaledOffset) /
sensorInfo.coefficientM);
-
- setReading(rawData, responseData);
+ setReading(rawData, &response);
return response;
}
diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index a506c82..473f6bc 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -417,8 +417,8 @@
uint5_t, // reserved
bool, // reading state
- bool, // sensor scanning state disabled
- bool, // all event message state disabled
+ bool, // 0 = sensor scanning state disabled
+ bool, // 0 = all event messages disabled
uint8_t, // threshold levels states
uint8_t // discrete reading sensor states
@@ -443,32 +443,15 @@
try
{
- ipmi::sensor::GetSensorResponse getResponse{};
- getResponse = iter->second.getFunc(iter->second);
+ ipmi::sensor::GetSensorResponse getResponse =
+ iter->second.getFunc(iter->second);
- constexpr uint8_t senReadingResp = 0;
- constexpr uint8_t senScanStateResp = 1;
- constexpr uint8_t assertionStatesLsbResp = 2;
- constexpr uint8_t assertionStatesMsbResp = 3;
- constexpr uint8_t senReadStateMask = 0x20;
- constexpr uint8_t senScanStateMask = 0x40;
- constexpr uint8_t allEventMessageStateMask = 0x80;
-
- uint8_t senReading = getResponse[senReadingResp];
- constexpr uint5_t reserved{0};
- bool readState =
- static_cast<bool>(getResponse[senScanStateResp] & senReadStateMask);
- bool senScanState =
- static_cast<bool>(getResponse[senScanStateResp] & senScanStateMask);
- bool allEventMessageState = static_cast<bool>(
- getResponse[senScanStateResp] & allEventMessageStateMask);
-
- uint8_t assertionStatesLsb = getResponse[assertionStatesLsbResp];
- uint8_t assertionStatesMsb = getResponse[assertionStatesMsbResp];
-
- return ipmi::responseSuccess(senReading, reserved, readState,
- senScanState, allEventMessageState,
- assertionStatesLsb, assertionStatesMsb);
+ return ipmi::responseSuccess(getResponse.reading, uint5_t(0),
+ getResponse.readingOrStateUnavailable,
+ getResponse.scanningEnabled,
+ getResponse.allEventMessagesEnabled,
+ getResponse.thresholdLevelsStates,
+ getResponse.discreteReadingSensorStates);
}
#ifdef UPDATE_FUNCTIONAL_ON_FAIL
catch (const SensorFunctionalError& e)
diff --git a/sensorhandler.hpp b/sensorhandler.hpp
index f5a8b41..8780235 100644
--- a/sensorhandler.hpp
+++ b/sensorhandler.hpp
@@ -644,15 +644,15 @@
* @param[in] offset - offset number.
* @param[in/out] resp - get sensor reading response.
*/
-inline void setOffset(uint8_t offset, ipmi::sensor::GetReadingResponse* resp)
+inline void setOffset(uint8_t offset, ipmi::sensor::GetSensorResponse* resp)
{
if (offset > 7)
{
- resp->assertOffset8_14 |= 1 << (offset - 8);
+ resp->discreteReadingSensorStates |= 1 << (offset - 8);
}
else
{
- resp->assertOffset0_7 |= 1 << offset;
+ resp->thresholdLevelsStates |= 1 << offset;
}
}
@@ -662,7 +662,7 @@
* @param[in] offset - offset number.
* @param[in/out] resp - get sensor reading response.
*/
-inline void setReading(uint8_t value, ipmi::sensor::GetReadingResponse* resp)
+inline void setReading(uint8_t value, ipmi::sensor::GetSensorResponse* resp)
{
resp->reading = value;
}
@@ -675,10 +675,10 @@
* @param[in/out] resp - get sensor reading response.
*/
inline void setAssertionBytes(uint16_t value,
- ipmi::sensor::GetReadingResponse* resp)
+ ipmi::sensor::GetSensorResponse* resp)
{
- resp->assertOffset0_7 = static_cast<uint8_t>(value & 0x00FF);
- resp->assertOffset8_14 = static_cast<uint8_t>(value >> 8);
+ resp->thresholdLevelsStates = static_cast<uint8_t>(value & 0x00FF);
+ resp->discreteReadingSensorStates = static_cast<uint8_t>(value >> 8);
}
/**
@@ -686,9 +686,11 @@
*
* @param[in/out] resp - get sensor reading response.
*/
-inline void enableScanning(ipmi::sensor::GetReadingResponse* resp)
+inline void enableScanning(ipmi::sensor::GetSensorResponse* resp)
{
- resp->operation = 1 << 6;
+ resp->readingOrStateUnavailable = false;
+ resp->scanningEnabled = true;
+ resp->allEventMessagesEnabled = false;
}
} // namespace sensor