PLDM: Fix in the setNumericEffecterValueHandler
The effecter value for the numeric effecter can be limited only
till uint32 as per PLDM spec.We have a property ScheduledTime
under the interface xyz.openbmc_project.State.ScheduledHostTransition
which has the propertyType as uint64. When host tries to set that
numeric effecter we were hitting an error. This commit resolves this.
Tested:
Before the fix
Received Msg
09 01 9d 02 31 71 00 04 00 00 00 00
Error setting property, ERROR=std::get: wrong index for variant
PROPERTY=ScheduledTime
INTERFACE=xyz.openbmc_project.State.ScheduledHostTransition
PATH=/xyz/openbmc_project/state/host0
Sending Msg
1d 02 31 01
After the fix
pldmtool raw -d 0x80 0x02 0x31 0x71 0x00 0x04 0x00 0x00 0x00 0x00
Request Message:
08 01 80 02 31 71 00 04 00 00 00 00
Response Message:
08 01 00 02 31 00
Signed-off-by: Pavithra Barithaya <pavithra.b@ibm.com>
Change-Id: I289471d1e11bc1f47ed8af4254ac6baf975c2d1d
diff --git a/libpldmresponder/platform_numeric_effecter.hpp b/libpldmresponder/platform_numeric_effecter.hpp
index 8aeaaa1..eea53e6 100644
--- a/libpldmresponder/platform_numeric_effecter.hpp
+++ b/libpldmresponder/platform_numeric_effecter.hpp
@@ -28,7 +28,8 @@
/** @brief Function to get the effecter value by PDR factor coefficient, etc.
* @param[in] pdr - The structure of pldm_numeric_effecter_value_pdr.
- * @tparam[in] effecterValue - effecter value.
+ * @param[in] effecterValue - effecter value.
+ * @param[in] propertyType - type of the D-Bus property.
*
* @return - std::pair<int, std::optional<PropertyValue>> - rc:Success or
* failure, PropertyValue: The value to be set
@@ -36,7 +37,7 @@
template <typename T>
std::pair<int, std::optional<PropertyValue>>
getEffecterRawValue(const pldm_numeric_effecter_value_pdr* pdr,
- T& effecterValue)
+ T& effecterValue, std::string propertyType)
{
// X = Round [ (Y - B) / m ]
// refer to DSP0248_1.2.0 27.8
@@ -55,6 +56,13 @@
rc = PLDM_ERROR_INVALID_DATA;
}
value = rawValue;
+ if (propertyType == "uint64_t")
+ {
+ std::cerr << " Inside if when propertytype is uint64_t"
+ << std::endl;
+ auto tempValue = std::get<uint8_t>(value);
+ value = static_cast<uint64_t>(tempValue);
+ }
break;
}
case PLDM_EFFECTER_DATA_SIZE_SINT8:
@@ -81,6 +89,11 @@
rc = PLDM_ERROR_INVALID_DATA;
}
value = rawValue;
+ if (propertyType == "uint64_t")
+ {
+ auto tempValue = std::get<uint16_t>(value);
+ value = static_cast<uint64_t>(tempValue);
+ }
break;
}
case PLDM_EFFECTER_DATA_SIZE_SINT16:
@@ -94,6 +107,11 @@
rc = PLDM_ERROR_INVALID_DATA;
}
value = rawValue;
+ if (propertyType == "uint64_t")
+ {
+ auto tempValue = std::get<int16_t>(value);
+ value = static_cast<uint64_t>(tempValue);
+ }
break;
}
case PLDM_EFFECTER_DATA_SIZE_UINT32:
@@ -107,6 +125,11 @@
rc = PLDM_ERROR_INVALID_DATA;
}
value = rawValue;
+ if (propertyType == "uint64_t")
+ {
+ auto tempValue = std::get<uint32_t>(value);
+ value = static_cast<uint64_t>(tempValue);
+ }
break;
}
case PLDM_EFFECTER_DATA_SIZE_SINT32:
@@ -120,6 +143,11 @@
rc = PLDM_ERROR_INVALID_DATA;
}
value = rawValue;
+ if (propertyType == "uint64_t")
+ {
+ auto tempValue = std::get<int32_t>(value);
+ value = static_cast<uint64_t>(tempValue);
+ }
break;
}
}
@@ -131,45 +159,47 @@
* @param[in] pdr - The structure of pldm_numeric_effecter_value_pdr.
* @param[in] effecterDataSize - effecter value size.
* @param[in,out] effecterValue - effecter value.
+ * @param[in] propertyType - type of the D-Bus property.
*
* @return std::pair<int, std::optional<PropertyValue>> - rc:Success or
* failure, PropertyValue: The value to be set
*/
std::pair<int, std::optional<PropertyValue>>
convertToDbusValue(const pldm_numeric_effecter_value_pdr* pdr,
- uint8_t effecterDataSize, uint8_t* effecterValue)
+ uint8_t effecterDataSize, uint8_t* effecterValue,
+ std::string propertyType)
{
if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_UINT8)
{
uint8_t currentValue = *(reinterpret_cast<uint8_t*>(&effecterValue[0]));
- return getEffecterRawValue<uint8_t>(pdr, currentValue);
+ return getEffecterRawValue<uint8_t>(pdr, currentValue, propertyType);
}
else if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_SINT8)
{
int8_t currentValue = *(reinterpret_cast<int8_t*>(&effecterValue[0]));
- return getEffecterRawValue<int8_t>(pdr, currentValue);
+ return getEffecterRawValue<int8_t>(pdr, currentValue, propertyType);
}
else if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_UINT16)
{
uint16_t currentValue =
*(reinterpret_cast<uint16_t*>(&effecterValue[0]));
- return getEffecterRawValue<uint16_t>(pdr, currentValue);
+ return getEffecterRawValue<uint16_t>(pdr, currentValue, propertyType);
}
else if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_SINT16)
{
int16_t currentValue = *(reinterpret_cast<int16_t*>(&effecterValue[0]));
- return getEffecterRawValue<int16_t>(pdr, currentValue);
+ return getEffecterRawValue<int16_t>(pdr, currentValue, propertyType);
}
else if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_UINT32)
{
uint32_t currentValue =
*(reinterpret_cast<uint32_t*>(&effecterValue[0]));
- return getEffecterRawValue<uint32_t>(pdr, currentValue);
+ return getEffecterRawValue<uint32_t>(pdr, currentValue, propertyType);
}
else if (effecterDataSize == PLDM_EFFECTER_DATA_SIZE_SINT32)
{
int32_t currentValue = *(reinterpret_cast<int32_t*>(&effecterValue[0]));
- return getEffecterRawValue<int32_t>(pdr, currentValue);
+ return getEffecterRawValue<int32_t>(pdr, currentValue, propertyType);
}
else
{
@@ -243,14 +273,6 @@
return PLDM_ERROR_INVALID_DATA;
}
- // convert to dbus effectervalue according to the factor
- auto [rc, dbusValue] =
- convertToDbusValue(pdr, effecterDataSize, effecterValue);
- if (rc != PLDM_SUCCESS)
- {
- return rc;
- }
-
try
{
const auto& [dbusMappings, dbusValMaps] =
@@ -258,6 +280,14 @@
DBusMapping dbusMapping{
dbusMappings[0].objectPath, dbusMappings[0].interface,
dbusMappings[0].propertyName, dbusMappings[0].propertyType};
+
+ // convert to dbus effectervalue according to the factor
+ auto [rc, dbusValue] = convertToDbusValue(
+ pdr, effecterDataSize, effecterValue, dbusMappings[0].propertyType);
+ if (rc != PLDM_SUCCESS)
+ {
+ return rc;
+ }
try
{
diff --git a/libpldmresponder/test/libpldmresponder_platform_test.cpp b/libpldmresponder/test/libpldmresponder_platform_test.cpp
index b8fe603..accdb60 100644
--- a/libpldmresponder/test/libpldmresponder_platform_test.cpp
+++ b/libpldmresponder/test/libpldmresponder_platform_test.cpp
@@ -344,7 +344,7 @@
uint16_t effecterId = 3;
uint32_t effecterValue = 2100000000; // 2036-07-18 21:20:00
- PropertyValue propertyValue = static_cast<uint32_t>(effecterValue);
+ PropertyValue propertyValue = static_cast<uint64_t>(effecterValue);
DBusMapping dbusMapping{"/foo/bar", "xyz.openbmc_project.Foo.Bar",
"propertyName", "uint64_t"};