libperfmgr: change to use Value directly in config

Using Value directly is more clear and intuitive than using an index of
Values.

Bug: 77282526
Test: libperfmgr_test and boot wahoo
Change-Id: I4414a2bb04be8bf3fc01c20695ebd6ac3c19df79

8 files changed, 46 insertions, 53 deletions

diff --git a/libperfmgr/HintManager.cc b/libperfmgr/HintManager.cc
index ce3d125c..691bd41b 100644
--- a/libperfmgr/HintManager.cc
+++ b/libperfmgr/HintManager.cc
@@ -82,11 +82,12 @@ std::vector<std::string> HintManager::GetHints() const {
 }
 
 void HintManager::DumpToFd(int fd) {
-    std::string header("========== Begin perfmgr nodes ==========\n"
-                       "Node Name\t"
-                       "Node Path\t"
-                       "Current Index\t"
-                       "Current Value\n");
+    std::string header(
+        "========== Begin perfmgr nodes ==========\n"
+        "Node Name\t"
+        "Node Path\t"
+        "Current Index\t"
+        "Current Value\n");
     if (!android::base::WriteStringToFd(header, fd)) {
         LOG(ERROR) << "Failed to dump fd: " << fd;
     }
@@ -158,7 +159,7 @@ std::vector<std::unique_ptr<Node>> HintManager::ParseNodes(
             LOG(ERROR) << "Duplicate Node[" << i << "]'s Name";
             nodes_parsed.clear();
             return nodes_parsed;
-        };
+        }
 
         std::string path = nodes[i]["Path"].asString();
         LOG(VERBOSE) << "Node[" << i << "]'s Path: " << path;
@@ -284,27 +285,18 @@ std::map<std::string, std::vector<NodeAction>> HintManager::ParseActions(
         }
         node_index = nodes_index[node_name];
 
