<html><body><p>Hello Yi,<br><br>Thanks for the patch. Here are my comments.<br><br><tt>"openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org> wrote on 11/04/2016 02:40:52 pm:<br><br>> From: OpenBMC Patches <openbmc-patches@stwcx.xyz></tt><br><tt>> To: openbmc@lists.ozlabs.org</tt><br><tt>> Date: 11/04/2016 02:41 pm</tt><br><tt>> Subject: [PATCH skeleton] skeleton: Add new API SystemManager::getFRUObject()</tt><br><tt>> Sent by: "openbmc" <openbmc-bounces+vishwanath=in.ibm.com@lists.ozlabs.org></tt><br><tt>> <br>> ipmi-fru-parser uses getObjectFromId('FRU_STR', fru_area_name) to<br>> ask for the dbus object path of the 'fru_area_name'. The returned<br>> value is also used to check the 'present' status of FRU. Sometimes<br>> the 'fru_area_name' does not defined in Skeleton's ID_LOOKUP table.<br>> system_manager will report a "ERROR: lookup not found" error message.<br>> </tt><br><br><tt><br>> This patch added an API similiar with getObjectFromId().<br>> It does not report error when the 'fru_area_name' is not defined.<br>> It can be used by ipmi-fru-parser to replace getObjectFromId().<br>> </tt><br><br><tt>If something is not there, it is still an error. So masking the error may not be a right approach.</tt><br><br><tt><br>> Also the orignal "ERROR" message is changed to "WARNING".<br>> </tt><br><br><tt>IMO, we may not be solving the root cause here.. "WARNING", -or- "ERROR" would still bother whoever looking into it. </tt><br><tt>So, I still feel that we need to make the actual change.</tt><br><br><tt>When I initially proposed a solution to Norm to remove those ERRORs, he had an opinion that "something not there is not there, so its an error".</tt><br><br><tt>Here is how I look into it ( one of the implementations .. )</tt><br><br><tt>1: writefru data will make a call into a new API passing the fru id.</tt><br><tt>2: Skeleton code will return the available sections for that fru id. ( example: chassis, board / board, product etc )</tt><br><tt>3: writefru will use that and then make getObjectFromByteId for a particular fruid_<area></tt><br><tt><br></tt><br><tt>> Signed-off-by: Yi Li <adamliyi@msn.com><br>> <br>> <!-- Reviewable:start --><br>> ---<br>> This change is <img src="https://reviewable.io/review_button.svg"
> height="35" align="absmiddle" alt="Reviewable"/>(https://<br>> reviewable.io/reviews/openbmc/skeleton/60)<br>> <!-- Reviewable:end --><br>> <br>> <br>> <a href="https://github.com/openbmc/skeleton/pull/60">https://github.com/openbmc/skeleton/pull/60</a><br>> <br>> Yi Li (1):<br>> skeleton: Add new API SystemManager::getFRUObject()<br>> <br>> bin/system_manager.py | 40 +++++++++++++++++++++++++++-------------<br>> 1 file changed, 27 insertions(+), 13 deletions(-)<br>> <br>> -- <br>> 2.7.1<br>> <br>> <br>> _______________________________________________<br>> openbmc mailing list<br>> openbmc@lists.ozlabs.org<br>> <a href="https://lists.ozlabs.org/listinfo/openbmc">https://lists.ozlabs.org/listinfo/openbmc</a><br></tt><BR>
</body></html>