Create and update Asset tag for the system #483
Conversation
SunnySrivastava1984
force-pushed
the
AssetTag
branch
from
5d25e1b
to
dd6b211
Compare
SunnySrivastava1984
requested review from
jinuthomas,
PriyangaRamasamy,
souvik1914581,
RekhaAparna01 and
branupama
| @@ -417,6 +417,8 @@ void Worker::setDeviceTreeAndJson() | |||
| ec.message()); | |||
| } | |||
| } | |||
|
|
|||
| m_isFactoryResetDone = true; | |||
There was a problem hiding this comment.
will the above throw cause any issue to this value ? guess not , wanted your confirmation
There was a problem hiding this comment.
No, as of now, in case of throw above this place, we exit the service. Hence from our application point of view we have still not processed factory reset flow.
| @@ -417,6 +417,8 @@ void Worker::setDeviceTreeAndJson() | |||
| ec.message()); | |||
| } | |||
| } | |||
|
|
|||
| m_isFactoryResetDone = true; | |||
There was a problem hiding this comment.
I think "m_isFactoryResetApplied" suits more
There was a problem hiding this comment.
No, Applied has a different meaning.
Done implies that an action has been finished. This variable denotes that when we are reaching this point of code, there has been a factory reset done in the system.
| if (auto l_parsedVpdMap = | ||
| std::get_if<types::IPZVpdMap>(&i_parsedVpdMap)) | ||
| { | ||
| auto l_itrToVsys = (*l_parsedVpdMap).find(constants::recVSYS); |
There was a problem hiding this comment.
we can create this variable inside if condition .
As this variable is not being used outside anywhere.
There was a problem hiding this comment.
"l_itrToVsys" is being used in the "if" condition itself.
We can't check it before creation.
There was a problem hiding this comment.
I meant to create in the if condition itself.
but still its fine as its scope is limited.
src/worker.cpp
Outdated
| if (l_itrToSystemPath == objectInterfaceMap.end()) | ||
| { | ||
| throw std::runtime_error( | ||
| "System Path not found in object map."); |
There was a problem hiding this comment.
All these throw will be caught at line 1194.
At line 1194 we have to implement a pel. There the path can be added as callout. If I add that again in the trace. It will be duplicated.
SunnySrivastava1984
force-pushed
the
AssetTag
branch
from
dd6b211
to
95fcb6e
Compare
SunnySrivastava1984
requested review from
jinuthomas,
PriyangaRamasamy and
branupama
SunnySrivastava1984
force-pushed
the
AssetTag
branch
2 times, most recently
from
892f784
to
ed55c01
Compare
src/worker.cpp
Outdated
| } | ||
| else | ||
| { | ||
| throw std::runtime_error("Asset tag string is empty."); |
There was a problem hiding this comment.
don't we need to still create the Asset tag object and property even if we cannot fill the value ?
There was a problem hiding this comment.
Yes we should add, so that it can be updated via GUI.
I have updated the code to handle the empty scenario as well.
The commit creates and Asset tag for the system in case of first time boot or factory reset situation and adds Asset tag inerface under system inevntory path. It also registers callback to detect any change in asset tag value and calls PIM notify to persist the data. Signed-off-by: Sunny Srivastava <[email protected]>
SunnySrivastava1984
force-pushed
the
AssetTag
branch
from
ed55c01
to
efdac64
Compare
| @@ -417,6 +417,8 @@ void Worker::setDeviceTreeAndJson() | |||
| ec.message()); | |||
| } | |||
| } | |||
|
|
|||
| m_isFactoryResetDone = true; | |||
| if (auto l_parsedVpdMap = | ||
| std::get_if<types::IPZVpdMap>(&i_parsedVpdMap)) | ||
| { | ||
| auto l_itrToVsys = (*l_parsedVpdMap).find(constants::recVSYS); |
There was a problem hiding this comment.
I meant to create in the if condition itself.
but still its fine as its scope is limited.
The commit creates and Asset tag for the system in case of first time boot or factory reset situation and adds Asset tag inerface under system inevntory path.
It also registers callback to detect any change in asset tag value and calls PIM notify to persist the data.