Implement vpd-tool dumpObject stub #501

souvik1914581 · 2024-11-22T06:56:12Z

This commit implements vpd-tool --dumpObject stub and some associated
utility methods and constants for the same.
This commit allows user to provide --dumpObject command line option to
vpd-tool.

For a given Object path, the object dump functionality prints the
following properties in a JSON format to the console:

Present property, Pretty Name, Location Code, Sub Model
SN, PN, CC, FN, DR keywords under VINI record

Note: This commit does not include actual implementation of
`--dumpObject`.

This commit includes changes and new files required for implementation of vpd-tool --dumpObject and --dumpInventory options. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

PriyangaRamasamy · 2024-11-22T07:43:06Z

This commit is not just for dump object and dump inventory. this is an initial setup required for vpd-tool APIs.

Can you please update the commit heading and the description.

This commit includes changes and new files required for implementation of vpd-tool APIs. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

souvik1914581 · 2024-11-22T08:58:40Z

This commit is not just for dump object and dump inventory. this is an initial setup required for vpd-tool APIs.

Can you please update the commit heading and the description.

Done

vpd-tool/include/tool_json_utility.hpp

vpd-tool/include/utils.hpp

This commit includes changes and new files required for implementation of vpd-tool APIs. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

branupama · 2024-11-25T04:24:56Z

This commit includes changes and new files required for implementation
of vpd-tool APIs.

The commit message is not giving any hint of why and what this commit is doing.

vpd-tool/include/utils.hpp

souvik1914581 · 2024-11-25T05:09:17Z

This commit includes changes and new files required for implementation
of vpd-tool APIs.

The commit message is not giving any hint of why and what this commit is doing.

Can you mention what is not clear? So that I can update the commit message accordingly.

This commit includes changes and new source code files required for implementation of vpd-tool APIs. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

This commit implements vpd-tool --dumpObject stub and some associated utility methods and constants for the same. This commit allows user to provide --dumpObject command line option to vpd-tool. Note: This commit does not include actual implementation of --dumpObject. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

PriyangaRamasamy · 2024-12-09T10:57:58Z

vpd-tool/include/vpd_tool.hpp

+     * Note: The caller of this API should
+     * check for empty JSON object
+     *
+     * @throw std::runtime_error


on error scenarios - sometimes you throw exception and sometimes you return empty JSON object. can we fix with any one ?

on failure empty JSON should be sent ?

PriyangaRamasamy · 2024-12-09T10:58:45Z

vpd-tool/src/vpd_tool.cpp

