pldmtool: Fix SetStateEffecterStates command
Fix and use the following command to set state effecter PDR:
pldmtool platform SetStateEffecterStates --id <effecterId> --count
<count> --data <setRequest effecterState ...>
Tested:
$ pldmtool platform SetStateEffecterStates -i 1 -c 1 -d 0 2
set_request = 0
Encode request successfully
Request Message:
08 01 80 02 39 01 00 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Success in creating the socket : RC = 3
Success in connecting to socket : RC = 0
Success in sending message type as pldm to mctp : RC = 0
Write to socket successful : RC = 24
Total length:24
Loopback response message:
08 01 80 02 39 01 00 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00
On first recv(),response == request : RC = 0
Total length: 6
Shutdown Socket successful : RC = 0
Response Message:
08 01 00 02 39 00
SetStateEffecterStates: SUCCESS
$ pldmtool platform SetStateEffecterStates -i 1 -c 1 -d 1 2
set_request = 1
Encode request successfully
Request Message:
08 01 80 02 39 01 00 01 01 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Success in creating the socket : RC = 3
Success in connecting to socket : RC = 0
Success in sending message type as pldm to mctp : RC = 0
Write to socket successful : RC = 24
Total length:24
Loopback response message:
08 01 80 02 39 01 00 01 01 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00
On first recv(),response == request : RC = 0
Total length: 6
Shutdown Socket successful : RC = 0
Response Message:
08 01 00 02 39 00
SetStateEffecterStates: SUCCESS
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I4a2e71b149040c3e0c0f585f8fae0e5e25ec7ceb
diff --git a/test/pldm_utils_test.cpp b/test/pldm_utils_test.cpp
index b512db9..c880c20 100644
--- a/test/pldm_utils_test.cpp
+++ b/test/pldm_utils_test.cpp
@@ -50,35 +50,30 @@
EXPECT_EQ(ret, false);
}
-TEST(decodeEffecterData, testGoodDecodeEffecterData)
+TEST(parseEffecterData, testGoodDecodeEffecterData)
{
- std::vector<uint8_t> effecterData = {2, 1, 1, 0, 1, 2};
- uint16_t effecter_id = 2;
+ std::vector<uint8_t> effecterData = {1, 1, 0, 1};
+ uint8_t effecterCount = 2;
set_effecter_state_field stateField0 = {1, 1};
- set_effecter_state_field stateField1 = {0, 0};
- set_effecter_state_field stateField2 = {1, 2};
+ set_effecter_state_field stateField1 = {0, 1};
- uint16_t retEffecter_id = 0;
- std::vector<set_effecter_state_field> stateField = {};
- auto rc = decodeEffecterData(effecterData, retEffecter_id, stateField);
+ auto effecterField = parseEffecterData(effecterData, effecterCount);
+ EXPECT_NE(effecterField, std::nullopt);
+ EXPECT_EQ(effecterCount, effecterField->size());
- EXPECT_EQ(rc, true);
- EXPECT_EQ(effecter_id, retEffecter_id);
+ std::vector<set_effecter_state_field> stateField = effecterField.value();
EXPECT_EQ(stateField[0].set_request, stateField0.set_request);
EXPECT_EQ(stateField[0].effecter_state, stateField0.effecter_state);
EXPECT_EQ(stateField[1].set_request, stateField1.set_request);
EXPECT_EQ(stateField[1].effecter_state, stateField1.effecter_state);
- EXPECT_EQ(stateField[2].set_request, stateField2.set_request);
- EXPECT_EQ(stateField[2].effecter_state, stateField2.effecter_state);
}
-TEST(decodeEffecterData, testBadDecodeEffecterData)
+TEST(parseEffecterData, testBadDecodeEffecterData)
{
- std::vector<uint8_t> effecterData = {2, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+ std::vector<uint8_t> effecterData = {0, 1, 0, 1, 0, 1};
+ uint8_t effecterCount = 2;
- uint16_t retEffecter_id = 0;
- std::vector<set_effecter_state_field> stateField = {};
- auto rc = decodeEffecterData(effecterData, retEffecter_id, stateField);
+ auto effecterField = parseEffecterData(effecterData, effecterCount);
- EXPECT_EQ(rc, false);
+ EXPECT_EQ(effecterField, std::nullopt);
}
diff --git a/tool/pldm_platform_cmd.cpp b/tool/pldm_platform_cmd.cpp
index ddf30fd..6b1081f 100644
--- a/tool/pldm_platform_cmd.cpp
+++ b/tool/pldm_platform_cmd.cpp
@@ -163,22 +163,28 @@
SetStateEffecter& operator=(const SetStateEffecter&) = delete;
SetStateEffecter& operator=(SetStateEffecter&&) = default;
- // effecterID(1) + compositeEffecterCount(value: 0x01 to 0x08) *
- // stateField(2)
- static constexpr auto maxEffecterDataSize = 17;
+ // compositeEffecterCount(value: 0x01 to 0x08) * stateField(2)
+ static constexpr auto maxEffecterDataSize = 16;
+
+ // compositeEffecterCount(value: 0x01 to 0x08)
+ static constexpr auto minEffecterCount = 1;
+ static constexpr auto maxEffecterCount = 8;
explicit SetStateEffecter(const char* type, const char* name,
CLI::App* app) :
CommandInterface(type, name, app)
{
app->add_option(
+ "-i, --id", effecterId,
+ "A handle that is used to identify and access the effecter")
+ ->required();
+ app->add_option("-c, --count", effecterCount,
+ "The number of individual sets of effecter information")
+ ->required();
+ app->add_option(
"-d,--data", effecterData,
- "set effecter state data, the noChange value is 0 and the "
- "requestSet value is 1 and access up to eight sets of state "
- "effector information. \n"
- "eg1: effecterID, requestSet0, effecterState0... \n"
- "eg2: effecterID, noChange0, requestSet1, effecterState1...")
- ->required()
- ->expected(-1);
+ "Set effecter state data\n"
+ "eg: requestSet0 effecterState0 noChange1 dummyState1 ...")
+ ->required();
}
std::pair<int, std::vector<uint8_t>> createRequestMsg() override
@@ -187,25 +193,35 @@
sizeof(pldm_msg_hdr) + PLDM_SET_STATE_EFFECTER_STATES_REQ_BYTES);
auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
- if (effecterData.size() > maxEffecterDataSize)
+ if (effecterCount > maxEffecterCount ||
+ effecterCount < minEffecterCount)
{
- std::cerr << "Request Message Error: effecterData size "
- << effecterData.size() << std::endl;
+ std::cerr << "Request Message Error: effecterCount size "
+ << effecterCount << "is invalid\n";
auto rc = PLDM_ERROR_INVALID_DATA;
return {rc, requestMsg};
}
- uint16_t effecterId = 0;
- std::vector<set_effecter_state_field> stateField = {};
- if (!decodeEffecterData(effecterData, effecterId, stateField))
+ if (effecterData.size() > maxEffecterDataSize)
{
+ std::cerr << "Request Message Error: effecterData size "
+ << effecterData.size() << "is invalid\n";
+ auto rc = PLDM_ERROR_INVALID_DATA;
+ return {rc, requestMsg};
+ }
+
+ auto stateField = parseEffecterData(effecterData, effecterCount);
+ if (!stateField)
+ {
+ std::cerr << "Failed to parse effecter data, effecterCount size "
+ << effecterCount << "\n";
auto rc = PLDM_ERROR_INVALID_DATA;
return {rc, requestMsg};
}
auto rc = encode_set_state_effecter_states_req(
- PLDM_LOCAL_INSTANCE_ID, effecterId, stateField.size(),
- stateField.data(), request);
+ PLDM_LOCAL_INSTANCE_ID, effecterId, effecterCount,
+ stateField->data(), request);
return {rc, requestMsg};
}
@@ -218,8 +234,7 @@
if (rc != PLDM_SUCCESS || completionCode != PLDM_SUCCESS)
{
std::cerr << "Response Message Error: "
- << "rc=" << rc << ",cc=" << (int)completionCode
- << std::endl;
+ << "rc=" << rc << ",cc=" << (int)completionCode << "\n";
return;
}
@@ -227,6 +242,8 @@
}
private:
+ uint16_t effecterId;
+ uint8_t effecterCount;
std::vector<uint8_t> effecterData;
};
diff --git a/utils.cpp b/utils.cpp
index a69abe6..846297a 100644
--- a/utils.cpp
+++ b/utils.cpp
@@ -49,44 +49,27 @@
return true;
}
-bool decodeEffecterData(const std::vector<uint8_t>& effecterData,
- uint16_t& effecter_id,
- std::vector<set_effecter_state_field>& stateField)
+std::optional<std::vector<set_effecter_state_field>>
+ parseEffecterData(const std::vector<uint8_t>& effecterData,
+ uint8_t effecterCount)
{
- int flag = 0;
- for (auto data : effecterData)
+ std::vector<set_effecter_state_field> stateField;
+
+ if (effecterData.size() != effecterCount * 2)
{
- switch (flag)
- {
- case 0:
- effecter_id = data;
- flag = 1;
- break;
- case 1:
- if (data == PLDM_REQUEST_SET)
- {
- flag = 2;
- }
- else
- {
- stateField.push_back({PLDM_NO_CHANGE, 0});
- }
- break;
- case 2:
- stateField.push_back({PLDM_REQUEST_SET, data});
- flag = 1;
- break;
- default:
- break;
- }
+ return std::nullopt;
}
- if (stateField.size() < 1 || stateField.size() > 8)
+ for (uint8_t i = 0; i < effecterCount; ++i)
{
- return false;
+ uint8_t set_request = effecterData[i * 2] == PLDM_REQUEST_SET
+ ? PLDM_REQUEST_SET
+ : PLDM_NO_CHANGE;
+ set_effecter_state_field filed{set_request, effecterData[i * 2 + 1]};
+ stateField.emplace_back(std::move(filed));
}
- return true;
+ return std::make_optional(std::move(stateField));
}
std::string DBusHandler::getService(const char* path,
diff --git a/utils.hpp b/utils.hpp
index fecf12e..db49f51 100644
--- a/utils.hpp
+++ b/utils.hpp
@@ -77,13 +77,14 @@
/** @brief Convert effecter data to structure of set_effecter_state_field
*
* @param[in] effecterData - the date of effecter
- * @param[out] effecter_id - a handle that is used to identify and access the
- * effecter
- * @param[out] stateField - structure of set_effecter_state_field
+ * @param[in] effecterCount - the number of individual sets of effecter
+ * information
+ * @return[out] parse success and get a valid set_effecter_state_field
+ * structure, return nullopt means parse failed
*/
-bool decodeEffecterData(const std::vector<uint8_t>& effecterData,
- uint16_t& effecter_id,
- std::vector<set_effecter_state_field>& stateField);
+std::optional<std::vector<set_effecter_state_field>>
+ parseEffecterData(const std::vector<uint8_t>& effecterData,
+ uint8_t effecterCount);
/**
* @brief creates an error log