-        Json::UInt64 value_index = 0;
-        if (actions[i]["ValueIndex"].empty() ||
-            !actions[i]["ValueIndex"].isUInt64()) {
-            LOG(ERROR) << "Failed to read Action" << i << "'s ValueIndex";
-            actions_parsed.clear();
-            return actions_parsed;
-        } else {
-            value_index = actions[i]["ValueIndex"].asUInt64();
-        }
-        std::size_t max_value_index = nodes[node_index]->GetValues().size() - 1;
-        if (value_index > max_value_index) {
-            LOG(ERROR) << "Failed to read Action" << i << "'s Duration";
-            LOG(ERROR) << "Action[" << i
-                       << "]'s ValueIndex out of bound, value index: "
-                       << value_index << ", max: " << max_value_index;
+        std::string value_name = actions[i]["Value"].asString();
+        LOG(VERBOSE) << "Action[" << i << "]'s Value: " << value_name;
+        std::size_t value_index = 0;
+
+        if (!nodes[node_index]->GetValueIndex(value_name, &value_index)) {
+            LOG(ERROR) << "Failed to read Action" << i << "'s Value";
+            LOG(ERROR) << "Action[" << i << "]'s Value " << value_name
+                       << " is not defined in Node[" << node_name;
             actions_parsed.clear();
             return actions_parsed;
         }
         LOG(VERBOSE) << "Action[" << i << "]'s ValueIndex: " << value_index;
-        LOG(VERBOSE) << "Action[" << i << "]'s Node Value: "
-                     << nodes[node_index]->GetValues()[value_index];
 
         Json::UInt64 duration = 0;
         if (actions[i]["Duration"].empty() ||
@@ -319,8 +311,7 @@ std::map<std::string, std::vector<NodeAction>> HintManager::ParseActions(
 
         if (actions_parsed.find(hint_type) == actions_parsed.end()) {
             actions_parsed[hint_type] = std::vector<NodeAction>{
-                {node_index, static_cast<std::size_t>(value_index),
-                 std::chrono::milliseconds(duration)}};
+                {node_index, value_index, std::chrono::milliseconds(duration)}};
         } else {
             for (const auto& action : actions_parsed[hint_type]) {
                 if (action.node_index == node_index) {
@@ -332,8 +323,7 @@ std::map<std::string, std::vector<NodeAction>> HintManager::ParseActions(
                 }
             }
             actions_parsed[hint_type].emplace_back(
-                node_index, static_cast<std::size_t>(value_index),
-                std::chrono::milliseconds(duration));
+                node_index, value_index, std::chrono::milliseconds(duration));
         }
 
         ++total_parsed;
diff --git a/libperfmgr/Node.cc b/libperfmgr/Node.cc
index 53677700..63cf8aaa 100644
--- a/libperfmgr/Node.cc
+++ b/libperfmgr/Node.cc
@@ -18,8 +18,8 @@
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
-#include <android-base/strings.h>
 #include <android-base/stringprintf.h>
+#include <android-base/strings.h>
 
 #include "perfmgr/Node.h"
 
@@ -154,11 +154,9 @@ void Node::DumpToFd(int fd) {
         LOG(ERROR) << "Failed to read node path: " << node_path_;
     }
     node_value = android::base::Trim(node_value);
-    std::string buf(android::base::StringPrintf("%s\t%s\t%zu\t%s\n",
-                                                name_.c_str(),
-                                                node_path_.c_str(),
-                                                current_val_index_,
-                                                node_value.c_str()));
+    std::string buf(android::base::StringPrintf(
+        "%s\t%s\t%zu\t%s\n", name_.c_str(), node_path_.c_str(),
+        current_val_index_, node_value.c_str()));
     if (!android::base::WriteStringToFd(buf, fd)) {
         LOG(ERROR) << "Failed to dump fd: " << fd;
     }
diff --git a/libperfmgr/config_schema.json b/libperfmgr/config_schema.json
index 0fffe509..b74bacbe 100644
--- a/libperfmgr/config_schema.json
+++ b/libperfmgr/config_schema.json
@@ -97,15 +97,15 @@
             "type": "string",
             "id": "/properties/Actions/items/properties/Node",
             "title": "The Node Schema.",
-            "description": "The Node name of the action.",
+            "description": "The Node name of the action, which is defined in Nodes.",
             "minLength": 1
           },
-          "ValueIndex": {
-            "type": "integer",
-            "id": "/properties/Actions/items/properties/ValueIndex",
+          "Value": {
+            "type": "string",
+            "id": "/properties/Actions/items/properties/Value",
             "title": "The Value Index Schema.",
-            "description": "The value index of the action.",
-            "minimum": 0
+            "description": "The value of action, which is defined in Nodes.",
+            "minLength": 1
           },
           "Duration": {
             "type": "integer",
diff --git a/libperfmgr/include/perfmgr/Node.h b/libperfmgr/include/perfmgr/Node.h
index f31b0128..ea4c3e34 100644
--- a/libperfmgr/include/perfmgr/Node.h
+++ b/libperfmgr/include/perfmgr/Node.h
@@ -42,8 +42,8 @@ namespace perfmgr {
 // value. For each value, there may be multiple requests because different
 // powerhints may request the same value, and the requests may have different
 // expiration times. All of the in-progress powerhints for a given value are
-// collected in a RequestGroup. Node class is not thread safe so it need protection
-// from caller e.g. NodeLooperThread.
+// collected in a RequestGroup. Node class is not thread safe so it needs
+// protection from caller e.g. NodeLooperThread.
 class Node {
   public:
     Node(std::string name, std::string node_path,
diff --git a/libperfmgr/tests/HintManagerTest.cc b/libperfmgr/tests/HintManagerTest.cc
index 0630fe7f..2355b8ff 100644
--- a/libperfmgr/tests/HintManagerTest.cc
+++ b/libperfmgr/tests/HintManagerTest.cc
@@ -61,19 +61,19 @@ constexpr auto kSLEEP_TOLERANCE_MS = 50ms;
 //         {
 //             "PowerHint": "INTERACTION",
 //             "Node": "CPUCluster1MinFreq",
-//             "ValueIndex": 1,
+//             "Value": "1134000",
 //             "Duration": 800
 //         },
 //         {
 //             "PowerHint": "LAUNCH",
 //             "Node": "CPUCluster0MinFreq",
-//             "ValueIndex": 1,
+//             "Value": "1134000",
 //             "Duration": 500
 //         },
 //         {
 //             "PowerHint": "LAUNCH",
 //             "Node": "CPUCluster1MinFreq",
-//             "ValueIndex": 0,
+//             "Value": "1512000",
 //             "Duration": 2000
 //         }
 //     ]
@@ -86,10 +86,12 @@ constexpr char kJSON_RAW[] =
     "\"Path\":\"/sys/devices/system/cpu/cpu4/cpufreq/"
     "scaling_min_freq\",\"Values\":[\"1512000\",\"1134000\",\"384000\"],"
     "\"HoldFd\":true}],\"Actions\":[{\"PowerHint\":\"INTERACTION\",\"Node\":"
-    "\"CPUCluster1MinFreq\",\"ValueIndex\":1,\"Duration\":800},{\"PowerHint\":"
-    "\"LAUNCH\",\"Node\":\"CPUCluster0MinFreq\",\"ValueIndex\":1,\"Duration\":"
+    "\"CPUCluster1MinFreq\",\"Value\":\"1134000\",\"Duration\":800},{"
+    "\"PowerHint\":"
+    "\"LAUNCH\",\"Node\":\"CPUCluster0MinFreq\",\"Value\":\"1134000\","
+    "\"Duration\":"
     "500},{\"PowerHint\":\"LAUNCH\",\"Node\":\"CPUCluster1MinFreq\","
-    "\"ValueIndex\":0,\"Duration\":2000}]}";
+    "\"Value\":\"1512000\",\"Duration\":2000}]}";
 
 class HintManagerTest : public ::testing::Test, public HintManager {
   protected:
@@ -150,7 +152,8 @@ class HintManagerTest : public ::testing::Test, public HintManager {
     std::string json_doc_;
 };
 
-static inline void _VerifyPathValue(const std::string& path, const std::string& value) {
+static inline void _VerifyPathValue(const std::string& path,
+                                    const std::string& value) {
     std::string s;
     EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno);
     EXPECT_EQ(value, s);
diff --git a/libperfmgr/tests/NodeLooperThreadTest.cc b/libperfmgr/tests/NodeLooperThreadTest.cc
index 6f96cf8f..24659cf7 100644
--- a/libperfmgr/tests/NodeLooperThreadTest.cc
+++ b/libperfmgr/tests/NodeLooperThreadTest.cc
@@ -54,7 +54,8 @@ class NodeLooperThreadTest : public ::testing::Test {
     std::vector<std::unique_ptr<TemporaryFile>> files_;
 };
 
-static inline void _VerifyPathValue(const std::string& path, const std::string& value) {
+static inline void _VerifyPathValue(const std::string& path,
+                                    const std::string& value) {
     std::string s;
     EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno);
     EXPECT_EQ(value, s);
diff --git a/libperfmgr/tests/NodeTest.cc b/libperfmgr/tests/NodeTest.cc
index 2e37a4d5..02429ccd 100644
--- a/libperfmgr/tests/NodeTest.cc
+++ b/libperfmgr/tests/NodeTest.cc
@@ -33,7 +33,8 @@ using namespace std::chrono_literals;
 constexpr double kTIMING_TOLERANCE_MS = std::chrono::milliseconds(25).count();
 constexpr auto kSLEEP_TOLERANCE_MS = 2ms;
 
-static inline void _VerifyPathValue(const std::string& path, const std::string& value) {
+static inline void _VerifyPathValue(const std::string& path,
+                                    const std::string& value) {
     std::string s;
     EXPECT_TRUE(android::base::ReadFileToString(path, &s)) << strerror(errno);
     EXPECT_EQ(value, s);
@@ -63,8 +64,8 @@ TEST(NodeTest, DumpToFdTest) {
     TemporaryFile dumptf;
     t.DumpToFd(dumptf.fd);
     fsync(dumptf.fd);
-    std::string buf(android::base::StringPrintf("test_dump\t%s\t1\tvalue1\n",
-                                                tf.path));
+    std::string buf(
+        android::base::StringPrintf("test_dump\t%s\t1\tvalue1\n", tf.path));
     _VerifyPathValue(dumptf.path, buf);
 }
 
diff --git a/libperfmgr/tools/ConfigVerifier.cc b/libperfmgr/tools/ConfigVerifier.cc
index 9fadd7d8..3b771a91 100644
--- a/libperfmgr/tools/ConfigVerifier.cc
+++ b/libperfmgr/tools/ConfigVerifier.cc
@@ -79,7 +79,7 @@ static void printUsage(const char* exec_name) {
         usage +
         " is a command-line tool to verify Nodes in Json config are writable.\n"
         "Usages:\n"
-        "    [su system]" +
+        "    [su system] " +
         exec_name +
         " [options]\n"
         "\n"