+{
+    nlohmann::json l_resultInJson = nlohmann::json::array({});
+    const nlohmann::json& l_sysCfgJsonObj =
+        vpd::jsonUtility::getParsedJson(INVENTORY_JSON_SYM_LINK);


getParsedJson() throws exception. can you catch the exception here and take necessary action in this API?
necessary action llike - catch the exception and return empty json from here.

PriyangaRamasamy · 2024-12-09T11:02:36Z

vpd-tool/include/vpd_tool.hpp

+     *
+     * @throw std::runtime_error
+     */
+    nlohmann::json getFruPropertiesJson(const std::string& i_fruPath) const;


name doesn't gives the clarity on what this API is.
can you change it to something like - getFruProperties() -> as return type tells the format, it's not required to mention JSON format in API name.

PriyangaRamasamy · 2024-12-09T11:04:30Z

vpd-tool/src/vpd_tool_main.cpp

@@ -46,6 +51,9 @@ int main(int argc, char** argv)
    auto l_hardwareFlag = l_app.add_flag("--Hardware, -H",
                                         "CAUTION: Developer only option.");

+    auto l_dumpObjFlag = l_app.add_flag("--dumpObject, -o", "Dump Object")


Description something like : "Dump inventory object properties."

PriyangaRamasamy · 2024-12-09T11:09:19Z

vpd-tool/meson.build

@@ -13,7 +13,7 @@ sources = ['src/vpd_tool_main.cpp',

 vpd_tool_exe = executable('vpd-tool',
                          sources,
-                          include_directories : ['include/'],
+                          include_directories : ['include/','../'],


why do you include the directory out of vpd-tool directory? ( ../)

PriyangaRamasamy · 2024-12-09T11:12:48Z

vpd-tool/include/vpd_tool.hpp

+     * of the FRU in JSON format to console:
+     * - Present property, Pretty Name, Location Code, Sub Model
+     * - SN, PN, CC, FN, DR keywords under VINI record.
+     * - type and TYPE of the FRU


what is type and TYPE of the FRU?

From the old vpd-tool ,

"type" property in object dump specifies the first extraInterfaces interface name which is null.Eg. xyz.openbmc_project.Inventory.Item.Cpu

"TYPE" tag default value is "FRU", unless there is a "type" tag in the System Config JSON. As per new requirement, since vpd-tool doesn't have System Config JSON, I think we may have to skip dumping this "TYPE" tag.

souvik1914581 · 2024-12-10T09:27:04Z

As per discussion on 10th December, 2024, following new design changes need to be implemented in dumpObject option:

Tool shouldn’t depend on the JSON
Use object paths from PIM
Dump Object in dictionary format(same as existing).
If Present is false: don’t show anything (for single object).
Display “FRU is not present in the system” to user.
See https://ibm.ent.box.com/file/1610305628598

This commit implements vpd-tool --dumpObject stub and some associated utility methods and constants for the same. This commit allows user to provide --dumpObject command line option to vpd-tool. Note: This commit does not include actual implementation of --dumpObject. Change-Id: I821777237a3080d390e2bba151cd685ef8994bf9 Signed-off-by: Souvik Roy <[email protected]>

branupama · 2024-12-12T06:37:45Z

vpd-tool/src/vpd_tool_main.cpp

-        "vpd-tool -r -H -O <EEPROM Path> -R <Record Name> -K <Keyword Name> --file <File Path>");
+        "vpd-tool -r -H -O <EEPROM Path> -R <Record Name> -K <Keyword Name> --file <File Path>\n"
+        "Dump Object:\n"
+        "    From dbus to console: "


to consistent with existing, use DBus

branupama · 2024-12-13T06:11:49Z

vpd-tool/include/vpd_tool.hpp

+     * Note: The caller of this API should
+     * check for empty JSON object
+     *
+     * @throw std::runtime_error


on failure empty JSON should be sent ?

branupama · 2024-12-13T06:12:29Z

vpd-tool/include/vpd_tool.hpp

+    /**
+     * @brief Dump the given inventory object in JSON format to console.
+     *
+     * For a given Object Path of a FRU, this API dumps the following properties


object path

branupama · 2024-12-13T06:32:50Z

vpd-tool/src/vpd_tool_main.cpp

+    else if (!l_dumpObjFlag->empty())
+    {
+        vpd::VpdTool l_vpdToolObj;
+        l_rc = l_vpdToolObj.dumpObject(l_vpdPath);


should we check l_vpdPath is empty, incase if Object path is provided like -O "" ?

branupama · 2024-12-13T06:34:15Z

vpd-tool/src/vpd_tool.cpp

+    }
+    catch (std::exception& l_ex)
+    {
+        std::cerr << "Dump Object failed for FRU: " << i_fruPath


add todo for enable log when verbose is enabled

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 255b227 to 46ace05 Compare November 22, 2024 06:57

souvik1914581 requested review from PriyangaRamasamy, branupama, jinuthomas, SunnySrivastava1984 and RekhaAparna01 November 22, 2024 07:02

souvik1914581 changed the title ~~Infra setup for vpd-tool dump object and inventory~~ Initial setup for vpd-tool APIs Nov 22, 2024

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 46ace05 to 8abb7f0 Compare November 22, 2024 08:57

PriyangaRamasamy suggested changes Nov 22, 2024

View reviewed changes

vpd-tool/include/tool_json_utility.hpp Outdated Show resolved Hide resolved

vpd-tool/include/utils.hpp Outdated Show resolved Hide resolved

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 8abb7f0 to c110e39 Compare November 22, 2024 09:14

souvik1914581 requested a review from PriyangaRamasamy November 25, 2024 03:12

branupama suggested changes Nov 25, 2024

View reviewed changes

vpd-tool/include/utils.hpp Outdated Show resolved Hide resolved

souvik1914581 marked this pull request as draft November 25, 2024 05:17

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from c110e39 to 4fd701e Compare November 25, 2024 05:35

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 4fd701e to 4f0b6b1 Compare November 26, 2024 04:49

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 4f0b6b1 to 3e2f9fb Compare December 2, 2024 06:27

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 3e2f9fb to f3bd78b Compare December 2, 2024 14:35

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 5a0f5e8 to 78db080 Compare December 9, 2024 06:48

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 78db080 to 3b0a111 Compare December 9, 2024 06:54

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 3b0a111 to 52e42e2 Compare December 9, 2024 06:56

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 52e42e2 to 52836c3 Compare December 9, 2024 07:01

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 52836c3 to 46f24dd Compare December 9, 2024 07:03

souvik1914581 requested review from branupama and jinuthomas December 9, 2024 07:06

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 46f24dd to 2b0ee20 Compare December 9, 2024 09:11

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 2b0ee20 to 47e30cc Compare December 9, 2024 09:18

PriyangaRamasamy suggested changes Dec 9, 2024

View reviewed changes

souvik1914581 marked this pull request as draft December 10, 2024 09:22

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 47e30cc to 878f6f9 Compare December 10, 2024 09:41

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 878f6f9 to 34ac1af Compare December 10, 2024 09:46

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 34ac1af to 715ae69 Compare December 10, 2024 09:50

souvik1914581 marked this pull request as ready for review December 10, 2024 09:54

souvik1914581 force-pushed the sr_vpdToolDumpStub branch from 715ae69 to 61a1d31 Compare December 10, 2024 09:57

souvik1914581 requested a review from PriyangaRamasamy December 10, 2024 10:18

branupama suggested changes Dec 13, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement vpd-tool dumpObject stub #501

Implement vpd-tool dumpObject stub #501

souvik1914581 commented Nov 22, 2024 •

edited

Loading

PriyangaRamasamy commented Nov 22, 2024

souvik1914581 commented Nov 22, 2024

branupama commented Nov 25, 2024

souvik1914581 commented Nov 25, 2024

PriyangaRamasamy Dec 9, 2024

souvik1914581 Dec 10, 2024

branupama Dec 13, 2024

PriyangaRamasamy Dec 9, 2024

PriyangaRamasamy Dec 9, 2024

PriyangaRamasamy Dec 9, 2024

PriyangaRamasamy Dec 9, 2024

PriyangaRamasamy Dec 9, 2024

souvik1914581 Dec 10, 2024 •

edited

Loading

souvik1914581 commented Dec 10, 2024

branupama Dec 12, 2024

branupama Dec 13, 2024

branupama Dec 13, 2024

branupama Dec 13, 2024

branupama Dec 13, 2024

Implement vpd-tool dumpObject stub #501

Are you sure you want to change the base?

Implement vpd-tool dumpObject stub #501

Conversation

souvik1914581 commented Nov 22, 2024 • edited Loading

PriyangaRamasamy commented Nov 22, 2024

souvik1914581 commented Nov 22, 2024

branupama commented Nov 25, 2024

souvik1914581 commented Nov 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

souvik1914581 Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

souvik1914581 commented Dec 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

souvik1914581 commented Nov 22, 2024 •

edited

Loading

souvik1914581 Dec 10, 2024 •

edited

Loading